Re: [PATCH] t4069: test diff behavior with i-t-a paths

[Eric Sunshine <sunshine@sunshineco.com>]

On Mon, Aug 10, 2020 at 4:54 AM Raymond E. Pasco <ray@ameretat.dev> wrote:
> Add a small test suite to test the behavior of diff with intent-to-add
> paths. Specifically, the diff between an i-t-a entry and a file in the
> worktree should be a "new file" diff, and the diff between an i-t-a
> entry and no file in the worktree should be a "deleted file" diff.
> However, if --ita-visible-in-index is passed, the former should instead
> be a diff from the empty blob.
>
> Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>
> ---
> diff --git a/t/t4069-diff-intent-to-add.sh b/t/t4069-diff-intent-to-add.sh
> @@ -0,0 +1,30 @@
> +test_expect_success 'diff between i-t-a and file should be new file' '
> +       cat blueprint >test-file &&
> +       git add -N test-file &&
> +       git diff >output &&
> +       grep "new file mode 100644" output
> +'

If someone comes along and inserts new tests above this one and those
new tests make their own changes to the index or worktree, how can
this test be sure that the "new file mode" line is about 'test-file'
rather than some other entry? It might be better to tighten this test,
perhaps like this:

    git diff -- test-file &&

Same comment applies to the other tests.

> +test_expect_success 'diff between i-t-a and no file should be deletion' '
> +       rm -f test-file &&
> +       git diff >output &&
> +       grep "deleted file mode 100644" output
> +'
> +
> +test_expect_success '--ita-visible-in-index diff should be from empty blob' '
> +       cat blueprint >test-file &&
> +       git diff --ita-visible-in-index >output &&
> +       grep "index e69de29" output
> +'

The hard-coded SHA-1 value in the "index" line is going to cause the
test to fail when the test suite is configured to run with SHA-256.
You could fix it by preparing two hash values -- one for SHA-1 and one
for SHA-256 -- and then looking up the value with test_oid() for use
with grep. On the other hand, if you're not interested in the exact
value, but care only that _some_ hash value is present, then you could
just grep for a hex-string.

But what is this test actually checking? In my experiments, this grep
expression will also successfully match the output from the test
preceding this one, which means that the conditions of this test are
too loose.

To tighten this test, perhaps it makes sense to take a different
approach and check the exact output rather than merely grepping for a
particular string. In other words, something like this might be better
(typed in email, so untested):

    cat >expect <<-\EOF &&
    diff --git a/test-file b/test-file
    index HEX..HEX HEX
    --- a/test-file
    +++ b/test-file
    EOF
    cat blueprint >test-file &&
    git diff --ita-visible-in-index -- test-file >raw &&
    sed "s/[0-9a-f][0-9a-f]*/HEX/g' raw >actual &&
    test_cmp expect actual

In fact, this likely would be a good model to use for all the tests.