As noted in v1 we have a test for "git describe" test that tests for nothing at all, due to being a nested test_expect_success. This series is based on my "test-lib.sh fixes" series[2] and fixes that bug, and should address all feedback on v1. In particular: * Shell quoting fixes * The "nothing should have stderr" general testing is gone per Junio's request. * Required test-lib.sh fixes moved to [1] * We no longer change the return value of test_expect_{success,failure} to narrowly catch the bug being fixed here. * Ejected SVN test fixup patches, needed for the now-ejected test_expect_{success,failure} change. Those fixes still make sense, but I'll submit them separately (they don't depend on anything else). I think the "catch the bug" is probably a good idea, but Junio's suggestion of tracking this via some env variable "stack depth" is something that would probably collide with t0000*.sh changes I have unsubmitted/outstanding, and I don't have time to pursue it now. So I've left that out. 1. https://lore.kernel.org/git/20210228195414.21372-1-avarab@xxxxxxxxx/#t 2. https://lore.kernel.org/git/cover-00.16-00000000000-20210412T110456Z-avarab@xxxxxxxxx/ Ævar Arnfjörð Bjarmason (5): describe tests: improve test for --work-tree & --dirty describe tests: refactor away from glob matching describe tests: don't rely on err.actual from "check_describe" describe tests: fix nested "test_expect_success" call describe tests: support -C in "check_describe" t/t6120-describe.sh | 134 ++++++++++++++++++++++++-------------------- 1 file changed, 72 insertions(+), 62 deletions(-) Range-diff against v1: 1: d76214a9171 = 1: c41a777462e describe tests: improve test for --work-tree & --dirty 2: c1b8625de15 ! 2: b428f468d68 describe tests: refactor away from glob matching @@ t/t6120-describe.sh: check_describe () { - esac + git describe $describe_opts 2>err.actual >raw && + sed -e "s/-g[0-9a-f]*\$/-gHASH/" <raw >actual && -+ echo $expect >expect && ++ echo "$expect" >expect && + test_cmp expect actual ' } @@ t/t6120-describe.sh: test_expect_success setup ' - + test_commit --no-tag x file ' -check_describe A-* HEAD 3: ac1a658d07f ! 3: 50b5a41f88d describe tests: always assert empty stderr from "describe" @@ Metadata Author: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> ## Commit message ## - describe tests: always assert empty stderr from "describe" + describe tests: don't rely on err.actual from "check_describe" - Invert a test added in 3291fe4072e (Add git-describe test for "verify - annotated tag names on output", 2008-03-03) to make checking that we - don't have warnings the rule rather than the exception. + Convert the one test that relied on the "err.actual" file produced by + check_describe() to instead do its own check of "git describe" + output. - There was only one case where we expected and got a warning. Let's - test for that case explicitly, and assert no warnings or other stderr - output for all the rest. + This means that the two tests won't have an inter-dependency (e.g. if + the earlier test is skipped). + + An earlier version of this patch instead asserted that no other test + had any output on stderr. We're not doing that here out of fear that + "gc --auto" or another future change to "git describe" will cause it + to legitimately emit output on stderr unexpectedly[1]. + + I'd think that inverting the test added in 3291fe4072e (Add + git-describe test for "verify annotated tag names on output", + 2008-03-03) to make checking that we don't have warnings the rule + rather than the exception would be the sort of thing the describe + tests should be catching, but for now let's leave it as it is. + + 1. http://lore.kernel.org/git/xmqqwnuqo8ze.fsf@xxxxxxxxxxxxxxxxxxxxxx Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> @@ t/t6120-describe.sh: check_describe () { describe_opts="$@" test_expect_success "describe $describe_opts" ' - git describe $describe_opts 2>err.actual >raw && -+ git describe $describe_opts 2>err >raw && -+ test_must_be_empty err && ++ git describe $describe_opts >raw && sed -e "s/-g[0-9a-f]*\$/-gHASH/" <raw >actual && - echo $expect >expect && + echo "$expect" >expect && test_cmp expect actual @@ t/t6120-describe.sh: test_expect_success 'describe --contains defaults to HEAD without commit-ish' ' ' @@ t/t6120-describe.sh: test_expect_success 'describe --contains defaults to HEAD w -' +test_expect_success 'renaming tag A to Q locally produces a warning' " + mv .git/refs/tags/A .git/refs/tags/Q && -+ git describe HEAD 2>actual >out && ++ git describe HEAD 2>err >out && + cat >expected <<-\EOF && + warning: tag 'Q' is externally known as 'A' + EOF -+ test_cmp expected actual && ++ test_cmp expected err && + grep -E '^A-8-g[0-9a-f]+$' out +" + 4: 15efc2c6242 < -: ----------- test-lib functions: add an --annotated-tag option to "test_commit" 5: 06a8904d693 < -: ----------- describe tests: convert setup to use test_commit 6: 91424c8392b = 4: 5c81358d6bb describe tests: fix nested "test_expect_success" call 7: ecb8f6fb02f ! 5: 798f6cd63c8 describe tests: support -C in "check_describe" @@ t/t6120-describe.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME shift describe_opts="$@" test_expect_success "describe $describe_opts" ' -- git describe $describe_opts 2>err >raw && -+ git ${indir:+ -C "$indir"} describe $describe_opts 2>err >raw && - test_must_be_empty err && +- git describe $describe_opts >raw && ++ git ${indir:+ -C "$indir"} describe $describe_opts >raw && sed -e "s/-g[0-9a-f]*\$/-gHASH/" <raw >actual && - echo $expect >expect && + echo "$expect" >expect && + test_cmp expect actual @@ t/t6120-describe.sh: test_expect_success 'setup: describe commits with disjoint bases' ' ) ' 8: be5ed59dc61 < -: ----------- svn tests: remove legacy re-setup from init-clone test 9: 0b4238d012a < -: ----------- svn tests: refactor away a "set -e" in test body 10: 4f2c4f1fdd5 < -: ----------- test-lib: return 1 from test_expect_{success,failure} -- 2.31.1.634.gb41287a30b0