Re: [PATCH/RFC v2 06/16] t3700: sleep for 1 second, to avoid interfering with the racy code

Junio C Hamano <gitster@pobox.com> writes:

OK, after staring this test for a long time, and going back to 3d1f148 (refresh_index: do not show unmerged path that is outside pathspec, 2012-02-17), I give up.

Let me ask the same question in a more direct way. Which part of this test break with your series?

        test_expect_success 'git add --refresh with pathspec' '
                git reset --hard &&
                echo >foo && echo >bar && echo >baz &&
                git add foo bar baz && H=$(git rev-parse :foo) && git rm -f foo &&
                echo "100644 $H 3	foo" | git update-index --index-info &&
	# "sleep 1 &&" in the update here ...
                test-chmtime -60 bar baz &&
                >expect &&
                git add --refresh bar >actual &&
                test_cmp expect actual &&

                git diff-files --name-only >actual &&
                ! grep bar actual&&
                grep baz actual
        '

We prepare an index with bunch of paths, we make "foo" unmerged, we smudge bar and baz stat-dirty, so that "diff-files" would report them, even though their contents match what is recorded in the index.

Then we say "git add --refresh bar". As far as I know, the output from "git add --refresh <pathspec>" is limited to "foo: needs merge" if and only if "foo" is covered by <pathspec> and "foo" is unmerged.

	Side note: If "--verbose" is given to the same command, we
	also give "Unstaged changes after refreshing the index:"
	followed by "M foo" or "U foo" if "foo" does not match the
	index but not unmerged, or if "foo" is unmerged, again if
	and only if "foo" is covered by <pathspec>.  But that is not
	how we invoke "git add --refresh" in this test.

So if you are getting a test failure from the test_cmp, wouldn't it mean that your series broke what 3d1f148 did (namely, make sure we report only on paths that are covered by <pathspec>, in this case "bar"), as the contents of "bar" in the working tree matches what is recorded in the index?

If the failure you are seeing is that "bar" appears in the output of "git diff-files --name-only", it means that "diff-files" noticed that "bar" is stat-dirty after "git add --refresh bar". Wouldn't it mean that the series broke "git add --refresh bar" in such a way that it does not to refresh what it was told to refresh?

Another test that could fail after the point you added "sleep 1" is that the output from "git diff-files --name-only" fails to list "baz" in its output, but with "test-chmtime -60 bar baz", we made sure that "bar" and "baz" are stat-dirty, and we only refreshed "bar" and not "baz". If that is the case, then would it mean that the series broke "git add --refresh bar" in such a way that it refreshes something other than what it was told to refresh?

In any case, having to change this test in any way smells like there is some breakage in the series; it is not immediately obvious to me that the current test is checking anything wrong as I suspected in the earlier message.

So,... I dunno.