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
