chore: make cleanup async
#1156
Conversation
|
Weirdly Windows e2e tests started to time out with 5s now. Even though we set the timeout separately for these tests. Looks like misconfiguration from our side, or a Jest bug. |
molant
force-pushed
the
fix/enotempty
branch
4 times, most recently
from
7bb398c
to
1a0b9bb
Compare
|
@thymikee this is green now. I had to make the cleanup synchronous in the e2e tests to not timeout 🤯 Hopefully this is good enough, can get merged, and Windows is more reliable from now on 😊 |
jest/helpers.ts
Outdated
| @@ -77,7 +80,13 @@ export const makeTemplate = ( | |||
| return values[number - 1]; | |||
| }); | |||
|
|
|||
| export const cleanup = (directory: string) => rimraf.sync(directory); | |||
| export const cleanup = (directory: string, async = true) => { | |||
There was a problem hiding this comment.
Can we have 2 functions named: cleanupSync and cleanup. Passing a boolean as a function parameter is a code-smell.
Using `rimraf.sync` can cause `ENOTEMPTY` and `EBUSY` errors on Windows. This can make the Windows CI to fail randomly. Changing to the async method defaults to autoretry 3 times and thus avoiding this issues.
Summary:
I'm trying to improve the reliability of Windows CI so I can get #1109 green.
I've found out that
cleanup()usesrimraf.syncunderneath. Windows FS is not very... reliable, and sometimes you can end up withENOTEMPTYerrors like this one. In fact,rimraf's documentation calls that out:The problem is the above does not apply when using the sync method:
I've changed it to use the async version and updated the tests. Hopefully that should make Windows more reliable.
Test Plan:
CI