On Sat, Aug 13, 2016 at 06:50:19PM +0200, Johannes Sixt wrote: > Am 12.08.2016 um 18:51 schrieb tboegi@xxxxxx: > >From: Torsten Bögershausen <tboegi@xxxxxx> > > > >When a non-reversible CRLF conversion is done in "git add", > >a warning is printed on stderr (or Git dies, depending on checksafe) > > > >The function commit_chk_wrnNNO() in t0027 was written to test this, > >but did the wrong thing: Instead of looking at the warning > >from "git add", it looked at the warning from "git commit". > > > >This is racy because "git commit" may not have to do CRLF conversion > >at all if it can use the sha1 value from the index (which depends on > >whether "add" and "commit" run in a single second). > > > >Correct this and replace the commit for each and every file with a commit > >of all files in one go. > > The new test code does not only fix the race condition, but also > tests different things, or prepares test cases in a different > manner. I would have appreciated an explanation why this is > necessary. Is it "on my machine, the race condition was triggered > consistently for a bunch of tests, and so I recorded the wrong > expected output in the test cases"? > Good point. The origanal intention was to let t0027 pass, and fix the bug in convert.c in a later commit. (and rename NNO in a 3rd commit, that is postponed until we figured this out). Looking at t0027, it turns out that the changes in the test matrix done in 1/2 are reverted in 2/2. This is not a good thing. Either (a) they should be marked as test_expect_failure in 1/2 and corrected in 2/2, or (b) 1/2 and 2/2 are squashed together and the noise in t0027 is minimal. I'll send a patch for (b) in a second. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html