Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values
- From
Philip Oakley <philipoakley@iee.org>
- Date
- Nov 20, 2017, 17:26 UTC
- Message-ID
- <5AE7AD53CF184A27BF8F484D415083D9@PhilipOakley>
- In-Reply-To
- <xmqqzi7hlhkx.fsf@gitster.mtv.corp.google.com>
From: "Junio C Hamano" <gitster@pobox.com>
Show 5 quoted lines
> Ann T Ropea <bedhanger@gmx.de> writes: > >> *1* We are being overly generous in t4013-diff-various.sh because we do >> not want to destroy/take apart the here-document. Given that all this a >> temporary measure, we should get away with it.
So, the need to reformat the test for the future post-deprecation period is being deferred to the time that the PRINT_SHA1_ELLIPSIS env variable, and all ellipis, is removed - is that the case? Maybe it just needs saying plainly.
Or is the env variable being retained as a fallback 'forever'? I'm half guessing that it may tend toward the latter as it's an easier backward compatibility decision.
[apologioes this is mid thread, I'm catching up on 2 weeks of emails]
Show 64 quoted lines
>
> I do not think the patch is being particularly generous. If
> anything, it is being unnecessarily sloppy by not adding new checks
> to verify the updated behaviour.
>
> The above comment mentions "destroy/take apart" the here-document,
> but I do see no need to destroy anything. All you need to do is to
> enhance and extend. For example, you could do it like so (this is
> written in my e-mail client, and not an output of diff, so the
> indentation etc. may be all off, but should be sufficient to
> illustrate the idea):
>
> while read cmd
> do
> case "$cmd" in
> '' | '#'*) continue ;;
> esac
> test=$(echo "$cmd" | sed -e 's|[/ ][/ ]*|_|g')
> pfx=$(printf "%04d" $test_count)
> expect="$TEST_DIRECTORY/t4013/diff.$test"
> actual="$pfx-diff.$test"
> + case "$cmd" in
> + X*) cmd=${cmd#X}; no_ellipses=" (no ellipses)" ;;
> + *) no_ellipses= ;;
> + esac
> - test_expect_success "git $cmd" '
> + test_expect_success "git $cmd$no_ellipses" '
> {
> echo "\$ git $cmd"
> - git $cmd |
> + if test -n "$no_ellipses"
> + then
> + git $cmd
> + else
> + PRINT_SHA1_ELLIPSES=yes git $cmd
> + fi |
> sed -e ....
> ...
> done <<\EOF
> diff-tree initial
> diff-tree -r initial
> diff-tree -r --abbrev initial
> diff-tree -r --abbrev=4 initial
> +Xdiff-tree -r --abbrev=4 initial
> ...
> EOF
>
> There is a new and duplicated line with a prefix X for one existing
> test in the above. The idea is that the ones marked as such will
> test and verify the effect of this new behaviour by not setting the
> environment variable. The expected and actual test output for the
> new test will have X prefixed to it. t4013 is arranged in such a
> way that it is easy to add a new test like this---you only need to
> add an expected output in a new file in t/t4013/. directory. And
> the output with these ellipses removed will be something we would
> expect see in the new world (without the escape hatch environment
> variable), we would need to add a new file there to record what the
> expected output from the command is.
>
> I singled out the diff-tree invocation with --abbrev=4 as an example
> in the above, but in a more thorough final version, we'd need to
> cover both "abbreviation with ellipses" and "abbreviation without
> ellipses" output for other lines in the test case listed in the
> here-document.