Diminishing tolerance for regressions

[totten]

Historically, we had a high tolerance for test-failures — it was, to some extent, an operational necessity when the effective feedback-cylce for tests was often days or weeks. Of course, it came with a cost: we accumulated debt over the course of the release-cycle, and at the end of the release-cycle we had to pay back that debt in a weeks-long test-fixing marathon.

In the past two months, we enabled PR-testing and worked through a transitional period. The codebase had a backlog of regressions, and that meant we had to remain tolerant of failures. For example, PR’s were often marked bad due to pre-existing failures, and we had to make an educated guess about whether to blame the PR.

We cleared through that debt in the unit-tests first, and a lot of hardwork has recently gone into clearing the debt for the WebTest suite — producing a significant (but-not-yet-complete) improvement. I believe this means we’re approaching a pivot point where our attitude toward test-failures should shift from tolerant to stringent.

In making that switch, it may be helpful to use a couple rules-of-thumb:

• Tests are ethical. Broken tests are a burden on others. When one ignores a test-failure, there are only two possible outcomes: either the cost of that failure transfers to someone else, or the test-suite falls into disrepair and we lose the investment (or both). Usually, the cost of the test-failure will amplify when it’s transferred to someone else -- and that "someone else" won'tbe expecting it.
• Tests are evidence. In complex software, it’s very hard to know that a change works and doesn’t break anything else. If we were disinterested third-parties (immune to day-to-day pressures), we’d look at every line of a change and say, “I don’t know if that works as expected for everyone. Where’s the proof or disproof?” We can’t go so far as to say that a test is a proper proof of correctness, but it’s certainly the only objective evidence of correctness we can produce easily.

What would a stringent, low-tolerance approach to test-failures look like?

• Don't merge a PR before the tests complete. (You might be able to get away with premature merges for JPGs or PNGs, but for edge cases like XMLs, it's easy to misjudge.)
• If a PR reports as failed, then always investigate. Check the PHPUnit results. Check the console output. Compare against the most recent general test-run (CiviCRM-Core-Matrix.)
• If something bad is merged inappropriately, revert it as soon as the mistake is discovered. The failure will sow confusion about the status of everyone's code.
• If it's too late to revert, then make sure to reopen, escalate to critical, and send annoying messages to anyone who wrote or approved the breakage.

[Coleman Watts]

I wonder if there's anything we can do with the GitHub UI to support this? Like disabling the merge button when running/failing tests? (would still be possible to merge on the command line of course). A quick google search suggests "no" but we can change the color to be a bit more unfriendly (like bright red). But maybe others know of a better solution?

[totten]

Well, if one *really* wanted to that, one could get the same effect by basically revoking everyone's merge privileges and reimplementing the merge functionality (with a mix of custom-code, third-party servers, bots), but that's too much work. For the moment, I think we should develop discipline.

[Eileen]

Do you think we will be at the point soon where we are testing PRs against webtests as well as unit tests?

[Coleman Watts]

@eileen I hope so. But first we need to get the webtests at a consistent 100% (which they haven't been at for many years). I've been following up on hunches about why some of them are failing sporadically and have had some success but still have > 40 failures to triage.

[Eileen]

Well I'm sure your hard work on that will be worth it.