Fix repeatedly failing windows test timeouts #5340
Conversation
| await testFile('dvc.yaml') | ||
| }) | ||
| }).timeout(WEBVIEW_TEST_TIMEOUT) | ||
| }).timeout(WEBVIEW_TEST_TIMEOUT) |
There was a problem hiding this comment.
There seems to be an issue with tests inside of describe blocks not getting the timeout. This test was failing with a timeout of 6000 until I moved the .timeout. This PR is also failing with:
1) Experiments Test Suite
Sorting
should not show duplicate rows when sorted:
Error: Timeout of 6000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (d:\a\vscode-dvc\vscode-dvc\extension\dist\test\suite\experiments\index.test.js)
at listOnTimeout (node:internal/timers:569:17)
at process.processTimers (node:internal/timers:512:7)
This sorting test is also relying on its parent describe block for its set webview timeout.
There was a problem hiding this comment.
Reviewed mocha prs and issues, but not seeing anything. It's also strange that we're hitting all the timeouts today when we haven't merged anything recently related to mocha (as far as I can tell).
There was a problem hiding this comment.
I would guess there shouldn't be multiple describe blocks nested (see below)
There was a problem hiding this comment.
Why shouldn't there be multiple describe blocks in a block? The mocha docs show examples of this. Or am I misunderstanding something?
There was a problem hiding this comment.
Why shouldn't there be multiple describe blocks in a block? The mocha docs show examples of this. Or am I misunderstanding something?
Have a look at this and this. For our purposes, we want to avoid extra complexity/parallelization. I have very little faith that suite -> describe -> describe -> it acts the same way as suite -> describe -> it
This sorting test is also relying on its parent describe block for its set webview timeout.
describe blocks should not have .timeout added to them. If you see it that is an error.
There was a problem hiding this comment.
Have a look at this and this. For our purposes, we want to avoid extra complexity/parallelization. I have very little faith that suite -> describe -> describe -> it acts the same way as suite -> describe -> it
Makes sense, thanks for the clarifications! I'm only seeing one example of suite -> describe -> describe -> which is our Custom Plots Creation. None of those tests have failed that I can see, but it would still be quick to remove the extra nesting there.
describe blocks should not have .timeout added to them. If you see it that is an error.
Oh, ok that explains the issue. I actually couldn't find documentation on .timeout which explains how I missed this 🤦♀️.
The tests appear to be running normally again but I'll go ahead and fix the issues mentioned above to ensure this doesn't happen again.
julieg18
force-pushed
the
fix-windows-tests-timeouts
branch
from
69503eb to
90c36c2
Compare
julieg18
changed the title
|
Code Climate has analyzed commit 4fc6f37 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 95.3%. View more on Code Climate. |
2 participants
A couple tests are repeatedly failing in windows. Attempting to fix by adjusting the timeouts.Tests are now running correctly, but there are still some issues that can be corrected:timeoutbeing set ondescribeblocksdescribeblocks being nested inside ofdescribeblocksshould be able to create a background process that does not exit when the parent process exitsneeding a larger timeout