16

I just changed branch settings on my GitHub repository, so that my [next] branch requires a passing CI build through a pull request.

A discussion followed with a number of team members, about failing tests.

For context's sake...

The repository has a [master] branch that's only PR'd into when there's a release, so [master] contains the code as of the last release, regardless of whether it's a major, a minor, a hotfix, a beta, alpha / pre-release build.

The [next] branch is the "default" one, where we intend to keep "release-ready" code; technically that branch could be PR'd into [master] any time, and released.

Individual forks have their own dev branches, and contributors PR to [next].

When I review a non-trivial PR, I'll merge the contributor's dev branch into my "review" branch, and if I see things I can fix quickly I'll commit/push changes and new (sometimes failing) tests, and PR back into the contributor's dev branch; when they merge my changes in, make the new failing tests pass, and then push, their PR synchronizes, and then I'll merge the PR into [next].

But this question isn't about passing tests, it's about the failing ones.

Failing tests document what needs to be fixed.

Known bugs should have tests written for, so that we know what's not working.

Technically the GitHub issues list (filtered for and/or labels) does that too. Is it a good practice to also have a bunch of failing tests to document bugs?

A failing build on [next] would mean that we're not release-ready... but then "being release-ready" is a bit like "being ready" to have kids - you're never quite ready for this, and something, somewhere (of variable importance) will inevitably go wrong with the release.

So we're only ever pushing passing tests to [next]. Where to push failing tests then? I mean, outside the PR/review process?

For example, a user reports a new bug in the issues list, and I'd like to write a failing test suite for it - so as to specify what needs to be done and where, which makes it easier for new contributors to pick up and eventually PR a fix.

Where should I be pushing these failing tests? Or is it even a good idea to push failing tests anywhere?

Improve this question
4
  • @PhilipKendall good point! we're still fine-tuning our process; I don't like how VS shoves "ignored" tests together with "inconclusive" tests - if one of the lower-level tests fails, we don't want half the tests failing, so we make them inconclusive when prerequisites aren't met; this makes tests report a failure for the right reasons, and "inconclusive" when they can't test what they were written for. We don't have a lot of those (anymore), so ignoring them could be an option... but then, is it a failing test if it's ignored?
    – Mathieu Guindon
    May 11, 2016 at 19:43
  • Why does part of the review process involve you writing any code? If you see a bug then you comment in the PR and, optionally, decline the PR.
    – James
    May 11, 2016 at 20:49
  • @James well, I like writing code.. besides as more contributors join, I'm doing more and more GitHub maintenance and PR (public relations) work than actual coding!
    – Mathieu Guindon
    May 11, 2016 at 20:50
  • see also: Should I intentionally break the build when a bug is found in production?
    – gnat
    Jul 9, 2016 at 12:42

2 Answers 2

Reset to default
12

What I'd do in this situation is to mark the failing tests as "ignored" - that way you still have the test so that you know what you need to fix in future, but you're not going to end up with broken builds.

If you also tag each test with the issue tracker reference for fixing the issue, that gives you an easy way to tie things together.

Improve this answer
4

Full disclosure: I am one of the participants in the discussion.

The repository's master branch is not it's master branch. The merge into master does not serve any "actual purpose" and that branch is not doing things a branch should do (namely move).
You're abusing this branch as a Tag to the latest release.

Instead of using a branch, use a Tag. When you wish to release, you make the necessary steps in a "Release-Branch", just like a topic branch. Then you merge that into [next] and put a Tag on it.

The role that [next] fulfils, is that of a master branch. Only release-ready code goes in there. Anything else would be a [develop] branch. A development branch contains work that is to be stabilized. This means: remove [master], repurpose [next] the way you already did and create another branch for "less stable" work.

Since the it's more the exception than the rule that there needs to be failing tests, that remind of outstanding bugs, it shouldn't be too much of a problem to create and destroy that less-stable branch as needed

Improve this answer
2
  • 1
    Having the latest release in [master] makes it easy to branch off the last release to issue a hotfix; then the hotfix can be PR'd into [next] and from there into every dev branch.. or am I missing something?
    – Mathieu Guindon
    May 11, 2016 at 21:49
  • 2
    You can just git checkout -b HotFix ReleaseTag (that is if I remember the branch creating checkout syntax correctly). This should create a HotFix branch off of the ReleaseTag
    – Vogel612
    May 11, 2016 at 21:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.