Hacker News Reader

Comment by usr1106 | Hacker News Reader

Every commit should build is a reasonable principle.

However, it's in contradiction to TDD. If you have a bug (in master) the first thing you are supposed to do is to create a test that fails because of the bug. This is a self-contained commit, but it will not build (assuming your test are run during the build as they should). The next commit is fixing the bug and now it builds again.

So the commit with the new test should be marked as unsuitable for bisecting. How to do that easily in away that bisecting remains easy I have yet to work out.

View on HN

Isn't this taking TDD to literally? Verifying that a new test is indeed broken doesn't deserve a separate commit. The presence of that test in the codebase with all tests passing is enough. If your devs are writing new tests that are already passing your team needs a better lead, not more build controls.

In normal development flow sure. But if the bug made it already to master I'd prefer the separate commit so it can be easily verified that the problem is gone.

I've been enforcing every commit to both build and pass tests, but now I might only require each commit to build and each merge request to pass tests.

In that case, you can have a merge request with a commit to add a test and then another commit to fix it. As long as both commits build, bisect will still work.

I agree with having CI enforce both passing tests and working builds, but why would you want the build step to require passing tests?

That's just terminology. When I said "does not build" I meant CI fails because a test fails. Of course the code would compile or what other steps you might have.

Change the CI to test that every commit builds and every merge request passes tests. If we don't enforce the tests to pass for every commit, that solves the problem. Right?

This is basically the approach that I take. I expect git log --first-parent (and git bisect --first-parent) on the main branch to pass full CI builds, but I'm increasingly more lax about individual commits the further you get out into the DAG. I think it way more useful to record the steps-in-progress than to toss them (especially when you are trying to debug the why or the how of a change months or years later from say a junior developer; it's a lot easier sometimes to see things like "oh, this was a test value meant to be replaced" when you see exactly which intermediate commit it was from deep in a PR).

(ETA: It's easy to forget that the I in CI stands for "integration" and was always focused on merges of changes more than specific changes.)

One of the many reasons I'm not a fan of the "pure" model of TDD.

If you want to work like that though one way to do it is to work in a branch/PR - building out your commit that fails, then your iterative commits - and then squash the branch into a single commit onto main layer on.

If anyone want to see your TDD tiny commits process later they can go and see it in the PR.

I have never seen TDD state that a test should not be in the same commit as a feature.

Reply on news.ycombinator.com