tests: fix flaky test caused by rapid IO reading and writing #2221
Conversation
WilliamBergamin
added
tests
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #2221 +/- ##
=======================================
Coverage 92.65% 92.65%
=======================================
Files 38 38
Lines 10527 10527
Branches 677 677
=======================================
Hits 9754 9754
Misses 761 761
Partials 12 12
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
@WilliamBergamin This is a super interesting deep dive 🙏 ✨
I'm now curious about the fs operations happening since AFAICT these should by synchronous? Some behind the scenes magic might not be out of the question...
Approving this now since I think it's great continuations of the epic #2159 but I'm also wondering if you noticed certain failures during development? I left comments about logging that might be related!
Overall too I'm a fan of experimenting on these tests in main after a few tests like you've done. The current failure rate is 8.97% in the last 7 days and I'm hoping these changes decrease that 🏁
| @@ -53,11 +53,15 @@ export class StateStoreChaiTestRunner { | |||
| it('should detect multiple consumption', async () => { | |||
| const { stateStore } = this; | |||
| const installUrlOptions = { scopes: ['channels:read'] }; | |||
| for (let i = 0; i < 200; i++) { | |||
| for (let i = 0; i < 100; i++) { | |||
There was a problem hiding this comment.
is reducing the limit by 100 an arbitrary choice for testing or is there a reason behind it?
Co-authored-by: Eden Zimbelman <[email protected]>
Co-authored-by: Eden Zimbelman <[email protected]>
Summary
All operating systems mask the fact that storage devices are slow by caching reads and writes. When you write to a file, it doesn't immediately write to the actual storage medium; it'll capture it in a cache, tell your program that the write has completed, and go and write the contents to the storage device in the background instead.
It also seems like Node.js can close a file and then try to interact with it before all data has been written to disk. In the context of the test (
should close all files after writing them), the FileStateStore is asked to perform a burst of I/O disk writing followed by a read. This leads me to believe that a race condition is created where Node.js tries to interact with a file before the OS has completed data persistence to storage.This PR updates the behavior of the flaky unit test by
Code cov test results
Requirements (place an
xin each[ ])