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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: "Junio C Hamano" <gitster@xxxxxxxxx>
Ann T Ropea <bedhanger@xxxxxx> 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]


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.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux