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 timeouts for test/issue-3346.js #3970

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

increase timeouts for test/issue-3346.js #3970

wants to merge 5 commits into from

Conversation

mcollina
Copy link
Member

Fixes #3969

@mcollina
Copy link
Member Author

cc @targos

@targos
Copy link
Member

targos commented Dec 23, 2024

It still fails on my mac:

test at test/issue-3356.js:1:1
✖ test/issue-3356.js (30003.947833ms)
  'test timed out after 30000ms'

I can try to debug a bit after lunch

@targos
Copy link
Member

targos commented Dec 23, 2024

It fails because the expected error doesn't happen (await response.text() resolves).

@targos
Copy link
Member

targos commented Dec 23, 2024

It works if I only increase the last timeout. I pushed some changes, but I don't know if the test is still valid that way.
My commit includes a refactoring to remove the use of tspl so that it doesn't wait for 30 seconds before failing the test in case there's a mismatch between assertions and plan.

@targos
Copy link
Member

targos commented Dec 23, 2024

There's something wrong with the test, I think. Even with 2 seconds, it fails (less often, but still)

@targos
Copy link
Member

targos commented Dec 23, 2024

Is it possible that the issue is not timing-related, but due to another test that has some unintended side-effect?

@targos
Copy link
Member

targos commented Dec 23, 2024

Note that the timeout before the try/catch can be removed and the test will still pass if run in isolation.

@mcollina
Copy link
Member Author

Then let's remove that.

Timing tests are flaky by nature unfortunately :(. We can 100% skip this in CITGM and other envs.

@mcollina mcollina requested a review from ronag December 23, 2024 23:08
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.

test/issue-3356.js very flaky when run with test runner
2 participants