Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase timeout of server tests #1769

Merged
merged 1 commit into from
Nov 29, 2017
Merged

Increase timeout of server tests #1769

merged 1 commit into from
Nov 29, 2017

Conversation

astorije
Copy link
Member

I've noticed this in #1726 and thought it was my fault, but master actually has that issue from time to time, and so did #1767.

I don't know if we want to be okay with the tests timing out or not, so here it is. Also, I didn't want to enable this globally for all tests, because it's only about one test right now. I'd rather avoid allow this globally 🤷‍♂️

@astorije astorije added Meta: Internal This is an internal codebase change (testing, linting, etc.). Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. labels Nov 29, 2017
@astorije astorije added this to the 2.7.0 milestone Nov 29, 2017
@xPaw
Copy link
Member

xPaw commented Nov 29, 2017

I don't like the idea of increasing timeout here, that's just hiding the problem.

@astorije
Copy link
Member Author

Absolutely agree in general. In this case, I figured this was like network-related or something, as we do start a server. I can see a few good reasons why this would range from 1 to 4 seconds to start (network, FS, Travis CPU, etc.).

@xPaw, feel free to close if you think this isn't right or if you know a better way to do that :) I agree, just don't know what can be done.

@xPaw
Copy link
Member

xPaw commented Nov 29, 2017

I don't think it hurts anyhow if the test is detected as slow? If not, just close this.

@astorije
Copy link
Member Author

It hurts when the server takes > 2s to start, CI fails. Unless I misunderstood your comment.

@xPaw
Copy link
Member

xPaw commented Nov 29, 2017

Oh, that was the issue here. In that case I'll merge it, but we probably should look into why that happens.

@astorije
Copy link
Member Author

but we probably should look into why that happens.

Agreed. Can you open an issue for it?

@xPaw xPaw merged commit 53968bf into master Nov 29, 2017
@xPaw xPaw deleted the astorije/test-timeout branch November 29, 2017 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta: Internal This is an internal codebase change (testing, linting, etc.). Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants