CiviCRM Community Forums (archive)

*

News:

Have a question about CiviCRM?
Get it answered quickly at the new
CiviCRM Stack Exchange Q+A site

This forum was archived on 25 November 2017. Learn more.
How to get involved.
What to do if you think you've found a bug.



  • CiviCRM Community Forums (archive) »
  • Old sections (read-only, deprecated) »
  • Developer Discussion (Moderator: Donald Lobo) »
  • Diminishing tolerance for regressions
Pages: [1]

Author Topic: Diminishing tolerance for regressions  (Read 572 times)

totten

  • Administrator
  • Ask me questions
  • *****
  • Posts: 695
  • Karma: 64
Diminishing tolerance for regressions
September 19, 2014, 05:05:22 pm
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

  • Administrator
  • I’m (like) Lobo ;)
  • *****
  • Posts: 2346
  • Karma: 183
  • CiviCRM version: The Bleeding Edge
  • CMS version: Various
Re: Diminishing tolerance for regressions
September 19, 2014, 05:31:37 pm
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?
Try asking your question on the new CiviCRM help site.

totten

  • Administrator
  • Ask me questions
  • *****
  • Posts: 695
  • Karma: 64
Re: Diminishing tolerance for regressions
September 20, 2014, 03:21:30 pm
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

  • Forum Godess / God
  • I’m (like) Lobo ;)
  • *****
  • Posts: 4195
  • Karma: 218
    • Fuzion
Re: Diminishing tolerance for regressions
September 21, 2014, 02:51:00 pm
Do you think we will be at the point soon where we are testing PRs against webtests as well as unit tests?
Make today the day you step up to support CiviCRM and all the amazing organisations that are using it to improve our world - http://civicrm.org/contribute

Coleman Watts

  • Administrator
  • I’m (like) Lobo ;)
  • *****
  • Posts: 2346
  • Karma: 183
  • CiviCRM version: The Bleeding Edge
  • CMS version: Various
Re: Diminishing tolerance for regressions
September 21, 2014, 03:36:37 pm
@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.
Try asking your question on the new CiviCRM help site.

Eileen

  • Forum Godess / God
  • I’m (like) Lobo ;)
  • *****
  • Posts: 4195
  • Karma: 218
    • Fuzion
Re: Diminishing tolerance for regressions
September 21, 2014, 03:48:55 pm
Well I'm sure your hard work on that will be worth it.
Make today the day you step up to support CiviCRM and all the amazing organisations that are using it to improve our world - http://civicrm.org/contribute

Pages: [1]
  • CiviCRM Community Forums (archive) »
  • Old sections (read-only, deprecated) »
  • Developer Discussion (Moderator: Donald Lobo) »
  • Diminishing tolerance for regressions

This forum was archived on 2017-11-26.