disable flaky unit tests [NET-5042]

[nfi-hashicorp]

Description

Postulate: flaky tests are net-negative value. Can we even really trust that their positive results are valid?

We should ideally fix flaky tests, obviously. But IME tests are flaky due to hard-to-reproduce timing bugs that only happen in CI.

What should the cutoff be to consider a test "flaky"? I arbitrarily decided 10%.

Dashboard

Abandoned Approaches

In #18424, I actually tried to fix these flaky tests by throwing some retries at them without really understanding the problem. Many are still flaky.

In https://github.com/hashicorp/consul/compare/retry-flaky (branched from a former commit of this branch), I tried to just wrap the test function body in a retry. That doesn't quite work for a few reasons, and would actually require more extensive edits.

PR Checklist

• updated test coverage
• external facing docs updated
• appropriate backport labels added
• not a security concern

[nfi-hashicorp]

Hm, I'm not sure this retry approach will work. Whereas gotestsum reruns the test by rerunning the binary, we're doing it with a function. The problem is t.Fail; we can't unmark a failed test. Maybe we do each rerun in a subtest?

[nfi-hashicorp]

In 846d383 (#18365), I attempted to contain the FailNow with a t.Run. That doesn't work because a FailNow causes the parent to fail, and we want the parent to pass if any of its children pass.

In 5772d44 (#18365), I attempted to avoid the retry lib to get tighter control over the defer/recover handling. Unfortunately I don't think it will work. testing.T#FailNow doesn't panic; it calls runtime.Goexit, which we cannot catch. So we'd need to pass the real test func some testing.T-compatible thing with a fake FailNow. retry.R does this. But we'd need to modify the tests (and called helpers) to work with that, or some interface. A non-trivial change vs just wrapping with gotestsum. :(

Given these issues, I'm going back to draft

[nfi-hashicorp]

I made an attempt to convert flaky tests to using retry.R as much as possible in 2f26b83, but it's not trivial. There are some instances where you have to think about when a retry.R is what you want vs the real testing.T. Some of these tests are kind of wacky, not sure what the intention is in some spots. By this point I probably could have just improved the tests to be less flaky. Probably not tenable to apply in a general way.

[nfi-hashicorp]

Back to disable only approach.

[github-actions]

This pull request has been automatically flagged for inactivity because it has not been acted upon in the last 60 days. It will be closed if no new activity occurs in the next 30 days. Please feel free to re-open to resurrect the change if you feel this has happened by mistake. Thank you for your contributions.

[github-actions]

Closing due to inactivity. If you feel this was a mistake or you wish to re-open at any time in the future, please leave a comment and it will be re-surfaced for the maintainers to review.