Re: [PATCH 6/6] t4124: fix test for non-compliance diff

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

 



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. ;)

> 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.

> 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
> +	fi

I 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



[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