Re: [lmi] [lmi-commits] master 6c0c0f0 3/4: Add numerous failing unit tests

[Vadim Zeitlin]

On Sun, 18 Mar 2018 20:02:19 -0400 (EDT) Greg Chicares <address@hidden> wrote:

GC> branch: master
GC> commit 6c0c0f04ad4f374710aaa5d46fc204f1acd40b4d
GC> Author: Gregory W. Chicares <address@hidden>
GC> Commit: Gregory W. Chicares <address@hidden>
GC> 
GC>     Add numerous failing unit tests

 Hello,

 I understand the rationale for doing it like this, but I'd just like to
mention that such commits risk making future use of (very useful) "git
bisect" more difficult than necessary because it's common to run "make
unit_tests" as automatic bisection command to find out when unit tests
started failing (of course, this is never supposed to happen in the first
place, but suppose that this is done under a different and previously
untested platform, or maybe with a different compiler or just a newer gcc
version) and such commits would result in false positives.

 Personally I sacrifice the atomicity of commits and put both the fix and
the new test in the same commit to prevent this from happening. It's not as
illustrative, even though I do usually mention that the new tests would
have failed before, but I don't see any real advantage in keeping the
(failing) tests in their own commit. If there is some, the order of commits
could be reversed, so only the passing tests are added, but I realize that
this would be somewhat unnatural.

 An alternative solution would be to create a branch with these changes and
then merge it into master (without using fast-forward merges), as this
would still keep the invariant "unit tests always pass in master" unbroken,
even if it was temporarily broken in the branch. But considering your
preference of linear history, I don't think this would be acceptable.

 Unfortunately I can't see any other approach, which brings me back to
recommending just committing the failing test and the fix for it as a
single commit.

 Regards,
VZ