| Message ID | a6e4ed6c3f1d37170d7e99a2fab9c90662cceb19.1616330120.git.avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | diff --no-index: fix test blind spots | expand |
On 21/03/2021 12:39, Ævar Arnfjörð Bjarmason wrote: > Add a test for --exit-code working with --no-index. There's no reason > to suppose it wouldn't, but we weren't testing for it anywhere in our > tests. Let's fix that blind spot. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > t/t4053-diff-no-index.sh | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh > index 0168946b639..9b7a8ebfd3f 100755 > --- a/t/t4053-diff-no-index.sh > +++ b/t/t4053-diff-no-index.sh > @@ -16,7 +16,12 @@ test_expect_success 'setup' ' > echo 1 >non/git/b > ' > > -test_expect_success 'git diff --no-index directories' ' > +test_expect_success 'git diff --no-index --exit-code' ' > + git diff --no-index --exit-code a/1 non/git/a && > + test_expect_code 1 git diff --no-index --exit-code a/1 a/2 > +' > + > +Test_expect_success 'git diff --no-index directories' ' I assume that s/test/Test/ was not intended. ;-) ATB, Ramsay Jones > test_expect_code 1 git diff --no-index a b >cnt && > test_line_count = 14 cnt > ' >
Ramsay Jones <ramsay@ramsayjones.plus.com> writes: > On 21/03/2021 12:39, Ævar Arnfjörð Bjarmason wrote: >> Add a test for --exit-code working with --no-index. There's no reason >> to suppose it wouldn't, but we weren't testing for it anywhere in our >> tests. Let's fix that blind spot. >> >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> >> --- >> t/t4053-diff-no-index.sh | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh >> index 0168946b639..9b7a8ebfd3f 100755 >> --- a/t/t4053-diff-no-index.sh >> +++ b/t/t4053-diff-no-index.sh >> @@ -16,7 +16,12 @@ test_expect_success 'setup' ' >> echo 1 >non/git/b >> ' >> >> -test_expect_success 'git diff --no-index directories' ' >> +test_expect_success 'git diff --no-index --exit-code' ' >> + git diff --no-index --exit-code a/1 non/git/a && >> + test_expect_code 1 git diff --no-index --exit-code a/1 a/2 >> +' >> + >> +Test_expect_success 'git diff --no-index directories' ' > > I assume that s/test/Test/ was not intended. ;-) ;-) Love to see reviewers are more careful than submitters' shells that are too lenient to allow such a test to pass before such a patch gets submitted.
On Sun, Mar 21 2021, Junio C Hamano wrote: > Ramsay Jones <ramsay@ramsayjones.plus.com> writes: > >> On 21/03/2021 12:39, Ævar Arnfjörð Bjarmason wrote: >>> Add a test for --exit-code working with --no-index. There's no reason >>> to suppose it wouldn't, but we weren't testing for it anywhere in our >>> tests. Let's fix that blind spot. >>> >>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> >>> --- >>> t/t4053-diff-no-index.sh | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh >>> index 0168946b639..9b7a8ebfd3f 100755 >>> --- a/t/t4053-diff-no-index.sh >>> +++ b/t/t4053-diff-no-index.sh >>> @@ -16,7 +16,12 @@ test_expect_success 'setup' ' >>> echo 1 >non/git/b >>> ' >>> >>> -test_expect_success 'git diff --no-index directories' ' >>> +test_expect_success 'git diff --no-index --exit-code' ' >>> + git diff --no-index --exit-code a/1 non/git/a && >>> + test_expect_code 1 git diff --no-index --exit-code a/1 a/2 >>> +' >>> + >>> +Test_expect_success 'git diff --no-index directories' ' >> >> I assume that s/test/Test/ was not intended. ;-) > > ;-) > > Love to see reviewers are more careful than submitters' shells that > are too lenient to allow such a test to pass before such a patch > gets submitted. Sorry about that, and thanks Ramsey for spotting it. FWIW I don't think there's any shell you can run the tests with that would catch this. We won't run the tests at all with "set -e"[1], and we don't use "set -e" in the tests themselves by convention. So you'll see a notice about an unknown Test_expect_success function in the output if you're not blind to it as I apparently was, but the the test will succeed, CI will pass etc. 1. In this case the thing holding it back is the structure of the sanity check added in 2006f0adaee (t/test-lib: make sure Git has already been built, 2012-09-17), we'd proceed if it was part of the "if", hrm...
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh index 0168946b639..9b7a8ebfd3f 100755 --- a/t/t4053-diff-no-index.sh +++ b/t/t4053-diff-no-index.sh @@ -16,7 +16,12 @@ test_expect_success 'setup' ' echo 1 >non/git/b ' -test_expect_success 'git diff --no-index directories' ' +test_expect_success 'git diff --no-index --exit-code' ' + git diff --no-index --exit-code a/1 non/git/a && + test_expect_code 1 git diff --no-index --exit-code a/1 a/2 +' + +Test_expect_success 'git diff --no-index directories' ' test_expect_code 1 git diff --no-index a b >cnt && test_line_count = 14 cnt '
Add a test for --exit-code working with --no-index. There's no reason to suppose it wouldn't, but we weren't testing for it anywhere in our tests. Let's fix that blind spot. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t4053-diff-no-index.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)