Merge ~racb/git-ubuntu:xfail-style into git-ubuntu:master

[Robie Basak]

Make Jenkins happy

[Bryce Harrington (bryce)]

I would only ask a minor change:

  "without its fix, then a good technique is to add the test first in an earlier commit,"

This does seem like a good style to recommend, and seems like it could make some reviews a lot easier to do. But I can imagine situations where it'd impose an extra hoop to jump through as a contributor, and may be most beneficial to git-ubuntu to express it more as a strong suggestion than requirement.

review: Needs Fixing

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:e198f3ae453abc977eb2ab25bd78d8e3da921447
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/473/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/473//rebuild

review: Approve (continuous-integration)

[Robie Basak (racb)]

Agreed. Done and force pushed. Here's the diff:

diff --git a/doc/STYLE.md b/doc/STYLE.md
index 373d3a1..36d9ee4 100644
--- a/doc/STYLE.md
+++ b/doc/STYLE.md
@@ -50,9 +50,9 @@
    this.

    If a bugfix comes with a test and the test can be arranged to work
- without its fix, then add the test first in an earlier commit,
- decorated with `@pytest.mark.xfail` or the `pytest.param`/`marks`
- equivalent.
+ without its fix, then a good technique is to add the test first in an
+ earlier commit, decorated with `@pytest.mark.xfail` or the
+ `pytest.param`/`marks` equivalent.

    Commits that cause previously fixed tests to pass (such as bugfix
    commits with a test added previously as above) should drop the xfail

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:2894c80181be3239908fd93d2919b46da0876b81
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/474/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/474//rebuild

review: Approve (continuous-integration)

[Robie Basak (racb)]

FTR, I adopted Bryce's suggestion and Bryce gave me a +1 for my revised branch in standup yesterday.

diff --git a/doc/STYLE.md b/doc/STYLE.md
index d7ee95b..36d9ee4 100644
--- a/doc/STYLE.md
+++ b/doc/STYLE.md
@@ -44,3 +44,24 @@
    brought into the namespace.
 
  * Keep imports sorted. This keeps diffs cleaner.
+
+ * Keep the test suite always passing and tests marked "xfail" always
+   failing. We use CI and `xfail_strict=true` in `pytest.ini` to enforce
+   this.
+
+   If a bugfix comes with a test and the test can be arranged to work
+   without its fix, then a good technique is to add the test first in an
+   earlier commit, decorated with `@pytest.mark.xfail` or the
+   `pytest.param`/`marks` equivalent.
+
+   Commits that cause previously fixed tests to pass (such as bugfix
+   commits with a test added previously as above) should drop the xfail
+   at the same time.
+
+   In other words, where possible we use `xfail` to keep the test suite
+   informed about known problems in the source tree at commit level
+   granularity.
+
+   This helps the reviewer easily check the correctness of any tests
+   being added, as well as their fixes. It also helps to keep CI "always
+   green", making future bisection and other code archaeology easier.