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

[Junio C Hamano <gitster@pobox.com>]

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

> So whether done with "sleep" or "test-chmtime", avoiding a racily
> clean situation sounds like sweeping a bug in the v5 code in racy
> situation under the rug to me (unless I am misunderstanding what
> you are doing with this change and in your explanation, or the test
> was checking a wrong thing, that is).
>
> Even more confused....

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.