| Message ID | 20201012091751.19594-4-michal@isc.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | diff: add -I<regex> that ignores matching changes | expand |
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 > >
Hi Johannes, > > 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. Right, I expressed my doubts in this area at the end of the cover letter for v1: >> - Should tests be added in a separate commit? This is what I did as I >> thought it would help with readability, but... I will be glad to follow any guidance provided, I just picked one of the two possible routes for v1. > 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. First, let me say that the goal of minimizing the run time of a test suite is close to my heart (it is an issue at my day job). Yet, I assumed that this new test would not be detrimental to test suite run times as it takes about half a second to run t4069-diff-ignore-regex.sh on my machine - and (I hope) its contents are in line with the "tests are the best documentation" proverb. That being said, I realize that the hosts used in various test environments may have different processing capabilities. I tried preparing something exhaustive and well-commented, so that it is clear what to expect from the new feature. Yet, if you would rather have me cut some things out, I am certainly not particularly attached to the tests from patch 3 and I will be glad to rip them out if that is the recommendation :-) > 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? Right, neat, though this does not (yet) test: - the interaction between -I and --ignore-blank-lines (this is visible in code coverage), - whether the list of hunks emitted varies for different -U<n> values, - diffstat with -I<regex>, - invalid regular expressions. Would you like me to add these tests to your proposal or to skip them, given that -I uses the same field for marking changes as ignored as --ignore-blank-lines does? > 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> Right, makes sense, I have not thought of that use case.
Hi Michał, On Tue, 13 Oct 2020, Michał Kępień wrote: > > 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. > > First, let me say that the goal of minimizing the run time of a test > suite is close to my heart (it is an issue at my day job). Yet, I > assumed that this new test would not be detrimental to test suite run > times as it takes about half a second to run t4069-diff-ignore-regex.sh > on my machine - and (I hope) its contents are in line with the "tests > are the best documentation" proverb. Sadly, the test is not quite as fast on Windows. I just ran this (on a not quite idle machine, admittedly) and it ended in this: # passed all 11 test(s) 1..11 real 0m51.470s user 0m0.046s sys 0m0.015s Yes, that's almost a minute. > > 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? > > Right, neat, though this does not (yet) test: > > - the interaction between -I and --ignore-blank-lines (this is visible > in code coverage), Right. Any chance you can finagle that in, e.g. by yet another `-e` argument to the `sed` call? > - whether the list of hunks emitted varies for different -U<n> values, I am not worried about that. > - diffstat with -I<regex>, I am not worried about that, either, as `diffstat` consumes `xdiff`'s output, therefore if one consumer works, another consumer will work, too. > - invalid regular expressions. Right, that should be super easy (and quick) to test. > Would you like me to add these tests to your proposal or to skip them, > given that -I uses the same field for marking changes as ignored as > --ignore-blank-lines does? I'd say it depends how easily (read: in how small a test case or modifications to an existing test case) you can add a test for that interaction. Thanks, Dscho
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> - diffstat with -I<regex>, > > I am not worried about that, either, as `diffstat` consumes `xdiff`'s > output, therefore if one consumer works, another consumer will work, too. Careful. Such a "we know what happens in the code" transparent-box testing attitude is laying a minefield for later our selves. As we learned in a recent bug in sequencer, some corners of implementation do the same thing in different codepaths as optimization. The really bad part of the story is that such an implementation detail can and will change over time, since that is the kind of thing we do when optimizing code. In other words, we only know what happens in the current code. And automated tests protect us from the future, when done right. If written with too intimate knowledge of how the current code works, well, what are we really testing? It's a balancing act and there is no single "right" answer, but I'd draw the line on a bit more careful side than you are drawing here. Thanks.
Hi Johannes, > > First, let me say that the goal of minimizing the run time of a test > > suite is close to my heart (it is an issue at my day job). Yet, I > > assumed that this new test would not be detrimental to test suite run > > times as it takes about half a second to run t4069-diff-ignore-regex.sh > > on my machine - and (I hope) its contents are in line with the "tests > > are the best documentation" proverb. > > Sadly, the test is not quite as fast on Windows. I just ran this (on a not > quite idle machine, admittedly) and it ended in this: > > # passed all 11 test(s) > 1..11 > > real 0m51.470s > user 0m0.046s > sys 0m0.015s > > Yes, that's almost a minute. Out of curiosity: is that under Cygwin? I have seen shell-based tests finishing in 15 *seconds* on Unix-like systems and in 15 *minutes* under Cygwin, which would be in line with your measurements provided above. > > Right, neat, though this does not (yet) test: > > > > - the interaction between -I and --ignore-blank-lines (this is visible > > in code coverage), > > Right. Any chance you can finagle that in, e.g. by yet another `-e` > argument to the `sed` call? I will try in v3 (while also looking at what I can do for other missing -I<regex> tests I pointed out).
Hi Michał, On Tue, 13 Oct 2020, Michał Kępień wrote: > > > First, let me say that the goal of minimizing the run time of a test > > > suite is close to my heart (it is an issue at my day job). Yet, I > > > assumed that this new test would not be detrimental to test suite run > > > times as it takes about half a second to run t4069-diff-ignore-regex.sh > > > on my machine - and (I hope) its contents are in line with the "tests > > > are the best documentation" proverb. > > > > Sadly, the test is not quite as fast on Windows. I just ran this (on a not > > quite idle machine, admittedly) and it ended in this: > > > > # passed all 11 test(s) > > 1..11 > > > > real 0m51.470s > > user 0m0.046s > > sys 0m0.015s > > > > Yes, that's almost a minute. > > Out of curiosity: is that under Cygwin? I have seen shell-based tests > finishing in 15 *seconds* on Unix-like systems and in 15 *minutes* under > Cygwin, which would be in line with your measurements provided above. Close enough: it's an MSYS2 Bash (and MSYS2 is a close fork of Cygwin). It is what Git for Windows uses. Ciao, Dscho
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
Exercise the new -I<regex> diff option in various scenarios to ensure it behaves as expected. Signed-off-by: Michał Kępień <michal@isc.org> --- t/t4069-diff-ignore-regex.sh | 426 +++++++++++++++++++++++++++++++++++ 1 file changed, 426 insertions(+) create mode 100755 t/t4069-diff-ignore-regex.sh