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

add async test timeout #412

Merged
merged 3 commits into from
Jan 11, 2018
Merged

add async test timeout #412

merged 3 commits into from
Jan 11, 2018

Conversation

willingc
Copy link
Collaborator

@willingc willingc commented Jan 11, 2018

Addresses #410

  • Increase ASYNC_TEST_TIMEOUT from default to 15 which is what we have been using for Tornado on JupyterHub tests
  • Increase sleep to 10s

@willingc willingc changed the title [WIP] add async test timeout add async test timeout Jan 11, 2018
@yuvipanda
Copy link
Collaborator

Thanks @willingc. These should fix it, but I think a better longer-term approach would be similar to #413:

  1. Use 'kubectl rollout' to check when hub is ready, rather than listing pods
  2. Make a HTTP request to hub and make sure it responds positively before moving on.

I'm happy to merge this as is too, since it'll fix the issues right now. Mid-to longer term we should do both the above things tor test-main too though.

@willingc
Copy link
Collaborator Author

@yuvipanda I think that we will likely need both this and #413. This one should help with the tests that use @pytest.gen while #413 should help with not starting until ready.

@yuvipanda
Copy link
Collaborator

@willingc oh yeah, I was only talking about the sleep, rather than the ASYNC_TIMEOUT.

@yuvipanda
Copy link
Collaborator

Going to merge this when tests pass unless you have objections!

@willingc
Copy link
Collaborator Author

Go for it @yuvipanda. I'm cool with removing the sleep 10 as well if you want. It was just giving a little extra margin.

@yuvipanda yuvipanda merged commit 12a34e5 into jupyterhub:master Jan 11, 2018
@yuvipanda
Copy link
Collaborator

Thanks for the patch @willingc! I'm happy we're now treating all CI flakes seriously :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants