Possibly fix flaky test seen to error in wasm2js3.test_stat_chmod_wasmfs. #24989
Conversation
There was a problem hiding this comment.
It seems likely this is a timing issue since its rely on sleep(1) and assumes the file created/touched before after the sleep will have different timestamps.
I had thought we had a bug open about this test already but I don't see one.
Perhaps you could open a bug and mark this test with @flaky(...bug url...).. that way it should get re-tried automatically in CI (see EMTEST_RETRY_FLAKY, you probably want to set that to a non-zero value in your CI too).
|
I found the existing issue : #24905 |
|
Could be, hence the "possibly fix." Nevertheless, long long shouldn't be narrowed to int, so this change is correct. If there's a filesystem that actually has 64-bit timestamps, then the code would be incorrect there. After PR #17401 this test wasn't adapted accordingly. |
Separate PR. |
| if nodefs and self.get_setting('WASMFS'): | ||
| self.skipTest('test requires symlink creation which currently missing from wasmfs+noderawfs') | ||
| self.do_runf('stat/test_chmod.c', 'success') | ||
| self.do_runf('stat/test_chmod.c', 'success', cflags=['-Werror=conversion']) |
There was a problem hiding this comment.
Makes me wonder if we should make an effort to enable this in whole test suite.
We currently enable -Werror by default:
Line 1202 in 4cab039
But we don't enable -Wall (which looks like it doesn't actually include -Wconversion anyway )
There was a problem hiding this comment.
I was expecting you might suggest that, and did give it a quick test before this PR, and was met with a million double -> float narrowing warnings with most tests failing miserably. It would be a nice idea, something to work on later.
I also searched the repo for other uses of .st_mtime and this was the only occurrence that assigned that to an int, other places have been converted already before.
There was a problem hiding this comment.
Yeah, no worries, I think we are long way from -Wall or -Wextra by default in the test suite !
2 participants
time_tislong long, test has a narrowing conversion toint.Fails nondeterministically in http://clbri.com:8010/api/v2/logs/20165/raw_inline