Stream: t-compiler/rust-analyzer

Topic: test marks flakey?


Josh Mcguigan (Mar 15 2021 at 13:49, on Zulip):

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.

Laurențiu (Mar 15 2021 at 13:51, on Zulip):

Not AFAIK

matklad (Mar 15 2021 at 13:52, on Zulip):

Marks might falsely pass, but they should never falsely fail

matklad (Mar 15 2021 at 13:53, on Zulip):

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'

matklad (Mar 15 2021 at 13:54, on Zulip):

cargo test --package ide_completion --lib -- context::tests::expected_type_if_let_without_leading_char --exact --nocapture fails for me locally reliably

matklad (Mar 15 2021 at 13:54, on Zulip):

so yeah, you just got lucky on CI :)

(

matklad (Mar 15 2021 at 13:55, on Zulip):

/me thinks if we should enable precise thread-local marks?

matklad (Mar 15 2021 at 13:59, on Zulip):

Yeah, let's try to activate this feature in all test-mark using crates. If it works, then let's roll with it.

Laurențiu (Mar 15 2021 at 14:04, on Zulip):

Right, what do we do with that mark?

Josh Mcguigan (Mar 15 2021 at 14:14, on Zulip):

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).

Laurențiu (Mar 15 2021 at 14:19, on Zulip):

Is that even with https://github.com/rust-analyzer/rust-analyzer/pull/8029?

Josh Mcguigan (Mar 15 2021 at 14:29, on Zulip):

No that is master. I'm looking at your PR now.

Josh Mcguigan (Mar 15 2021 at 14:41, on Zulip):

I've confirmed it consistently fails on my machine with the thread-local feature. Thanks for the quick help resolving this!

matklad (Mar 15 2021 at 14:51, on Zulip):

@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

Josh Mcguigan (Mar 15 2021 at 14:56, on Zulip):

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?

matklad (Mar 15 2021 at 14:56, on Zulip):

right

Last update: Jul 29 2021 at 08:00UTC