Re: [PATCH 02/10] describe tests: refactor away from glob matching
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 1, 2021, 21:26 UTC
- Message-ID
- <xmqq1rcypnua.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20210228195414.21372-3-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 8 quoted lines
> 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.
Show 12 quoted lines
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> 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 "$*".
> 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".
> + test_cmp expect actual > ' > }