mirror of
https://github.com/oxc-project/oxc.git
synced 2026-09-14 19:36:11 +08:00
leemr
9771ee151c
`test_suppressing_errors_update_the_file_when_errors_are_decreased` and `test_prunning_errors_update_the_file_when_errors_are_decreased` both point at `fixtures/suppression/with_arg_and_decreased_errors`. Every other `SuppressionTester` fixture has a single owner. Both of these run oxlint in that directory, which rewrites `oxlint-suppressions.json`, and both then delete it and restore it from the backup in `Drop`. It showed up as an intermittent red in `just ready` while I was working on something unrelated, and it cost enough re-runs that I went looking. I flagged it at the end of #25412. This is that fix. Measured with `cargo test -p oxlint suppress`, ten runs per setting, on an 18 core machine: ``` --test-threads failures 1 0/10 12 0/10 16 0/10 17 0/10 18 3/10 20 0/10 24 0/10 ``` 18 is `hw.ncpu` there, and it is what `cargo test` picks by default. 20 and 24 are more parallel and never failed, so this is not load and it is not machine speed. The two tests have to land in the same dispatch wave, which depends on the divisor. The `Test Linux` job runs `cargo test --all-features` and stays green, which I read as a different core count rather than anything about the runner. Three distinct failure modes came out of the one shared directory: ``` tester.rs:249 one test's opening assert runs while the other sits between remove_file and fs::copy tester.rs:295 both Drop calls delete the same path, and the second gets ENOENT tester.rs:274 one test reads the file while the other is inside a truncating write ``` Before settling on the fixture split I checked whether repairing `Drop` would be enough, since deleting before copying is what opens the widest window. It is not enough. Guarding the delete with `&& !self.have_backup_file`, so the file is never absent, left the rate where it was and only changed the message: ``` Drop shape directory failures delete then copy, as today shared 3/20 copy only shared 4/40 unchanged separate 0/40 ``` To find out what the reader was actually getting in that third mode, I patched the panic at `tester.rs:274` to print the observed content. That patch was throwaway and is not in this diff: ``` PROBE observed len=0 serde=EOF while parsing a value at line 1 column 0 first80="" ``` Length zero, not a torn file. `SuppressionFile::save` uses `fs::write`, which is `File::create` plus `write_all`, and `File::create` opens with `O_TRUNC`. The truncate commits at open time and the bytes arrive on the next syscall, so the file is briefly empty. A separate directory is the only change that closes both that window and the `Drop` one. Calling out one thing up front: the new directory is a byte for byte copy of the old one. `--suppress-all` and `--prune-suppressions` converge in the decreased case, so the expected content is the same for both tests. The directory exists for isolation, not for content. `with_arg_and_increased_errors` and `with_arg_and_increased_errors_prune` are the same split for the case where the two commands diverge, so five of six files match there and six of six match here. This is the same shape as #20717, where `--init` writes leaked across parallel tests. With the change in, the suppression module is 0/20 at 18 threads and 0/10 at the default. The rest of `just ready` is green as well: `typos`, `cargo lintgen`, `just fmt`, `just check`, `just test`, `just lint`, `just doc` and `just ast`, with 4446 tests passing under `--test-threads=1`. Three other suppression fixtures have more than one owner, and I left all three alone. `fixed_violations_are_reported` has two readers. `type_check_only_with_regular_rule` also has two, and although the second passes `--suppress-all` and `--prune-suppressions`, `lint.rs:426` rejects that combination before any lint runs, so nothing writes. `diagnostics_filtered_if_count_is_the_same` is the only other one where a write really happens, so it carries the truncating write window in principle, but that window is a single `fs::write` over a few hundred bytes, where the one this PR fixes spanned a whole `Drop` delete-then-copy plus the other test's entire run. Forty runs with those two tests forced to overlap found nothing. I did not want to change something I cannot show is broken. Happy to fold it in if the whole class should close in one go. Disclosure, per CONTRIBUTING.md: I used Claude for the investigation and for the patch. I have reviewed and tested all of it myself.