Repository navigation
Ruthlessly mark tests that fail frequently as flaky #43955
Description
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.
Activity
mcollina commentedon Jul 23, 2022
cc @nodejs/tsc
tniessen commentedon Jul 23, 2022
Context: #43754 (comment) and #43929 (comment)
lpinca commentedon Jul 23, 2022
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 commentedon Jul 23, 2022
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 commentedon Jul 23, 2022
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 commentedon Jul 23, 2022
@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 commentedon Jul 24, 2022
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:
and the likely underlying bug
tniessen commentedon Jul 24, 2022
@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.
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 commentedon Jul 24, 2022
Note that close to 50% of our failures are of the obscure nature, either Jenkins or build-related issues.
bnoordhuis commentedon Jul 24, 2022
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 commentedon Jul 26, 2022
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.
[-]Ruthlessly mark tests that fails frequently as flaky[/-][+]Ruthlessly mark tests that fail frequently as flaky[/+]mcollina commentedon Aug 10, 2022
This can be closed, CI is mostly better now.