Re: [lmi] Failing tests and Git (was: [lmi-commits] master 6c0c0f0 3/4: Add numerous failing unit tests)

[Vadim Zeitlin]

On Mon, 19 Mar 2018 13:47:51 +0000 Greg Chicares <address@hidden> wrote:

GC> What about the following idea? I haven't tested it yet, but I think
GC> it should keep 'master' linear and atomic, while branches could be
GC> checked out separately at any time.

 Yes, this would work.

GC> 
---------8<--------8<--------8<--------8<--------8<--------8<--------8<-------
GC>   # Commit to a digressionary branch...
GC> 
GC>   # See:
GC>   #   https://lists.nongnu.org/archive/html/lmi/2018-03/msg00082.html
GC>   # Use consecutive branch names 'q[0-9]\{5}', i.e., q00000-q99999
GC>   # Rationale:
GC>   #   - in English words, initial 'q' is almost always followed by 'u'
GC>   #   - 10000 such names should be enough for lmi's lifetime

 I can't say I like the idea of using intentionally cryptic names for the
branches though, what possible purpose could this serve? I don't really
understand what purpose does this "qNNNN" have at all, but if we need to
use it, could it perhaps be followed by some meaningful part, e.g.
"q00001-failing-floor-tests"? I still think that "test/failing-floor" would
be an even better name for this branch, however...

GC> >  An alternative solution would be to create a branch with these changes 
and
GC> > then merge it into master (without using fast-forward merges), as this
GC> 
GC> why avoid fast-forward merges in the failing-unit-test case,
GC> especially if a failing test's commit is immediately followed
GC> by a code change to make the test pass?

 Not avoiding fast-forward would result in exactly the same situation that
we have now, i.e.

        M <--- Fail <---- Fix

where "M" is the master commit before these changes, "Fail" is the commit
adding the failing tests and "Fix" is the commit fixing them; so creating a
branch would have been useless.

GC> AIUI, git would do a fast-forward merge anyway, because it knows it can
GC> in that case.

 Yes, but --no-ff "git merge" option can be used to prevent this from
happening. In this case, the situation would be

        M <------------------------ Merge
        ^                             |
        \                            /
         ----- Fail <---- Fix <------

and the tests would still pass for all master commits, which is a
potentially nice to have invariant.


GC> I don't understand these git concepts well enough to know whether
GC> cherry-picking is orthogonal to fast-forwarding.

 I have some trouble answering the question in this form too, but I think
it is, in the sense that there is normally no need to cherry-pick if you
can fast-forward. Cherry-picking is an act of taking some commit(s) from
another branch and grafting it to the current one, which is only used when
a merge is undesirable, while fast-forwarding is a (trivial) form of
merging. Notice that in normal Git use, merging is by far the preferred
operation because, unlike cherry-pick, it keeps the history of changes and
so, for example, would do the right thing if you merge multiple times.

GC> I've just fallen into the habit of cherry-picking instead of merging
GC> because it seems more "familiar".

 It is in the sense that it isn't really a Git (or DVCS) operation at all,
it's just like applying a patch. So it's indeed simple (and Git nice
implementation of it makes it even simpler to use), but normally it should
be used sparingly. As long as you use it only to cherry-pick from a branch
in my repository that is going to be destroyed afterwards anyhow, there is
no real problem with using it, but if we ever have any long-lived branches,
cherry-picking could become problematic.

GC> I guess cherry-picking amounts to merging a single commit

 At the work tree level, it does. But at the repository history level, it's
quite different because a merge is recorded in this history, while a
cherry-pick is not.


GC> At any rate, what I'm hoping to achieve is
GC> 
GC>     master  branches
GC>     ------
GC>    commit1
GC>    commit2
GC>             q00000 failing test
GC>    commit3
GC>      = q00000 + change to make test pass, combined atomically

 You can do exactly this by using either "git cherry-pick -n q00000" or
"git merge --no-commit q00000" and then adding the changes fixing the
problem and committing everything together. Or you could also make
"commit3" with just these changes and then do a (real, i.e. not
fast-forward) "git merge" to obtain

        C2 <----- C3 <--- Merge
         ^                  |
         \                 /
          ----- q0000 <----

 Normally I'd say that the latter is preferable as you still have the
separation between the commit adding the failing test and the fix but,
again, in this particular case (and all similar ones) I don't really see
anything wrong with sacrificing commit atomicity for simplicity.

GC>    commit4
GC>    commit5
GC>             q00001 store an interesting dead twig off the trunk
GC>    commit6
GC>      [resume main line of development, forgetting q00001]

 This, of course, is just fine (except for the branch name, that I'd still
like to be more meaningful).

 Regards,
VZ