Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 20, 2017, 04:06 UTC
Message-ID: <xmqqzi7hlhkx.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171119184113.16630-5-bedhanger@gmx.de>

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.

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.