Flaky tests - consider auto-closing stale flaky test reports

[talldan]

What problem does this address?

It might be good to auto-close stale flaky test reports if the flakiness hasn't reoccured for a while.

At the moment this requires manual triage. I've had a quick look through the issues sorted by 'least recently updated' to find stale reports (and closed a bunch):
https://github.com/WordPress/gutenberg/issues?q=is%3Aopen+is%3Aissue+label%3A%22%5BType%5D+Flaky+Test%22+sort%3Aupdated-asc

The only caveat to this auto-close idea is that there are a few different reasons for a flaky test not to reoccur:

• The test was fixed (great, the issue should be closed)
• The test was renamed (that's ok, I think the issue can still be closed, if the test is still flaky a new issue will be opened)
• The test was skipped (this is a bit trickier, what should we do in this situation?)

What is your proposed solution?

A github action to auto-close stale flaky test issues on a regular basis.

Looking for ideas on how to handle skipped tests.

[kevin940726]

Interesting! I think it's a great idea!

Looking for ideas on how to handle skipped tests.

I think we can do some checking in the GH action to filter out skipped tests? Pseudo code below:

onDailyOrWeekly(async () => {
  const flakyTestReports = await fetchAllFlakyTestIssues();

  const staleFlakyTestReports = flakyTestReports
    .filter(flakyTestReport => isStale(flakyTestReport) && isNotSkipped(flakyTestReport));

  for (const staleFlakyTestReport of staleFlakyTestReports) {
    await closeIssue(staleFlakyTestReport);
  }
});

function isStale(flakyTestReport) {
  return Date.now() - flakyTestReport.lastUpdated >= 2; // weeks
}

function isNotSkipped(flakyTestReport) {
  // `--listTests` could be useful for this function: https://jestjs.io/docs/cli#--listtests
  const allTests = await findAllTests();
  return allTests.some(test => test.title === flakyTestReport.title);
}

[talldan]

Yeah, that's something I had in mind too. I was thinking we might have to grep the codebase, but thanks for pointing out the Jest documentation, that would be a lot better, I'll take a look.

It might be good to get the bot to label those skipped tests if we can detect them.

[priethor]

Yes, please! Great idea!

I've also been triaging some of the Flaky tests issues, and a significant number of them go unnoticed. If no action is taken in a predefined time, I think it should be closed as it is unlikely that it gets attention unless the test fails further down the road, in which case the issue can be reopened.

[talldan]

So Jest's listTests option only outputs the test files. I tested doing something like this:

jest --listTests --json --config=packages/e2e-tests/jest.config.js

Not very useful for finding stale tests.

The other idea I had is to use the test file path from the flaky test issue description and parse the contents of the file into an AST, and then walk the AST to find out whether a test is skipped. That would make it possible to capture wrapping describe.skip calls, but it's a bit of work, so I'm wondering how worth it it is.

[kevin940726]

I see.

I think it depends on how often we want to run this. If we're only going to run this biweekly, I guess we can just run the whole test suites even though it's going to take longer to finish.

npx jest --config=packages/e2e-tests/jest.config.js --json > tests.json 2> /dev/null

This command should output the --json result to tests.json so that we can parse it and find the tests. 2> /dev/null is just a unix command to silent stderr, which we don't need.

The result should have testResults[index].assertionResults[index].title pointing to the test title, and status in the same object should match the type here. (However, in my testing, skipped tests show pending instead of skipped though. Probably a bug of Jest or jest-circus.)

And/Or we could alter the default timeout to maybe like 1 millisecond to let tests fail early? Tricky 🤔 .