fix(TestCache): eliminate hanging Windows tests#978
Conversation
|
I've tested this branch on MacOS by running many times the TestCache test and I get no failures now. It seems that this fix makes the test stable on MacOS too. |
|
Closing all test caches when we're done with them doesn't (as expected) change the speed of the tests on Linux. It's definitely interesting that the Windows tests take so much longer to run in GitHub Actions so it might be worth the refactor I mentioned above anyway. |
|
Good catch! Let's take the win and fix the CI for now. An automated cleanup could make sense but I'm not sure how that would combine with that specific test where we check a bunch of things after closing. |
|
@zinderic I think if you rebase #933 from |
|
@MichaelMure I'll put a separate PR in for the As an aside, did you have ideas for testing the bug and identities caches (and covering some of the other critical paths in the |
As seen on PR #933 and others, it's common for the Windows test suite to fail when run via GitHub Actions. In that PR, @zinderic also noted that it's less prevalent but can also occur on Mac OSX.
I can't test on Mac OSX but on the
ext4file system, theTempDiris deleted but it's asynchronous and only occurs when there are no open file handles (when thego test ./...ends). On Windows, removing the temporary directory is (more?) synchronous and so Go waits for the OS to remove theTempDirwhile the OS is waiting for the open file handlers to be closed.As seen in the changed file, the solution is to explicitly close both instances of the cache as soon as they're no longer needed. I don't quite understand why the other cache tests don't have this problem but my intuition says that the tests would run faster if those caches were also explicitly closed.
I'm considering refactoring these tests a bit so that there's a
t.Helper()to create and clean-up the caches being tested - what do you think?