| Message ID | patch-v3-7.8-307f25db831-20221202T114733Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tests: fix ignored & hidden exit codes | expand |
Am 02.12.22 um 12:52 schrieb Ævar Arnfjörð Bjarmason: > Change tests that would lose the "git" exit code via a negation > pattern to: > > - In the case of "t0055-beyond-symlinks.sh" compare against the > expected output instead. > > - For "t3700-add.sh" use "sed -n" to print the expected "bad" part, > and use "test_must_be_empty" to assert that it's not there. > > We can also remove a repeated invocation of "git ls-files" for the > last test that's being modified in that file, and search the > existing "files" output instead. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > t/t0055-beyond-symlinks.sh | 14 ++++++++++++-- > t/t3700-add.sh | 18 +++++++++++++----- > 2 files changed, 25 insertions(+), 7 deletions(-) > > diff --git a/t/t0055-beyond-symlinks.sh b/t/t0055-beyond-symlinks.sh > index 6bada370225..c3eb1158ef9 100755 > --- a/t/t0055-beyond-symlinks.sh > +++ b/t/t0055-beyond-symlinks.sh > @@ -15,12 +15,22 @@ test_expect_success SYMLINKS setup ' > > test_expect_success SYMLINKS 'update-index --add beyond symlinks' ' > test_must_fail git update-index --add c/d && > - ! ( git ls-files | grep c/d ) > + cat >expect <<-\EOF && > + a > + b/d > + EOF > + git ls-files >actual && > + test_cmp expect actual > ' Hmm, this makes the test depend on the other files. Adding more of them for a different test now requires updating this test as well. Why not handle it like you did in t3700 below? > > test_expect_success SYMLINKS 'add beyond symlinks' ' > test_must_fail git add c/d && > - ! ( git ls-files | grep c/d ) > + cat >expect <<-\EOF && > + a > + b/d > + EOF > + git ls-files >actual && > + test_cmp expect actual > ' Same here. > > test_done > diff --git a/t/t3700-add.sh b/t/t3700-add.sh > index 51afbd7b24a..82dd768944f 100755 > --- a/t/t3700-add.sh > +++ b/t/t3700-add.sh > @@ -106,24 +106,32 @@ test_expect_success '.gitignore test setup' ' > > test_expect_success '.gitignore is honored' ' > git add . && > - ! (git ls-files | grep "\\.ig") > + git ls-files >files && > + sed -n "/\\.ig/p" <files >actual && > + test_must_be_empty actual > ' You use sed instead of grep because no match is expected and sed returns 0 even then, while grep would exit with code 1, correct? OK. Can we use that, though? I.e. how about this? git ls-files >files && test_expect_code 1 grep "\\.ig" files It would print the unexpected lines in verbose mode like this: expecting success of 3700.12 '.gitignore is honored': git add . && git ls-files >files && echo foo.ig >files && # inject bogus value test_expect_code 1 grep "\\.ig" files foo.ig test_expect_code: command exited with 0, we wanted 1 grep \.ig files Or can we trust ls-files' own filtering? git ls-files "*.ig" >actual && test_must_be_empty actual Both variants are shorter and should be slightly faster, which can matter if we use that pattern more widely. (Didn't measure here, but from a recent foray into t3920 on Windows I took home that removing commands has a small, but measurable impact there: https://lore.kernel.org/git/203cb627-2423-8a35-d280-9f9ffc66e072@web.de/T/#u) > > test_expect_success 'error out when attempting to add ignored ones without -f' ' > test_must_fail git add a.?? && > - ! (git ls-files | grep "\\.ig") > + git ls-files >files && > + sed -n "/\\.ig/p" <files >actual && > + test_must_be_empty actual > ' > > test_expect_success 'error out when attempting to add ignored ones without -f' ' > test_must_fail git add d.?? && > - ! (git ls-files | grep "\\.ig") > + git ls-files >files && > + sed -n "/\\.ig/p" <files >actual && > + test_must_be_empty actual > ' > > test_expect_success 'error out when attempting to add ignored ones but add others' ' > touch a.if && > test_must_fail git add a.?? && > - ! (git ls-files | grep "\\.ig") && > - (git ls-files | grep a.if) > + git ls-files >files && > + sed -n "/\\.ig/p" <files >actual && > + test_must_be_empty actual && > + grep a.if files > ' > > test_expect_success 'add ignored ones with -f' '
diff --git a/t/t0055-beyond-symlinks.sh b/t/t0055-beyond-symlinks.sh index 6bada370225..c3eb1158ef9 100755 --- a/t/t0055-beyond-symlinks.sh +++ b/t/t0055-beyond-symlinks.sh @@ -15,12 +15,22 @@ test_expect_success SYMLINKS setup ' test_expect_success SYMLINKS 'update-index --add beyond symlinks' ' test_must_fail git update-index --add c/d && - ! ( git ls-files | grep c/d ) + cat >expect <<-\EOF && + a + b/d + EOF + git ls-files >actual && + test_cmp expect actual ' test_expect_success SYMLINKS 'add beyond symlinks' ' test_must_fail git add c/d && - ! ( git ls-files | grep c/d ) + cat >expect <<-\EOF && + a + b/d + EOF + git ls-files >actual && + test_cmp expect actual ' test_done diff --git a/t/t3700-add.sh b/t/t3700-add.sh index 51afbd7b24a..82dd768944f 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -106,24 +106,32 @@ test_expect_success '.gitignore test setup' ' test_expect_success '.gitignore is honored' ' git add . && - ! (git ls-files | grep "\\.ig") + git ls-files >files && + sed -n "/\\.ig/p" <files >actual && + test_must_be_empty actual ' test_expect_success 'error out when attempting to add ignored ones without -f' ' test_must_fail git add a.?? && - ! (git ls-files | grep "\\.ig") + git ls-files >files && + sed -n "/\\.ig/p" <files >actual && + test_must_be_empty actual ' test_expect_success 'error out when attempting to add ignored ones without -f' ' test_must_fail git add d.?? && - ! (git ls-files | grep "\\.ig") + git ls-files >files && + sed -n "/\\.ig/p" <files >actual && + test_must_be_empty actual ' test_expect_success 'error out when attempting to add ignored ones but add others' ' touch a.if && test_must_fail git add a.?? && - ! (git ls-files | grep "\\.ig") && - (git ls-files | grep a.if) + git ls-files >files && + sed -n "/\\.ig/p" <files >actual && + test_must_be_empty actual && + grep a.if files ' test_expect_success 'add ignored ones with -f' '
Change tests that would lose the "git" exit code via a negation pattern to: - In the case of "t0055-beyond-symlinks.sh" compare against the expected output instead. - For "t3700-add.sh" use "sed -n" to print the expected "bad" part, and use "test_must_be_empty" to assert that it's not there. We can also remove a repeated invocation of "git ls-files" for the last test that's being modified in that file, and search the existing "files" output instead. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t0055-beyond-symlinks.sh | 14 ++++++++++++-- t/t3700-add.sh | 18 +++++++++++++----- 2 files changed, 25 insertions(+), 7 deletions(-)