1

A rule of thumb with git is, don't make commits where the project doesn't compile or the tests don't pass. All the commits should build. Then, when you discover a problem that wasn't previously caught by the tests, you can use git bisect to discover where it was introduced quickly. If you are git bisecting for some particular problem, and you run into commits that don't build, or the tests fail for unrelated reasons, it can be really annoying and time consuming to figure out how to proceed with the bisect.

However, TDD says that when I do development work on a new feature, I should begin by writing tests for the new feature that don't pass yet. (Figure out what the feature is supposed to do, then go do it.)

Recently I had a situation where I developed new code for project A, which passed it's internal tests. However, before I merged the develop branch, I decided to test downstream dependencies B and C. The new version of A broke those builds, so I need to now go back and fix the A develop branch.

The first thing I want to do is, reduce the failing code from B and C to test cases which will get committed to A, and become my new tests for the next development iteration.

However, it may be relatively easy to create the new tests, but relatively hard to fix the new problem. I don't like leaving uncommitted work in my tree for a long time, especially if it is valuable stuff like new tests. For instance, what if I start working on a new machine, or share the progress with a collaborator. But if I commit the new tests, then I've created a bad commit where the build is broken.

Are there any good techniques or established practices for reconciling these concerns? Here are some things I thought of:

  1. When the new tests are ready, commit them as usual but make some kind of note in the commit message "note: new tests have intentionally broken the build". When git bisecting, if you reach such a commit, then immediately perform git checkout HEAD^, and test that commit instead to try to make progress in the bisect (Repeat if that commit also has such a note.) If the parent commit is bad, then git bisect won't take you back to the new tests commit, since it's looking for the first bad commit. If the parent commit is good, and git bisect takes you back to the new tests commit, then you should arbitrarily mark it as good, because the code didn't change there, only the new tests were added, so it can't logically be the first bad commit in regards to whatever problem you are actually testing.

  2. When the new tests are ready, commit them on a new-tests branch. Then work on the feature against develop branch. There may be multiple develop branches off of new-tests if there are multiple competing approaches. If the tests need to change, then change them and rebase the develop branches against new-tests. At the end when everything is passing, squash it all into one commit with the new tests and the working new features, completing the TDD cycle but also not making any dead commits that end up on master.

  3. The project should actually be building two test executables -- a "required" tests and a "desired" tests. In CI, both executables are built but the "desired" tests are allowed to fail without the build being marked as failed. When new tests are created they go in the "desired" target. When the feature is finished and the desired tests are passing, they get switched over to required. The only problem with this is, what if the new tests don't even compile yet. Should desired tests not even get compiled as part of CI? Is there even any point to have a "desired" tests target?

  • 2
    "All the commits should build". I couldn't disagree more. There are many times when committing as advantageous, even if your codebase doesn't build. It buys you freedom to experiment and change things around, with a really reliable "undo" mechanism. So long as you're not merging these broken commits into release branches, you're fine. – Alexander Jul 25 '17 at 22:35
  • 2
    All commits from specific branches need to build. Your personal feature branch can be as buggy and uncompilant as you want. But you're only allowed to merge it into master/develop/$whatever when it compiles sucessfully (see "Git Flow") – BlueWizard Jul 26 '17 at 8:40
  • 4
    "There are many times when committing as advantageous, even if your codebase doesn't build." no doubt, but I for myself use the --amend switch until the commit is "complete" or squash "incomplete" commits before rebasing... – Timothy Truckle Jul 26 '17 at 9:12
8

TDD says to write one failing test, then to make that one test pass, then optionally refactor, then repeat. That cycle usually takes no more than a few minutes, and you should have no need to commit in the middle of it. If you're writing all your tests, then writing all your code, that's not TDD, it's something much more difficult and less useful.

  • 2
    something much more difficult and less useful torture driven development? – enderland Jul 26 '17 at 2:35
  • 3
    Personally, my workflow is: red-commit-green-commit-refactor-commit-squash-repeat. Remember: in Git committing != publishing. – Jörg W Mittag Jul 27 '17 at 10:06
4

I think your premise is incorrect.

A rule of thumb with git is, don't make commits where the project doesn't compile or the tests don't pass.

I don't think this is true at all. You shouldn't be afraid to make commits in GIT and even push them to the remote for collaboration. You should however have a stable branch that should always build cleanly and pass all tests. For you this is the develop branch. You should not push broken tests to the develop branch.

So IMO #2 is the correct approach. Instead of naming the feature branch new-tests, name it something meaningful to the feature you've written tests for. You can push the tests to this branch, your colleague can contribute the implementation so that the tests pass. Once all tests pass and the branch is ready you can rebase and squash commits (if desired) on latest develop and fast-forward merge into develop.

  • "A rule of thumb with git is, don't make commits where the project doesn't compile or the tests don't pass. I don't think this is true at all." I for myself really find it helpful to fire the unittests after resolving conflicts when I rebase my feature upon the new tip of the develop branch... – Timothy Truckle Jul 26 '17 at 9:10
  • @TimothyTruckle: But do you also ensure that your feature branch is building (with green tests) at all times? Or do you find it acceptable to sometimes commit broken code to a feature branch when you know that the branch isn't ready to be merged? – Bart van Ingen Schenau Jul 26 '17 at 20:00
  • @BartvanIngenSchenau When doing TDD you feature branch is in red state (not compiling or test broken) typically for only a few second (less than 2 minutes in average). I don't have the need to check in in that short time. During the refactoring phase you can check in any time since the code base does not get broken (not compiling or test broken) by definition. And the way to achieve this is by using the IDEs automated refactorings. And actually I check in every single refactoring step since this makes rebasing (and bisect) easier. – Timothy Truckle Jul 27 '17 at 8:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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