All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: "Michał Kępień" <michal@isc.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 3/3] t: add -I<regex> tests
Date: Mon, 12 Oct 2020 13:49:32 +0200 (CEST)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.2010121320190.50@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <20201012091751.19594-4-michal@isc.org>

[-- Attachment #1: Type: text/plain, Size: 13600 bytes --]

Hi Michał,

On Mon, 12 Oct 2020, Michał Kępień wrote:

> Exercise the new -I<regex> diff option in various scenarios to ensure it
> behaves as expected.

Excellent. I was actually looking for a test in patch 2/3 and almost
commented about that.

>
> Signed-off-by: Michał Kępień <michal@isc.org>
> ---
>  t/t4069-diff-ignore-regex.sh | 426 +++++++++++++++++++++++++++++++++++

Hmm. I wonder whether we could do with a much more concise test script.
The test suite already takes a quite long time to run, which is not a
laughing matter: we had issues in the past where contributors would skip
running it because it took too long, and this test is sure to exacerbate
that problem.

I could imagine, for example, that it would be plenty enough to do
something like this instead:

-- snip --
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 5c7b0122b4f..bf158be137f 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -6,6 +6,7 @@
 test_description='Various diff formatting options'

 . ./test-lib.sh
+. "$TEST_DIRECTORY"/diff-lib.sh

 test_expect_success setup '

@@ -473,4 +474,24 @@ test_expect_success 'diff-tree --stdin with log formatting' '
 	test_cmp expect actual
 '

+test_expect_success '-I<regex>' '
+	seq 50 >I.txt &&
+	sed -e "s/13/ten and three/" -e "/7\$/d" <I.txt >J.txt &&
+	test_must_fail git diff --no-index -I"ten.*e" -I"^[124-9]" I.txt J.txt >actual &&
+	cat >expect <<-\EOF &&
+	diff --git a/I.txt b/J.txt
+	--- a/I.txt
+	+++ b/J.txt
+	@@ -34,7 +31,6 @@
+	 34
+	 35
+	 36
+	-37
+	 38
+	 39
+	 40
+	EOF
+	compare_diff_patch expect actual
+'
+
 test_done
-- snap --

Note how it tests various things in one go?

Thanks,
Dscho

P.S.: My main interest in the `-I` option is its use case for `git
range-diff` in Git's own context: if you want to compare your patches to
what entered the `seen` branch, there will _always_ be a difference
because Junio adds their DCO. Something like this can help that:

git range-diff \
	-I'^    Signed-off-by: Junio C Hamano <gitster@pobox.com>$' \
	<my-patch-range> <the-equivalent-in-seen>

>  1 file changed, 426 insertions(+)
>  create mode 100755 t/t4069-diff-ignore-regex.sh
>
> diff --git a/t/t4069-diff-ignore-regex.sh b/t/t4069-diff-ignore-regex.sh
> new file mode 100755
> index 0000000000..4ddf5c67ae
> --- /dev/null
> +++ b/t/t4069-diff-ignore-regex.sh
> @@ -0,0 +1,426 @@
> +#!/bin/sh
> +
> +test_description='Test diff -I<regex>'
> +
> +. ./test-lib.sh
> +. "$TEST_DIRECTORY"/diff-lib.sh
> +
> +test_expect_success setup '
> +	test_seq 20 >x &&
> +	git update-index --add x
> +'
> +
> +test_expect_success 'one line changed' '
> +	test_seq 20 | sed "s/10/100/" >x &&
> +
> +	# Get plain diff
> +	git diff >plain &&
> +	cat >expected <<-EOF &&
> +	diff --git a/x b/x
> +	--- a/x
> +	+++ b/x
> +	@@ -7,7 +7,7 @@
> +	 7
> +	 8
> +	 9
> +	-10
> +	+100
> +	 11
> +	 12
> +	 13
> +	EOF
> +	compare_diff_patch expected plain &&
> +
> +	# Both old and new line match regex - ignore change
> +	git diff -I "^10" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Both old and new line match some regex - ignore change
> +	git diff -I "^10\$" -I "^100" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Only old line matches regex - do not ignore change
> +	git diff -I "^10\$" >actual &&
> +	compare_diff_patch plain actual &&
> +
> +	# Only new line matches regex - do not ignore change
> +	git diff -I "^100" >actual &&
> +	compare_diff_patch plain actual &&
> +
> +	# Only old line matches some regex - do not ignore change
> +	git diff -I "^10\$" -I "^101" >actual &&
> +	compare_diff_patch plain actual &&
> +
> +	# Only new line matches some regex - do not ignore change
> +	git diff -I "^11\$" -I "^100" >actual &&
> +	compare_diff_patch plain actual
> +'
> +
> +test_expect_success 'one line removed' '
> +	test_seq 20 | sed "10d" >x &&
> +
> +	# Get plain diff
> +	git diff >plain &&
> +	cat >expected <<-EOF &&
> +	diff --git a/x b/x
> +	--- a/x
> +	+++ b/x
> +	@@ -7,7 +7,6 @@
> +	 7
> +	 8
> +	 9
> +	-10
> +	 11
> +	 12
> +	 13
> +	EOF
> +	compare_diff_patch expected plain &&
> +
> +	# Removed line matches regex - ignore change
> +	git diff -I "^10" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Removed line matches some regex - ignore change
> +	git diff -I "^10" -I "^100" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Removed line does not match regex - do not ignore change
> +	git diff -I "^101" >actual &&
> +	compare_diff_patch plain actual &&
> +
> +	# Removed line does not match any regex - do not ignore change
> +	git diff -I "^100" -I "^101" >actual &&
> +	compare_diff_patch plain actual
> +'
> +
> +test_expect_success 'one line added' '
> +	test_seq 21 >x &&
> +
> +	# Get plain diff
> +	git diff >plain &&
> +	cat >expected <<-EOF &&
> +	diff --git a/x b/x
> +	--- a/x
> +	+++ b/x
> +	@@ -18,3 +18,4 @@
> +	 18
> +	 19
> +	 20
> +	+21
> +	EOF
> +	compare_diff_patch expected plain &&
> +
> +	# Added line matches regex - ignore change
> +	git diff -I "^21" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Added line matches some regex - ignore change
> +	git diff -I "^21" -I "^22" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Added line does not match regex - do not ignore change
> +	git diff -I "^212" >actual &&
> +	compare_diff_patch plain actual &&
> +
> +	# Added line does not match any regex - do not ignore change
> +	git diff -I "^211" -I "^212" >actual &&
> +	compare_diff_patch plain actual
> +'
> +
> +test_expect_success 'last two lines changed' '
> +	test_seq 20 | sed "s/19/21/; s/20/22/" >x &&
> +
> +	# Get plain diff
> +	git diff >plain &&
> +	cat >expected <<-EOF &&
> +	diff --git a/x b/x
> +	--- a/x
> +	+++ b/x
> +	@@ -16,5 +16,5 @@
> +	 16
> +	 17
> +	 18
> +	-19
> +	-20
> +	+21
> +	+22
> +	EOF
> +	compare_diff_patch expected plain &&
> +
> +	# All changed lines match regex - ignore change
> +	git diff -I "^[12]" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# All changed lines match some regex - ignore change
> +	git diff -I "^1" -I "^2" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Not all changed lines match regex - do not ignore change
> +	git diff -I "^2" >actual &&
> +	compare_diff_patch plain actual &&
> +
> +	# Not all changed lines match some regex - do not ignore change
> +	git diff -I "^2" -I "^3" >actual &&
> +	compare_diff_patch plain actual
> +'
> +
> +test_expect_success 'two non-adjacent lines removed in the same hunk' '
> +	test_seq 20 | sed "1d; 3d" >x &&
> +
> +	# Get plain diff
> +	git diff >plain &&
> +	cat >expected <<-EOF &&
> +	diff --git a/x b/x
> +	--- a/x
> +	+++ b/x
> +	@@ -1,6 +1,4 @@
> +	-1
> +	 2
> +	-3
> +	 4
> +	 5
> +	 6
> +	EOF
> +	compare_diff_patch expected plain &&
> +
> +	# Both removed lines match regex - ignore hunk
> +	git diff -I "^[1-3]" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Both removed lines match some regex - ignore hunk
> +	git diff -I "^1" -I "^3" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# First removed line does not match regex - do not ignore hunk
> +	git diff -I "^[2-3]" >actual &&
> +	compare_diff_patch plain actual &&
> +
> +	# First removed line does not match any regex - do not ignore hunk
> +	git diff -I "^2" -I "^3" >actual &&
> +	compare_diff_patch plain actual &&
> +
> +	# Second removed line does not match regex - do not ignore hunk
> +	git diff -I "^[1-2]" >actual &&
> +	compare_diff_patch plain actual &&
> +
> +	# Second removed line does not match any regex - do not ignore hunk
> +	git diff -I "^1" -I "^2" >actual &&
> +	compare_diff_patch plain actual
> +'
> +
> +test_expect_success 'two non-adjacent lines removed in the same hunk, with -U1' '
> +	test_seq 20 | sed "1d; 3d" >x &&
> +
> +	# Get plain diff
> +	git diff -U1 >plain &&
> +	cat >expected <<-EOF &&
> +	diff --git a/x b/x
> +	--- a/x
> +	+++ b/x
> +	@@ -1,4 +1,2 @@
> +	-1
> +	 2
> +	-3
> +	 4
> +	EOF
> +	compare_diff_patch expected plain &&
> +
> +	# Both removed lines match regex - ignore hunk
> +	git diff -U1 -I "^[1-3]" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Both removed lines match some regex - ignore hunk
> +	git diff -U1 -I "^1" -I "^3" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# First removed line does not match regex, but is out of context - ignore second change
> +	git diff -U1 -I "^[2-3]" >actual &&
> +	cat >second-change-ignored <<-EOF &&
> +	diff --git a/x b/x
> +	--- a/x
> +	+++ b/x
> +	@@ -1,2 +1 @@
> +	-1
> +	 2
> +	EOF
> +	compare_diff_patch second-change-ignored actual &&
> +
> +	# First removed line does not match any regex, but is out of context - ignore second change
> +	git diff -U1 -I "^2" -I "^3" >actual &&
> +	compare_diff_patch second-change-ignored actual &&
> +
> +	# Second removed line does not match regex, but is out of context - ignore first change
> +	git diff -U1 -I "^[1-2]" >actual &&
> +	cat >first-change-ignored <<-EOF &&
> +	diff --git a/x b/x
> +	--- a/x
> +	+++ b/x
> +	@@ -2,3 +1,2 @@
> +	 2
> +	-3
> +	 4
> +	EOF
> +	compare_diff_patch first-change-ignored actual &&
> +
> +	# Second removed line does not match any regex, but is out of context - ignore first change
> +	git diff -U1 -I "^1" -I "^2" >actual &&
> +	compare_diff_patch first-change-ignored actual
> +'
> +
> +test_expect_success 'multiple hunks' '
> +	test_seq 20 | sed "1d; 20d" >x &&
> +
> +	# Get plain diff
> +	git diff >plain &&
> +	cat >expected <<-EOF &&
> +	diff --git a/x b/x
> +	--- a/x
> +	+++ b/x
> +	@@ -1,4 +1,3 @@
> +	-1
> +	 2
> +	 3
> +	 4
> +	@@ -17,4 +16,3 @@
> +	 17
> +	 18
> +	 19
> +	-20
> +	EOF
> +	compare_diff_patch expected plain &&
> +
> +	# Ignore both hunks (single regex)
> +	git diff -I "^[12]" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Ignore both hunks (multiple regexes)
> +	git diff -I "^1" -I "^2" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Only ignore first hunk (single regex)
> +	git diff -I "^1" >actual &&
> +	cat >first-hunk-ignored <<-EOF &&
> +	diff --git a/x b/x
> +	--- a/x
> +	+++ b/x
> +	@@ -17,4 +16,3 @@
> +	 17
> +	 18
> +	 19
> +	-20
> +	EOF
> +	compare_diff_patch first-hunk-ignored actual &&
> +
> +	# Only ignore first hunk (multiple regexes)
> +	git diff -I "^0" -I "^1" >actual &&
> +	compare_diff_patch first-hunk-ignored actual &&
> +
> +	# Only ignore second hunk (single regex)
> +	git diff -I "^2" >actual &&
> +	cat >second-hunk-ignored <<-EOF &&
> +	diff --git a/x b/x
> +	--- a/x
> +	+++ b/x
> +	@@ -1,4 +1,3 @@
> +	-1
> +	 2
> +	 3
> +	 4
> +	EOF
> +	compare_diff_patch second-hunk-ignored actual &&
> +
> +	# Only ignore second hunk (multiple regexes)
> +	git diff -I "^2" -I "^3" >actual &&
> +	compare_diff_patch second-hunk-ignored actual
> +'
> +
> +test_expect_success 'multiple hunks, with --ignore-blank-lines' '
> +	echo >x &&
> +	test_seq 21 >>x &&
> +
> +	# Get plain diff
> +	git diff >plain &&
> +	cat >expected <<-EOF &&
> +	diff --git a/x b/x
> +	--- a/x
> +	+++ b/x
> +	@@ -1,3 +1,4 @@
> +	+
> +	 1
> +	 2
> +	 3
> +	@@ -18,3 +19,4 @@
> +	 18
> +	 19
> +	 20
> +	+21
> +	EOF
> +	compare_diff_patch expected plain &&
> +
> +	# -I does not override --ignore-blank-lines - ignore both hunks (single regex)
> +	git diff --ignore-blank-lines -I "^21" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# -I does not override --ignore-blank-lines - ignore both hunks (multiple regexes)
> +	git diff --ignore-blank-lines -I "^21" -I "^12" >actual &&
> +	test_must_be_empty actual
> +'
> +
> +test_expect_success 'diffstat' '
> +	test_seq 20 | sed "s/^5/0/p; s/^15/10/; 16d" >x &&
> +
> +	# Get plain diffstat
> +	git diff --stat >actual &&
> +	cat >expected <<-EOF &&
> +	 x | 6 +++---
> +	 1 file changed, 3 insertions(+), 3 deletions(-)
> +	EOF
> +	test_cmp expected actual &&
> +
> +	# Ignore both hunks (single regex)
> +	git diff --stat -I "^[0-5]" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Ignore both hunks (multiple regexes)
> +	git diff --stat -I "^0" -I "^1" -I "^5" >actual &&
> +	test_must_be_empty actual &&
> +
> +	# Only ignore first hunk (single regex)
> +	git diff --stat -I "^[05]" >actual &&
> +	cat >expected <<-EOF &&
> +	 x | 3 +--
> +	 1 file changed, 1 insertion(+), 2 deletions(-)
> +	EOF
> +	test_cmp expected actual &&
> +
> +	# Only ignore first hunk (multiple regexes)
> +	git diff --stat -I "^0" -I "^5" >actual &&
> +	test_cmp expected actual &&
> +
> +	# Only ignore second hunk (single regex)
> +	git diff --stat -I "^1" >actual &&
> +	cat >expected <<-EOF &&
> +	 x | 3 ++-
> +	 1 file changed, 2 insertions(+), 1 deletion(-)
> +	EOF
> +	test_cmp expected actual &&
> +
> +	# Only ignore second hunk (multiple regexes)
> +	git diff --stat -I "^1" -I "^2" >actual &&
> +	test_cmp expected actual
> +'
> +
> +test_expect_success 'invalid regexes' '
> +	>x &&
> +
> +	# Single invalid regex
> +	git diff -I "^[1" 2>&1 | grep "invalid regex: " &&
> +
> +	# Two regexes: first invalid, second valid
> +	git diff -I "^[1" -I "^1" 2>&1 | grep "invalid regex: " &&
> +
> +	# Two invalid regexes
> +	git diff -I "^[1" -I "^[2" 2>&1 | grep "invalid regex: "
> +'
> +
> +test_done
> --
> 2.28.0
>
>

  reply	other threads:[~2020-10-12 11:54 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-01 12:06 [PATCH 0/2] diff: add -I<regex> that ignores matching changes Michał Kępień
2020-10-01 12:06 ` [PATCH 1/2] " Michał Kępień
2020-10-01 18:21   ` Junio C Hamano
2020-10-07 19:48     ` Michał Kępień
2020-10-07 20:08       ` Junio C Hamano
2020-10-01 12:06 ` [PATCH 2/2] t: add -I<regex> tests Michał Kępień
2020-10-01 17:02 ` [PATCH 0/2] diff: add -I<regex> that ignores matching changes Junio C Hamano
2020-10-12  9:17 ` [PATCH v2 0/3] " Michał Kępień
2020-10-12  9:17   ` [PATCH v2 1/3] merge-base, xdiff: zero out xpparam_t structures Michał Kępień
2020-10-12 11:14     ` Johannes Schindelin
2020-10-12 17:09       ` Junio C Hamano
2020-10-12 19:52     ` Junio C Hamano
2020-10-13  6:35       ` Michał Kępień
2020-10-12  9:17   ` [PATCH v2 2/3] diff: add -I<regex> that ignores matching changes Michał Kępień
2020-10-12 11:20     ` Johannes Schindelin
2020-10-12 20:00       ` Junio C Hamano
2020-10-12 20:39         ` Johannes Schindelin
2020-10-12 21:43           ` Junio C Hamano
2020-10-13  6:37             ` Michał Kępień
2020-10-13 15:49               ` Junio C Hamano
2020-10-13  6:36       ` Michał Kępień
2020-10-13 12:02         ` Johannes Schindelin
2020-10-13 15:53           ` Junio C Hamano
2020-10-13 18:45           ` Michał Kępień
2020-10-12 18:01     ` Junio C Hamano
2020-10-13  6:38       ` Michał Kępień
2020-10-12 20:04     ` Junio C Hamano
2020-10-13  6:38       ` Michał Kępień
2020-10-12  9:17   ` [PATCH v2 3/3] t: add -I<regex> tests Michał Kępień
2020-10-12 11:49     ` Johannes Schindelin [this message]
2020-10-13  6:38       ` Michał Kępień
2020-10-13 12:00         ` Johannes Schindelin
2020-10-13 16:00           ` Junio C Hamano
2020-10-13 19:01           ` Michał Kępień
2020-10-15 11:45             ` Johannes Schindelin
2020-10-15  7:24   ` [PATCH v3 0/2] diff: add -I<regex> that ignores matching changes Michał Kępień
2020-10-15  7:24     ` [PATCH v3 1/2] merge-base, xdiff: zero out xpparam_t structures Michał Kępień
2020-10-15  7:24     ` [PATCH v3 2/2] diff: add -I<regex> that ignores matching changes Michał Kępień
2020-10-16 15:32       ` Phillip Wood
2020-10-16 18:04         ` Junio C Hamano
2020-10-19  9:48           ` Michał Kępień
2020-10-16 18:16       ` Junio C Hamano
2020-10-19  9:55         ` Michał Kępień
2020-10-19 17:29           ` Junio C Hamano
2020-10-16 10:00     ` [PATCH v3 0/2] " Johannes Schindelin
2020-10-20  6:48     ` [PATCH v4 " Michał Kępień
2020-10-20  6:48       ` [PATCH v4 1/2] merge-base, xdiff: zero out xpparam_t structures Michał Kępień
2020-10-20  6:48       ` [PATCH v4 2/2] diff: add -I<regex> that ignores matching changes Michał Kępień
2021-02-05 14:13       ` [PATCH 1/2] diff: add an API for deferred freeing Ævar Arnfjörð Bjarmason
2021-02-10 16:00         ` Johannes Schindelin
2021-02-11  3:00           ` Ævar Arnfjörð Bjarmason
2021-02-11  9:40             ` Johannes Schindelin
2021-02-11 10:21               ` Jeff King
2021-02-11 10:45                 ` [PATCH v2 0/2] " Ævar Arnfjörð Bjarmason
2021-02-11 10:45                 ` [PATCH v2 1/2] " Ævar Arnfjörð Bjarmason
2021-02-11 10:45                 ` [PATCH v2 2/2] diff: plug memory leak from regcomp() on {log,diff} -I Ævar Arnfjörð Bjarmason
2021-02-05 14:13       ` [PATCH " Ævar Arnfjörð Bjarmason

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

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

  git send-email \
    --in-reply-to=nycvar.QRO.7.76.6.2010121320190.50@tvgsbejvaqbjf.bet \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=michal@isc.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.