-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Increase timeout to 10s for flaky 3D tileset spec #13061
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
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for the pull request, @Beilinson! ✅ We can confirm we have a CLA on file for you. |
|
This has been ""tracked"" in #12823 and #11446 (note: one is closed, the other one open), with several PRs associated with them where this happened or was supposed to be fixed. We considered that it might be a race condition (e.g. in this issue, but also that it might just be a test that sometimes takes 4999ms and sometimes 5001ms. (I also saw that failure occasionally, but ... who knows what's really happening there...? ... so just shrugged it off and restarted the test....) When you say that increasing the timeout fixes it, then ... I'm a bit surprised: What is actually taking 5 seconds there? And... of course, it's nearly impossible to verify that this consistently fixes the issue. But even if it only causes the issue to come up less frequently, that could be worth merging. |
|
Agreed that its difficult to verify that this even is the problem, thing is I never have encountered this test failing on either my personal laptop or pc from a timeout, meanwhile it fails on at least half of the PR CI runs I've submitted, leading me to think its genuinely a slightly slow test suite that fails sometimes in CI. I did try debugging the test locally and couldn't see anything that would cause this test to fail as often as it does in CI. #11446 I see that @ggetz also suggested increasing the timeout in the poller, so I added that as well just in case |
|
I dont think will fix all of the issues with that suite however, for example https://github.com/CesiumGS/cesium/actions/runs/19678965074/job/56367761968 this run has a different test failing with |
|
I think its worth it to merge just because this issue only appears in CI, if it goes away then great if not then this can be reverted and back to the drawing board 🙄 |

Some of the tests in
Cesium3DTilesetSpec.jshave consistently been failing for a long time already due to a default timeout of 5s, this PR provides a simple fix by increasing the default jasmine timeout to 10s for each of the tests in this suite exclusively, and then resetting it to the original default.Author checklist
CONTRIBUTORS.mdCHANGES.mdwith a short summary of my change