Re: [PATCH 6/6] t4124: fix test for non-compliance diff
- From
Jeff King <peff@peff.net>
- Date
- Mar 19, 2020, 16:33 UTC
- Message-ID
- <20200319163334.GG3513282@coredump.intra.peff.net>
- In-Reply-To
- <285c6830c5182cb602d4fe559525083f69a158e9.1584625896.git.congdanhqx@gmail.com>
On Thu, Mar 19, 2020 at 09:00:07PM +0700, Đoàn Trần Công Danh wrote:
> POSIX's diff(1) requires output in normal diff format. > However, busybox's diff's output is written in unified format.
That's a pretty big difference. I'm surprised this only produces one problem in the test scripts. ;)
Show 7 quoted lines
> POSIX requires no option for normal-diff format. > > A hint in test-lib-functions::test_cmp said `diff -u` isn't available > everywhere. > > Workaround this problem by assuming `diff(1)` output is unified > if we couldn't make anything from normal-diff format.
I wonder if we could use "git diff" here. We have to be careful about circular reasoning in our tests (i.e., making sure we're not verifying output with the same code that we're testing), but I think here we're checking how "apply --whitespace=fix" works.
But if this is the only spot, then adjusting to handle unified or normal diff isn't too bad.
Show 14 quoted lines
> diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
> index 971a5a7512..2a54ce96b5 100755
> --- a/t/t4124-apply-ws-rule.sh
> +++ b/t/t4124-apply-ws-rule.sh
> @@ -52,6 +52,12 @@ test_fix () {
>
> # find touched lines
> $DIFF file target | sed -n -e "s/^> //p" >fixed
> + if ! test -s fixed; then
> + $DIFF file target |
> + grep '^+' |
> + grep -v '^+++' |
> + sed -e "s/+//" >fixed
> + fiI think those greps could be lumped into sed like:
sed -ne "s/^+[^+]//p"
(at the cost of missing blank lines, but I think that's OK for our purposes here; it could be fixed with an ERE).
Could we then make a single invocation that covers both diff formats? We can further observe that the only thing we do with the "fixed" file is count the lines, so we can leave the markers. Which means we could ditch sed entirely and use grep. Something like:
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh index 971a5a7512..15cb0c81b7 100755 --- a/t/t4124-apply-ws-rule.sh +++ b/t/t4124-apply-ws-rule.sh @@ -50,8 +50,9 @@ test_fix () { # fix should not barf apply_patch --whitespace=fix || return 1 - # find touched lines - $DIFF file target | sed -n -e "s/^> //p" >fixed + # find touched lines; handle either normal or unified + # diff, as system diff may generate either + $DIFF file target | grep '^[>+][^+]' >fixed # the changed lines are all expected to change fixed_cnt=$(wc -l <fixed) seems to work for with both busybox diff and GNU diff. -Peff