git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 6/6] t4124: fix test for non-compliance diff
Date: Thu, 19 Mar 2020 12:33:34 -0400	[thread overview]
Message-ID: <20200319163334.GG3513282@coredump.intra.peff.net> (raw)
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. ;)

> 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

  reply	other threads:[~2020-03-19 16:33 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-19 14:00 [PATCH 0/6] fix test failure with busybox Đoàn Trần Công Danh
2020-03-19 14:00 ` [PATCH 1/6] t4061: use POSIX compliance regex(7) Đoàn Trần Công Danh
2020-03-19 15:53   ` Jeff King
2020-03-19 16:01     ` Eric Sunshine
2020-03-19 22:02       ` Junio C Hamano
2020-03-20  1:35         ` Danh Doan
2020-03-19 14:00 ` [PATCH 2/6] test-lib-functions: test_cmp: eval $GIT_TEST_CMP Đoàn Trần Công Danh
2020-03-19 16:02   ` Jeff King
2020-03-19 16:14     ` Eric Sunshine
2020-03-20  1:29     ` Danh Doan
2020-03-19 14:00 ` [PATCH 3/6] t5003: skip conversion test if unzip -a is unavailable Đoàn Trần Công Danh
2020-03-19 16:03   ` Jeff King
2020-03-20  0:39     ` Danh Doan
2020-03-20  5:32       ` Jeff King
2020-03-19 14:00 ` [PATCH 4/6] t5616: use rev-parse instead to get HEAD's object_id Đoàn Trần Công Danh
2020-03-19 16:07   ` Jeff King
2020-03-20  0:57     ` Danh Doan
2020-03-19 14:00 ` [PATCH 5/6] t7063: use POSIX find(1) syntax Đoàn Trần Công Danh
2020-03-19 16:12   ` Jeff King
2020-03-19 22:16     ` Junio C Hamano
2020-03-20  1:41       ` Danh Doan
2020-03-20  2:20         ` Danh Doan
2020-03-20  5:37         ` Jeff King
2020-03-22  0:37           ` Danh Doan
2020-03-22  6:05             ` Jeff King
2020-03-19 14:00 ` [PATCH 6/6] t4124: fix test for non-compliance diff Đoàn Trần Công Danh
2020-03-19 16:33   ` Jeff King [this message]
2020-03-19 22:58     ` Junio C Hamano
2020-03-20  5:20       ` Jeff King
2020-03-20  1:52     ` Danh Doan
2020-03-20  5:23       ` Jeff King
2020-03-19 15:51 ` [PATCH 0/6] fix test failure with busybox Jeff King
2020-03-20  0:37   ` Danh Doan
2020-03-20  5:30     ` Jeff King
2020-03-19 16:34 ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200319163334.GG3513282@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=congdanhqx@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).