Re: [PATCH 02/10] describe tests: refactor away from glob matching
From: Junio C Hamano <hidden>
Date: 2021-03-01 21:30:18
Ævar Arnfjörð Bjarmason [off-list ref] writes:
Change the glob matching via a "case" statement to a "test_cmp" after we've stripped out the hash-specific g<hash-abbrev> suffix. 5312ab11fbf (Add describe test., 2007-01-13). This means that we can use test_cmp to compare the output. I could omit the "-8" change of e.g. "A-*" to "A-8-gHASH", but I think it makes sense to test that here explicitly. It means you need to add new tests to the bottom of the file, but that's not a burden in this case.
I think the point in these tests are that they consider "which tip
the tested commit is the closest" is the only piece of information
that matter, and allows the numbers ("number of commits on top of")
to be redefined in the future without having to change the tests,
but I tend to agree that such a change should be accompanied by and
documented with changes to these tests.
quoted hunk
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted> --- t/t6120-describe.sh | 78 ++++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 40 deletions(-)diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh index 7bc2aaa46e..e4fd5d567f 100755 --- a/t/t6120-describe.sh +++ b/t/t6120-describe.sh@@ -21,12 +21,10 @@ check_describe () { shift describe_opts="$@"
Just a style thing, when we are not invoking the "each positional arg is double-quoted individually against being split at $IFS" magic, we prefer to spell this as "$*".
quoted hunk
test_expect_success "describe $describe_opts" ' + git describe $describe_opts 2>err.actual >raw && + sed -e "s/-g[0-9a-f]*\$/-gHASH/" <raw >actual &&
The exact ones would be passed as-is (i.e. "test_cmp raw actual" could pass), which is what we want anyway. If we are planning to further extend this helper to make it more capable, we might want to consider quoting $describe to be evaled properly so that we can do an equivalent of check_describe A-8-gHASH --dirty='.d i r t y' HEAD when we gain new option that is more intresting than --dirty=<mark> that legitimately would benefit from being able to pass arguments with $IFS whitespace in them. But that is outside the scope of this step. I haven't seen the overall topic yet, so it may or may not be within the scope of this series. We'll see.
+ echo $expect >expect &&
Let's double-quote to relieve readers from having to wonder if you are expecting the callers to feed crazy things like "a b" and this echo to normalize it to "a b".
quoted hunk
+ test_cmp expect actual ' }