Ruthlessly mark tests that fail frequently as flaky

[mcollina]

The status of our CI is deteriorating to the point of being impossible to land anything.

Here is my proposal: declare bankruptcy and move all tests detected in https://github.com/nodejs/reliability as flaky.

[mcollina]

cc @nodejs/tsc

[tniessen]

Context: #43754 (comment) and #43929 (comment)

[lpinca]

To be honest I don't think this is a good idea. More time should be spent on fixing flaky tests before adding new features. If this is not done the number of flaky tests will only grow over time defeating the whole point of testing.

[devsnek]

Do we have any context on whether the CI run was intended to pass? Sometimes I will be running on battery, so instead of building/testing/linting on my laptop I will let the CI do it, which generally comes up red.

@lpinca I agree with you in principle but that hinges on getting actual humans to spend actual time fixing the tests. Do we have the assurance that this can be done?

[lpinca]

Do we have the assurance that this can be done?

I don't know but we should at least try. I only remember a handful of contributors who took the time to do this. I also think that we should help new contributors not to create flaky tests. For example, discouraging the use of timers unless strictly needed.

[tniessen]

More time should be spent on fixing flaky tests before adding new features. If this is not done the number of flaky tests will only grow over time defeating the whole point of testing.

@lpinca I agree with you. However, as you said these tests are flaky, so marking them as flaky seems like the right approach. If it is not then we should use different terms for tests that are flaky and tests that should be marked as flaky.

[lpinca]

However, as you said these tests are flaky

Sometimes flakiness is caused by real bugs. If we simply mark tests flaky with no investigation, we might hide real issues and make it harder to discover and fix bugs. Here are two recent examples:

• test: mark sequential/test-gc-http-client-onerror as flaky #43957
• test: mark test-gc-http-client-timeout as flaky on arm #43754

and the likely underlying bug

• http: fix http server connection list when close #43949

[tniessen]

Sometimes flakiness is caused by real bugs.

@lpinca I am not disagreeing, I'd even go as far as to say that almost all flaky tests are caused by bugs -- either in node, in the test itself, or in the test environment.

If we simply mark tests flaky with no investigation, we might hide real issues and make it harder to discover and fix bugs.

That's not the goal; we should treat the list of flaky tests as an urgent TODO list. My point is: flaky tests should not make contributing a worse experience for everyone.

[mcollina]

Note that close to 50% of our failures are of the obscure nature, either Jenkins or build-related issues.

[bnoordhuis]

Looking at the latest reliability report, nearly half are infrastructural failures; marking things flaky won't help there.

Some of the real flakes, like parallel/test-heapsnapshot-near-heap-limit-worker, are probably impossible to make reliable except in a statistical sense ("on average didn't fail more than 1 out of 10 times during the last 100 runs.")

Tests like that we could either disable/remove/mark flaky, or build out the CI to track swings in statistical flakiness. That's what e.g. Mozilla does for their test suites.

[mhdawson]

I'd agree that for tests we cannot make reliable we should remove them.

In the bigger picture I don't think we should just automatically mark failed tests as flaky but I am ok with us making people feel more comfortable about doing that when tests are flaky and time is needed to investigate.

I think we should also try something like "flaky fix days/week" or something like that. We tried some of those within RH and I think we helped make some progress. If we did it as a project I think we should make it a "only fix flaky tests day/week" ie nothing else should be tested/landed during that period of time.