diff mbox series

[02/10] describe tests: refactor away from glob matching

Message ID 20210228195414.21372-3-avarab@gmail.com (mailing list archive)
State New
Headers show
Series describe: dont abort too early when searching tags | expand

Commit Message

Ævar Arnfjörð Bjarmason Feb. 28, 2021, 7:54 p.m. UTC
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.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t6120-describe.sh | 78 ++++++++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 40 deletions(-)

Comments

Junio C Hamano March 1, 2021, 9:26 p.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> 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. 

> 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
>  	'
>  }
diff mbox series

Patch

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="$@"
 	test_expect_success "describe $describe_opts" '
-	R=$(git describe $describe_opts 2>err.actual) &&
-	case "$R" in
-	$expect)	echo happy ;;
-	*)	echo "Oops - $R is not $expect" &&
-		false ;;
-	esac
+		git describe $describe_opts 2>err.actual >raw &&
+		sed -e "s/-g[0-9a-f]*\$/-gHASH/" <raw >actual &&
+		echo $expect >expect &&
+		test_cmp expect actual
 	'
 }
 
@@ -91,29 +89,29 @@  test_expect_success setup '
 
 '
 
-check_describe A-* HEAD
-check_describe A-* HEAD^
-check_describe R-* HEAD^^
-check_describe A-* HEAD^^2
+check_describe A-8-gHASH HEAD
+check_describe A-7-gHASH HEAD^
+check_describe R-2-gHASH HEAD^^
+check_describe A-3-gHASH HEAD^^2
 check_describe B HEAD^^2^
-check_describe R-* HEAD^^^
+check_describe R-1-gHASH HEAD^^^
 
-check_describe c-* --tags HEAD
-check_describe c-* --tags HEAD^
-check_describe e-* --tags HEAD^^
-check_describe c-* --tags HEAD^^2
+check_describe c-7-gHASH --tags HEAD
+check_describe c-6-gHASH --tags HEAD^
+check_describe e-1-gHASH --tags HEAD^^
+check_describe c-2-gHASH --tags HEAD^^2
 check_describe B --tags HEAD^^2^
 check_describe e --tags HEAD^^^
 
 check_describe heads/main --all HEAD
-check_describe tags/c-* --all HEAD^
+check_describe tags/c-6-gHASH --all HEAD^
 check_describe tags/e --all HEAD^^^
 
-check_describe B-0-* --long HEAD^^2^
-check_describe A-3-* --long HEAD^^2
+check_describe B-0-gHASH --long HEAD^^2^
+check_describe A-3-gHASH --long HEAD^^2
 
-check_describe c-7-* --tags
-check_describe e-3-* --first-parent --tags
+check_describe c-7-gHASH --tags
+check_describe e-3-gHASH --first-parent --tags
 
 test_expect_success 'describe --contains defaults to HEAD without commit-ish' '
 	echo "A^0" >expect &&
@@ -134,7 +132,7 @@  test_expect_success 'rename tag A to Q locally' '
 cat - >err.expect <<EOF
 warning: tag 'Q' is externally known as 'A'
 EOF
-check_describe A-* HEAD
+check_describe A-8-gHASH HEAD
 test_expect_success 'warning was displayed for Q' '
 	test_cmp err.expect err.actual
 '
@@ -161,22 +159,22 @@  test_expect_success 'rename tag Q back to A' '
 '
 
 test_expect_success 'pack tag refs' 'git pack-refs'
-check_describe A-* HEAD
+check_describe A-8-gHASH HEAD
 
 test_expect_success 'describe works from outside repo using --git-dir' '
 	git clone --bare "$TRASH_DIRECTORY" "$TRASH_DIRECTORY/bare" &&
 	git --git-dir "$TRASH_DIRECTORY/bare" describe >out &&
-	grep -E "^A-[1-9][0-9]?-g[0-9a-f]+$" out
+	grep -E "^A-8-g[0-9a-f]+$" out
 '
 
-check_describe "A-*[0-9a-f]" --dirty
+check_describe "A-8-gHASH" --dirty
 
 test_expect_success 'describe --dirty with --work-tree' '
 	(
 		cd "$TEST_DIRECTORY" &&
 		git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe --dirty >"$TRASH_DIRECTORY/out"
 	) &&
-	grep -E "^A-[1-9][0-9]?-g[0-9a-f]+$" out
+	grep -E "^A-8-g[0-9a-f]+$" out
 '
 
 test_expect_success 'set-up dirty work tree' '
@@ -189,7 +187,7 @@  test_expect_success 'describe --dirty with --work-tree (dirty)' '
 		cd "$TEST_DIRECTORY" &&
 		git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe --dirty >"$TRASH_DIRECTORY/out"
 	) &&
-	grep -E "^A-[1-9][0-9]?-g[0-9a-f]+-dirty$" out &&
+	grep -E "^A-8-g[0-9a-f]+-dirty$" out &&
 	test_cmp expected out
 '
 
@@ -199,7 +197,7 @@  test_expect_success 'describe --dirty=.mod with --work-tree (dirty)' '
 		cd "$TEST_DIRECTORY" &&
 		git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe --dirty=.mod >"$TRASH_DIRECTORY/out"
 	) &&
-	grep -E "^A-[1-9][0-9]?-g[0-9a-f]+.mod$" out &&
+	grep -E "^A-8-g[0-9a-f]+.mod$" out &&
 	test_cmp expected out
 '
 
@@ -223,21 +221,21 @@  test_expect_success 'set-up matching pattern tests' '
 
 '
 
-check_describe "test-annotated-*" --match="test-*"
+check_describe "test-annotated-3-gHASH" --match="test-*"
 
-check_describe "test1-lightweight-*" --tags --match="test1-*"
+check_describe "test1-lightweight-2-gHASH" --tags --match="test1-*"
 
-check_describe "test2-lightweight-*" --tags --match="test2-*"
+check_describe "test2-lightweight-1-gHASH" --tags --match="test2-*"
 
-check_describe "test2-lightweight-*" --long --tags --match="test2-*" HEAD^
+check_describe "test2-lightweight-0-gHASH" --long --tags --match="test2-*" HEAD^
 
-check_describe "test2-lightweight-*" --long --tags --match="test1-*" --match="test2-*" HEAD^
+check_describe "test2-lightweight-0-gHASH" --long --tags --match="test1-*" --match="test2-*" HEAD^
 
-check_describe "test2-lightweight-*" --long --tags --match="test1-*" --no-match --match="test2-*" HEAD^
+check_describe "test2-lightweight-0-gHASH" --long --tags --match="test1-*" --no-match --match="test2-*" HEAD^
 
-check_describe "test1-lightweight-*" --long --tags --match="test1-*" --match="test3-*" HEAD
+check_describe "test1-lightweight-2-gHASH" --long --tags --match="test1-*" --match="test3-*" HEAD
 
-check_describe "test1-lightweight-*" --long --tags --match="test3-*" --match="test1-*" HEAD
+check_describe "test1-lightweight-2-gHASH" --long --tags --match="test3-*" --match="test1-*" HEAD
 
 test_expect_success 'set-up branches' '
 	git branch branch_A A &&
@@ -247,11 +245,11 @@  test_expect_success 'set-up branches' '
 	git update-ref refs/original/original_branch_A test-annotated~2
 '
 
-check_describe "heads/branch_A*" --all --match="branch_*" --exclude="branch_C" HEAD
+check_describe "heads/branch_A-11-gHASH" --all --match="branch_*" --exclude="branch_C" HEAD
 
-check_describe "remotes/origin/remote_branch_A*" --all --match="origin/remote_branch_*" --exclude="origin/remote_branch_C" HEAD
+check_describe "remotes/origin/remote_branch_A-11-gHASH" --all --match="origin/remote_branch_*" --exclude="origin/remote_branch_C" HEAD
 
-check_describe "original/original_branch_A*" --all test-annotated~1
+check_describe "original/original_branch_A-6-gHASH" --all test-annotated~1
 
 test_expect_success '--match does not work for other types' '
 	test_must_fail git describe --all --match="*original_branch_*" test-annotated~1
@@ -521,7 +519,7 @@  test_expect_success 'describe commits with disjoint bases' '
 		git tag B -a -m B &&
 		git merge --no-ff --allow-unrelated-histories main -m x &&
 
-		check_describe "A-3-*" HEAD
+		check_describe "A-3-gHASH" HEAD
 	)
 '
 
@@ -547,7 +545,7 @@  test_expect_success 'describe commits with disjoint bases 2' '
 		git tag B -a -m B &&
 		git merge --no-ff --allow-unrelated-histories main -m x &&
 
-		check_describe "B-3-*" HEAD
+		check_describe "B-3-gHASH" HEAD
 	)
 '