git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 10/12] test-lib-functions: remove bug-inducing "diagnostics" helper param

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Feb 9, 2021, 21:41 UTC
Message-ID
<20210209214159.22815-11-avarab@gmail.com>
In-Reply-To
<20210209214159.22815-1-avarab@gmail.com>

Remove the optional "diagnostics" parameter of the test_path_is_{file,dir,missing} functions.

We have a lot of uses of these functions, but the only legitimate use of the diagnostics parameter is from when the functions themselves were introduced in 2caf20c52b7 (test-lib: user-friendly alternatives to test [-d|-f|-e], 2010-08-10).

But as the the rest of this diff demonstrates its presence did more to silently introduce bugs in our tests. Fix such bugs in the tests added in ae4e89e549b (gc: add --keep-largest-pack option, 2018-04-15), and c04ba51739a (t6046: testcases checking whether updates can be skipped in a merge, 2018-04-19).

Let's also assert that those functions are called with exactly one parameter, a follow-up commit will add similar asserts to other functions in test-lib-functions.sh that we didn't have existing misuse of.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/README                               |  8 ++++----
 t/t3404-rebase-interactive.sh          |  3 ++-
 t/t6426-merge-skip-unneeded-updates.sh | 16 ++++++++++++----
 t/t6500-gc.sh                          |  4 ++--
 t/test-lib-functions.sh                | 11 ++++++-----
 5 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/t/README b/t/README
index c730a707705..df38d8d6be3 100644
--- a/t/README
+++ b/t/README
@@ -917,13 +917,13 @@ library for your script to use.
 
    Check whether a file has the length it is expected to.
 
- - test_path_is_file <path> [<diagnosis>]
-   test_path_is_dir <path> [<diagnosis>]
-   test_path_is_missing <path> [<diagnosis>]
+ - test_path_is_file <path>
+   test_path_is_dir <path>
+   test_path_is_missing <path>
 
    Check if the named path is a file, if the named path is a
    directory, or if the named path does not exist, respectively,
-   and fail otherwise, showing the <diagnosis> text.
+   and fail otherwise.
 
  - test_when_finished <script>
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 1e738df81d5..28c2d15d807 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -101,7 +101,8 @@ test_expect_success 'rebase -i with the exec command' '
 	) &&
 	test_path_is_file touch-one &&
 	test_path_is_file touch-two &&
-	test_path_is_missing touch-three " (should have stopped before)" &&
+	# Missing because we should have stopped by now.
+	test_path_is_missing touch-three &&
 	test_cmp_rev C HEAD &&
 	git rebase --continue &&
 	test_path_is_file touch-three &&
diff --git a/t/t6426-merge-skip-unneeded-updates.sh b/t/t6426-merge-skip-unneeded-updates.sh
index d7eeee43106..7b5f1c1dcd1 100755
--- a/t/t6426-merge-skip-unneeded-updates.sh
+++ b/t/t6426-merge-skip-unneeded-updates.sh
@@ -492,7 +492,9 @@ test_expect_success '3a-L: bq_1->foo/bq_2 on A, foo/->bar/ on B' '
 		test_cmp expect actual &&
 
 		test_must_fail git rev-parse HEAD:bq HEAD:foo/bq &&
-		test_path_is_missing bq foo/bq foo/whatever
+		test_path_is_missing bq &&
+		test_path_is_missing foo/bq &&
+		test_path_is_missing foo/whatever
 	)
 '
 
@@ -522,7 +524,9 @@ test_expect_success '3a-R: bq_1->foo/bq_2 on A, foo/->bar/ on B' '
 		test_cmp expect actual &&
 
 		test_must_fail git rev-parse HEAD:bq HEAD:foo/bq &&
-		test_path_is_missing bq foo/bq foo/whatever
+		test_path_is_missing bq &&
+		test_path_is_missing foo/bq &&
+		test_path_is_missing foo/whatever
 	)
 '
 
@@ -588,7 +592,9 @@ test_expect_success '3b-L: bq_1->foo/bq_2 on A, foo/->bar/ on B' '
 		test_cmp expect actual &&
 
 		test_must_fail git rev-parse HEAD:bq HEAD:foo/bq &&
-		test_path_is_missing bq foo/bq foo/whatever
+		test_path_is_missing bq &&
+		test_path_is_missing foo/bq &&
+		test_path_is_missing foo/whatever
 	)
 '
 
@@ -618,7 +624,9 @@ test_expect_success '3b-R: bq_1->foo/bq_2 on A, foo/->bar/ on B' '
 		test_cmp expect actual &&
 
 		test_must_fail git rev-parse HEAD:bq HEAD:foo/bq &&
-		test_path_is_missing bq foo/bq foo/whatever
+		test_path_is_missing bq &&
+		test_path_is_missing foo/bq &&
+		test_path_is_missing foo/whatever
 	)
 '
 
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 4a3b8f48ac6..7879a6b8c51 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -78,7 +78,7 @@ test_expect_success 'gc --keep-largest-pack' '
 		git gc &&
 		( cd .git/objects/pack && ls *.pack ) >pack-list &&
 		test_line_count = 1 pack-list &&
-		BASE_PACK=.git/objects/pack/pack-*.pack &&
+		cp pack-list base-pack-list &&
 		test_commit four &&
 		git repack -d &&
 		test_commit five &&
@@ -90,7 +90,7 @@ test_expect_success 'gc --keep-largest-pack' '
 		test_line_count = 2 pack-list &&
 		awk "/^P /{print \$2}" <.git/objects/info/packs >pack-info &&
 		test_line_count = 2 pack-info &&
-		test_path_is_file $BASE_PACK &&
+		test_path_is_file .git/objects/pack/$(cat base-pack-list) &&
 		git fsck
 	)
 '
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index f228647c2b8..109d1b548ce 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -717,12 +717,12 @@ test_external_without_stderr () {
 }
 
 # debugging-friendly alternatives to "test [-f|-d|-e]"
-# The commands test the existence or non-existence of $1. $2 can be
-# given to provide a more precise diagnosis.
+# The commands test the existence or non-existence of $1
 test_path_is_file () {
+	test "$#" -ne 1 && BUG "1 param"
 	if ! test -f "$1"
 	then
-		echo "File $1 doesn't exist. $2"
+		echo "File $1 doesn't exist"
 		false
 	fi
 }
@@ -730,15 +730,16 @@ test_path_is_file () {
 test_path_is_dir () {
 	if ! test -d "$1"
 	then
-		echo "Directory $1 doesn't exist. $2"
+		echo "Directory $1 doesn't exist"
 		false
 	fi
 }
 
 test_path_exists () {
+	test "$#" -ne 1 && BUG "1 param"
 	if ! test -e "$1"
 	then
-		echo "Path $1 doesn't exist. $2"
+		echo "Path $1 doesn't exist"
 		false
 	fi
 }
-- 
2.30.0.284.gd98b1dd5eaa7
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 14 of 38 in “test-lib: misc improvements”
  1. 00/12 test-lib: misc improvementsÆvar Arnfjörð Bjarmason, Feb 9, 2021
  2. 01/12 test-lib: remove check_var_migrationÆvar Arnfjörð Bjarmason, Feb 9, 2021
  3. Junio C HamanoFeb 10, 2021
  4. 02/12 test lib: change "error" to "BUG" as appropriateÆvar Arnfjörð Bjarmason, Feb 9, 2021
  5. 03/12 test-lib-functions: move test_set_index_version() to its userÆvar Arnfjörð Bjarmason, Feb 9, 2021
  6. 04/12 test-lib-functions: remove generate_zero_bytes() wrapperÆvar Arnfjörð Bjarmason, Feb 9, 2021
  7. 12/12 test-lib-functions: split out {debug,path,text} helpersÆvar Arnfjörð Bjarmason, Feb 9, 2021
  8. Denton LiuFeb 9, 2021
  9. Junio C HamanoFeb 10, 2021
  10. SZEDER GáborFeb 11, 2021
  11. Johannes SchindelinFeb 11, 2021
  12. Ævar Arnfjörð BjarmasonFeb 13, 2021
  13. 05/12 test libs: rename bundle helper to "lib-bundle.sh"Ævar Arnfjörð Bjarmason, Feb 9, 2021
  14. 10/12 test-lib-functions: remove bug-inducing "diagnostics" helper paramÆvar Arnfjörð Bjarmason, Feb 9, 2021
  15. 06/12 test libs: rename gitweb-lib.sh to lib-gitweb.shÆvar Arnfjörð Bjarmason, Feb 9, 2021
  16. 09/12 test libs: rename "diff-lib" to "lib-diff"Ævar Arnfjörð Bjarmason, Feb 9, 2021
  17. Junio C HamanoFeb 10, 2021
  18. Johannes SchindelinFeb 11, 2021
  19. Junio C HamanoFeb 11, 2021
  20. 11/12 test-lib-functions: assert correct parameter countÆvar Arnfjörð Bjarmason, Feb 9, 2021
  21. 08/12 t/.gitattributes: sort linesÆvar Arnfjörð Bjarmason, Feb 9, 2021
  22. 07/12 test-lib-functions: move function to lib-bitmap.shÆvar Arnfjörð Bjarmason, Feb 9, 2021
  23. SZEDER GáborFeb 10, 2021
  24. Jeff KingFeb 10, 2021
  25. SZEDER GáborFeb 11, 2021
  26. 00/11 test-lib: misc improvementsÆvar Arnfjörð Bjarmason, Feb 12, 2021
  27. Junio C HamanoFeb 12, 2021
  28. 02/11 test lib: change "error" to "BUG" as appropriateÆvar Arnfjörð Bjarmason, Feb 12, 2021
  29. 01/11 test-lib: remove check_var_migrationÆvar Arnfjörð Bjarmason, Feb 12, 2021
  30. 03/11 test-lib-functions: move test_set_index_version() to its userÆvar Arnfjörð Bjarmason, Feb 12, 2021
  31. 04/11 test-lib-functions: remove generate_zero_bytes() wrapperÆvar Arnfjörð Bjarmason, Feb 12, 2021
  32. 05/11 test libs: rename bundle helper to "lib-bundle.sh"Ævar Arnfjörð Bjarmason, Feb 12, 2021
  33. 06/11 test libs: rename gitweb-lib.sh to lib-gitweb.shÆvar Arnfjörð Bjarmason, Feb 12, 2021
  34. 07/11 test-lib-functions: move function to lib-bitmap.shÆvar Arnfjörð Bjarmason, Feb 12, 2021
  35. 08/11 t/.gitattributes: sort linesÆvar Arnfjörð Bjarmason, Feb 12, 2021
  36. 09/11 test libs: rename "diff-lib" to "lib-diff"Ævar Arnfjörð Bjarmason, Feb 12, 2021
  37. 10/11 test-lib-functions: remove bug-inducing "diagnostics" helper paramÆvar Arnfjörð Bjarmason, Feb 12, 2021
  38. 11/11 test-lib-functions: assert correct parameter countÆvar Arnfjörð Bjarmason, Feb 12, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.