Are test marks known to ever be flakey? I'm pretty sure I have seen false failures (at least once) for mark not hit when running the tests locally, and I just saw it in CI - https://github.com/rust-analyzer/rust-analyzer/pull/8027. Re-running caused the CI tests to pass. Specifically it is the
expected_type_if_let_without_leading_char mark. I don't think there is anything in that test that is non-deterministic.
Marks might falsely pass, but they should never falsely fail
That being said, we've recently switched from an in-tree impl to one from crates.io. they should be equvalent, but maybe they arent'
cargo test --package ide_completion --lib -- context::tests::expected_type_if_let_without_leading_char --exact --nocapture fails for me locally reliably
so yeah, you just got lucky on CI :)
/me thinks if we should enable precise thread-local marks?
Yeah, let's try to activate this feature in all test-mark using crates. If it works, then let's roll with it.
Right, what do we do with that mark?
So it does fail locally for me too when running that test in isolation. But
cargo test -p ide_completion is flakey, mostly passing (although I have seen one failure in a handful of runs).
Is that even with https://github.com/rust-analyzer/rust-analyzer/pull/8029?
No that is master. I'm looking at your PR now.
I've confirmed it consistently fails on my machine with the
thread-local feature. Thanks for the quick help resolving this!
@Josh Mcguigan so the way mark work is that each mark is an atomic counter, incremented by each
hit. The check then looks at the marks at the start and end of the scope, and makes sure that there was at least one increment. So, if the mark is incremented by an unrelated test, that would get counted as a hit.
This is architectured to allow spawning threads from within the test, and get them pass. There's a thread local opt-out. If all our tests actually work with thread-local, I am tempted to release a 2.0 version, which makes thread-local based impl default, and uses a feature to opt-into non-thread-safetyz
Good to know! Just to make sure I am understanding, does this mean that when we enable
thread-local, if we had a test that spawned a thread and that thread hit the mark we'd get a false-fail?