[PATCH 00/12] test-lib: misc improvements

[PATCH 00/12] test-lib: misc improvements

[Ævar Arnfjörð Bjarmason]

Various miscellaneous small and easy to review cleanups & fixes to the
test libraries. This doesn't conflict with the current "seen" branch.

Ævar Arnfjörð Bjarmason (12):
  test-lib: remove check_var_migration
  test lib: change "error" to "BUG" as appropriate
  test-lib-functions: move test_set_index_version() to its user
  test-lib-functions: remove generate_zero_bytes() wrapper
  test libs: rename bundle helper to "lib-bundle.sh"
  test libs: rename gitweb-lib.sh to lib-gitweb.sh
  test-lib-functions: move function to lib-bitmap.sh
  t/.gitattributes: sort lines
  test libs: rename "diff-lib" to "lib-diff"
  test-lib-functions: remove bug-inducing "diagnostics" helper param
  test-lib-functions: assert correct parameter count
  test-lib-functions: split out {debug,path,text} helpers

 t/.gitattributes                              |   4 +-
 t/README                                      |   8 +-
 t/lib-bitmap.sh                               |  26 ++
 t/{test-bundle-functions.sh => lib-bundle.sh} |   0
 t/{diff-lib.sh => lib-diff.sh}                |   0
 t/{diff-lib => lib-diff}/COPYING              |   0
 t/{diff-lib => lib-diff}/README               |   0
 t/{gitweb-lib.sh => lib-gitweb.sh}            |   0
 t/t2104-update-index-skip-worktree.sh         |   5 +
 t/t3404-rebase-interactive.sh                 |   3 +-
 t/t4000-diff-format.sh                        |   2 +-
 t/t4001-diff-rename.sh                        |   2 +-
 t/t4003-diff-rename-1.sh                      |   6 +-
 t/t4004-diff-rename-symlink.sh                |   2 +-
 t/t4005-diff-rename-2.sh                      |   6 +-
 t/t4007-rename-3.sh                           |   6 +-
 t/t4008-diff-break-rewrite.sh                 |   6 +-
 t/t4009-diff-rename-4.sh                      |   6 +-
 t/t4010-diff-pathspec.sh                      |   2 +-
 t/t4011-diff-symlink.sh                       |   2 +-
 t/t4013-diff-various.sh                       |   2 +-
 t/t4015-diff-whitespace.sh                    |   2 +-
 t/t4027-diff-submodule.sh                     |   2 +-
 t/t4034-diff-words.sh                         |   2 +-
 t/t4038-diff-combined.sh                      |   2 +-
 t/t4061-diff-indent.sh                        |   2 +-
 t/t4206-log-follow-harder-copies.sh           |   2 +-
 t/t5310-pack-bitmaps.sh                       |   2 +
 t/t5318-commit-graph.sh                       |   2 +-
 t/t5510-fetch.sh                              |   2 +-
 t/t6020-bundle-misc.sh                        |   2 +-
 t/t6113-rev-list-bitmap-filters.sh            |   1 +
 t/t6426-merge-skip-unneeded-updates.sh        |  16 +-
 t/t6500-gc.sh                                 |   4 +-
 t/t7501-commit-basic-functionality.sh         |   2 +-
 t/t9300-fast-import.sh                        |   2 +-
 t/t9500-gitweb-standalone-no-errors.sh        |   2 +-
 t/t9501-gitweb-standalone-http-status.sh      |   2 +-
 t/t9502-gitweb-standalone-parse-output.sh     |   2 +-
 t/test-lib-functions.sh                       | 305 +-----------------
 t/test-lib-functions/debug.sh                 |  99 ++++++
 t/test-lib-functions/path.sh                  | 137 ++++++++
 t/test-lib-functions/text.sh                  |  33 ++
 t/test-lib.sh                                 |  30 --
 44 files changed, 370 insertions(+), 373 deletions(-)
 create mode 100644 t/lib-bitmap.sh
 rename t/{test-bundle-functions.sh => lib-bundle.sh} (100%)
 rename t/{diff-lib.sh => lib-diff.sh} (100%)
 rename t/{diff-lib => lib-diff}/COPYING (100%)
 rename t/{diff-lib => lib-diff}/README (100%)
 rename t/{gitweb-lib.sh => lib-gitweb.sh} (100%)
 create mode 100644 t/test-lib-functions/debug.sh
 create mode 100644 t/test-lib-functions/path.sh
 create mode 100644 t/test-lib-functions/text.sh

-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH 01/12] test-lib: remove check_var_migration

[Ævar Arnfjörð Bjarmason]

Remove the check_var_migration() migration helper. This was added back
in [1], [2] and [3] to warn users to migrate from e.g. the
"GIT_FSMONITOR_TEST" name to "GIT_TEST_FSMONITOR".

I daresay that having been warning about this since late 2018 (or
v2.20.0) was sufficient time to give everyone interested a heads-up
about moving to the new names.

I don't see the need for going through the "do this later" codepath
anticipated in [1], let's just remove this instead.

1. 4cb54d0aa8e (fsmonitor: update GIT_TEST_FSMONITOR support,
   2018-09-18)
2. 1f357b045b5 (read-cache: update TEST_GIT_INDEX_VERSION support,
   2018-09-18)
3. 5765d97b71d (preload-index: update GIT_FORCE_PRELOAD_TEST support,
   2018-09-18)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh | 30 ------------------------------
 1 file changed, 30 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 76062db2964..61bda88245c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -462,36 +462,6 @@ export GIT_DEFAULT_HASH
 GIT_TRACE_BARE=1
 export GIT_TRACE_BARE
 
-check_var_migration () {
-	# the warnings and hints given from this helper depends
-	# on end-user settings, which will disrupt the self-test
-	# done on the test framework itself.
-	case "$GIT_TEST_FRAMEWORK_SELFTEST" in
-	t)	return ;;
-	esac
-
-	old_name=$1 new_name=$2
-	eval "old_isset=\${${old_name}:+isset}"
-	eval "new_isset=\${${new_name}:+isset}"
-
-	case "$old_isset,$new_isset" in
-	isset,)
-		echo >&2 "warning: $old_name is now $new_name"
-		echo >&2 "hint: set $new_name too during the transition period"
-		eval "$new_name=\$$old_name"
-		;;
-	isset,isset)
-		# do this later
-		# echo >&2 "warning: $old_name is now $new_name"
-		# echo >&2 "hint: remove $old_name"
-		;;
-	esac
-}
-
-check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
-check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
-check_var_migration GIT_FORCE_PRELOAD_TEST GIT_TEST_PRELOAD_INDEX
-
 # Use specific version of the index file format
 if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
 then
-- 
2.30.0.284.gd98b1dd5eaa7

Re: [PATCH 01/12] test-lib: remove check_var_migration

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Remove the check_var_migration() migration helper. This was added back
> in [1], [2] and [3] to warn users to migrate from e.g. the
> "GIT_FSMONITOR_TEST" name to "GIT_TEST_FSMONITOR".
>
> I daresay that having been warning about this since late 2018 (or
> v2.20.0) was sufficient time to give everyone interested a heads-up
> about moving to the new names.

As this is not end-user facing backward-compatibility issue, I am
perfectly fine with the abbreviated schedule.

Queued.

[PATCH 02/12] test lib: change "error" to "BUG" as appropriate

[Ævar Arnfjörð Bjarmason]

Change two uses of "error" in test-lib-functions.sh to "BUG".

In the first instance in "test_cmp_rev" the author of the "BUG"
function added in [1] had another in-flight patch adding this in [2],
and the two were never consolidated.

In the second case in "test_atexit" added in [3] that we could have
instead used "BUG" appears to have been missed.

1. 165293af3ce (tests: send "bug in the test script" errors to the
   script's stderr, 2018-11-19)

2. 30d0b6dccbc (test-lib-functions: make 'test_cmp_rev' more
   informative on failure, 2018-11-19)

3. 900721e15c4 (test-lib: introduce 'test_atexit', 2019-03-13)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib-functions.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 6bca0023168..0d40f8a598e 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1105,7 +1105,7 @@ test_cmp_rev () {
 	fi
 	if test $# != 2
 	then
-		error "bug in the test script: test_cmp_rev requires two revisions, but got $#"
+		BUG "test_cmp_rev requires two revisions, but got $#"
 	else
 		local r1 r2
 		r1=$(git rev-parse --verify "$1") &&
@@ -1216,7 +1216,7 @@ test_atexit () {
 	# doing so on Bash is better than nothing (the test will
 	# silently pass on other shells).
 	test "${BASH_SUBSHELL-0}" = 0 ||
-	error "bug in test script: test_atexit does nothing in a subshell"
+	BUG "test_atexit does nothing in a subshell"
 	test_atexit_cleanup="{ $*
 		} && (exit \"\$eval_ret\"); eval_ret=\$?; $test_atexit_cleanup"
 }
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH 03/12] test-lib-functions: move test_set_index_version() to its user

[Ævar Arnfjörð Bjarmason]

Move the test_set_index_version() function to its only user. This
function has only been used in one place since its addition in
5d9fc888b48 (test-lib: allow setting the index format version,
2014-02-23). Let's have that test script define it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t2104-update-index-skip-worktree.sh | 5 +++++
 t/test-lib-functions.sh               | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t2104-update-index-skip-worktree.sh b/t/t2104-update-index-skip-worktree.sh
index 7e2e7dd4ae5..30666fc70d2 100755
--- a/t/t2104-update-index-skip-worktree.sh
+++ b/t/t2104-update-index-skip-worktree.sh
@@ -9,6 +9,11 @@ test_description='skip-worktree bit test'
 
 sane_unset GIT_TEST_SPLIT_INDEX
 
+test_set_index_version () {
+    GIT_INDEX_VERSION="$1"
+    export GIT_INDEX_VERSION
+}
+
 test_set_index_version 3
 
 cat >expect.full <<EOF
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 0d40f8a598e..728bdf94684 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -32,11 +32,6 @@ test_set_editor () {
 	export EDITOR
 }
 
-test_set_index_version () {
-    GIT_INDEX_VERSION="$1"
-    export GIT_INDEX_VERSION
-}
-
 test_decode_color () {
 	awk '
 		function name(n) {
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH 04/12] test-lib-functions: remove generate_zero_bytes() wrapper

[Ævar Arnfjörð Bjarmason]

Since d5cfd142ec1 (tests: teach the test-tool to generate NUL bytes
and use it, 2019-02-14) the generate_zero_bytes() functions has been a
thin wrapper for "test-tool genzeros". Let's have its only user call
that directly instead.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t5318-commit-graph.sh | 2 +-
 t/test-lib-functions.sh | 7 -------
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 2ed0c1544da..368d1cfb448 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -529,7 +529,7 @@ corrupt_graph_and_verify() {
 	zero_pos=${4:-${orig_size}} &&
 	printf "$data" | dd of="$objdir/info/commit-graph" bs=1 seek="$pos" conv=notrunc &&
 	dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" if=/dev/null &&
-	generate_zero_bytes $(($orig_size - $zero_pos)) >>"$objdir/info/commit-graph" &&
+	test-tool genzeros $(($orig_size - $zero_pos)) >>"$objdir/info/commit-graph" &&
 	corrupt_graph_verify "$grepstr"
 
 }
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 728bdf94684..55d58b4a6ac 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -111,13 +111,6 @@ remove_cr () {
 	tr '\015' Q | sed -e 's/Q$//'
 }
 
-# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes).
-# If $1 is 'infinity', output forever or until the receiving pipe stops reading,
-# whichever comes first.
-generate_zero_bytes () {
-	test-tool genzeros "$@"
-}
-
 # In some bourne shell implementations, the "unset" builtin returns
 # nonzero status when a variable to be unset was not set in the first
 # place.
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH 05/12] test libs: rename bundle helper to "lib-bundle.sh"

[Ævar Arnfjörð Bjarmason]

Rename the recently introduced test-bundle-functions.sh to be
consistent with other lib-*.sh files, which is the convention for
these sorts of shared test library functions.

The new test-bundle-functions.sh was introduced in 9901164d81d (test:
add helper functions for git-bundle, 2021-01-11). It was the only
test-*.sh of this nature.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/{test-bundle-functions.sh => lib-bundle.sh} | 0
 t/t5510-fetch.sh                              | 2 +-
 t/t6020-bundle-misc.sh                        | 2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename t/{test-bundle-functions.sh => lib-bundle.sh} (100%)

diff --git a/t/test-bundle-functions.sh b/t/lib-bundle.sh
similarity index 100%
rename from t/test-bundle-functions.sh
rename to t/lib-bundle.sh
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 42f55030047..c1e0eb39231 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -9,7 +9,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/test-bundle-functions.sh
+. "$TEST_DIRECTORY"/lib-bundle.sh
 
 D=$(pwd)
 
diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh
index 6249420a806..da5fe1ba6de 100755
--- a/t/t6020-bundle-misc.sh
+++ b/t/t6020-bundle-misc.sh
@@ -9,7 +9,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/test-bundle-functions.sh
+. "$TEST_DIRECTORY"/lib-bundle.sh
 
 # Create a commit or tag and set the variable with the object ID.
 test_commit_setvar () {
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH 06/12] test libs: rename gitweb-lib.sh to lib-gitweb.sh

[Ævar Arnfjörð Bjarmason]

Rename gitweb-lib.sh to lib-gitweb.sh for consistency with other test
library files.

When it was introduced in 05526071cb5 (gitweb: split test suite into
library and tests, 2009-08-27) this naming pattern was more
common.

Since then all but one other such library which didn't start with
"lib-*.sh" such as t6000lib.sh has been been renamed, see
e.g. 9d488eb40e2 (Move t6000lib.sh to lib-*, 2010-05-07).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/{gitweb-lib.sh => lib-gitweb.sh}        | 0
 t/t9500-gitweb-standalone-no-errors.sh    | 2 +-
 t/t9501-gitweb-standalone-http-status.sh  | 2 +-
 t/t9502-gitweb-standalone-parse-output.sh | 2 +-
 4 files changed, 3 insertions(+), 3 deletions(-)
 rename t/{gitweb-lib.sh => lib-gitweb.sh} (100%)

diff --git a/t/gitweb-lib.sh b/t/lib-gitweb.sh
similarity index 100%
rename from t/gitweb-lib.sh
rename to t/lib-gitweb.sh
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index ee8c6e30e67..0333065d4d6 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -13,7 +13,7 @@ or warnings to log.'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
-. ./gitweb-lib.sh
+. ./lib-gitweb.sh
 
 # ----------------------------------------------------------------------
 # no commits (empty, just initialized repository)
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
index 141610de546..32814e75df5 100755
--- a/t/t9501-gitweb-standalone-http-status.sh
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -13,7 +13,7 @@ code and message.'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
-. ./gitweb-lib.sh
+. ./lib-gitweb.sh
 
 #
 # Gitweb only provides the functionality tested by the 'modification times'
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
index 9cf7ab30a8d..3167473b303 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -13,7 +13,7 @@ in the HTTP header or the actual script output.'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
-. ./gitweb-lib.sh
+. ./lib-gitweb.sh
 
 # ----------------------------------------------------------------------
 # snapshot file name and prefix
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH 07/12] test-lib-functions: move function to lib-bitmap.sh

[Ævar Arnfjörð Bjarmason]

Move a function added to test-lib-functions.sh in ea047a8eb4f (t5310:
factor out bitmap traversal comparison, 2020-02-14) into a new
lib-bitmap.sh.

The test-lib-functions.sh file should be for functions that are widely
used across the test suite, if something's only used by a few tests it
makes more sense to have it in a lib-*.sh file.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/lib-bitmap.sh                    | 26 ++++++++++++++++++++++++++
 t/t5310-pack-bitmaps.sh            |  2 ++
 t/t6113-rev-list-bitmap-filters.sh |  1 +
 t/test-lib-functions.sh            | 27 ---------------------------
 4 files changed, 29 insertions(+), 27 deletions(-)
 create mode 100644 t/lib-bitmap.sh

diff --git a/t/lib-bitmap.sh b/t/lib-bitmap.sh
new file mode 100644
index 00000000000..fe3f98be24f
--- /dev/null
+++ b/t/lib-bitmap.sh
@@ -0,0 +1,26 @@
+# Compare a file containing rev-list bitmap traversal output to its non-bitmap
+# counterpart. You can't just use test_cmp for this, because the two produce
+# subtly different output:
+#
+#   - regular output is in traversal order, whereas bitmap is split by type,
+#     with non-packed objects at the end
+#
+#   - regular output has a space and the pathname appended to non-commit
+#     objects; bitmap output omits this
+#
+# This function normalizes and compares the two. The second file should
+# always be the bitmap output.
+test_bitmap_traversal () {
+	if test "$1" = "--no-confirm-bitmaps"
+	then
+		shift
+	elif cmp "$1" "$2"
+	then
+		echo >&2 "identical raw outputs; are you sure bitmaps were used?"
+		return 1
+	fi &&
+	cut -d' ' -f1 "$1" | sort >"$1.normalized" &&
+	sort "$2" >"$2.normalized" &&
+	test_cmp "$1.normalized" "$2.normalized" &&
+	rm -f "$1.normalized" "$2.normalized"
+}
diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index 5ba76031090..40b9f632441 100755
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -5,6 +5,8 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-bundle.sh
+. "$TEST_DIRECTORY"/lib-bitmap.sh
 
 objpath () {
 	echo ".git/objects/$(echo "$1" | sed -e 's|\(..\)|\1/|')"
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index 2b551e6fd0c..3f889949ca1 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -2,6 +2,7 @@
 
 test_description='rev-list combining bitmaps and filters'
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-bitmap.sh
 
 test_expect_success 'set up bitmapped repo' '
 	# one commit will have bitmaps, the other will not
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 55d58b4a6ac..f228647c2b8 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1602,33 +1602,6 @@ test_set_port () {
 	eval $var=$port
 }
 
-# Compare a file containing rev-list bitmap traversal output to its non-bitmap
-# counterpart. You can't just use test_cmp for this, because the two produce
-# subtly different output:
-#
-#   - regular output is in traversal order, whereas bitmap is split by type,
-#     with non-packed objects at the end
-#
-#   - regular output has a space and the pathname appended to non-commit
-#     objects; bitmap output omits this
-#
-# This function normalizes and compares the two. The second file should
-# always be the bitmap output.
-test_bitmap_traversal () {
-	if test "$1" = "--no-confirm-bitmaps"
-	then
-		shift
-	elif cmp "$1" "$2"
-	then
-		echo >&2 "identical raw outputs; are you sure bitmaps were used?"
-		return 1
-	fi &&
-	cut -d' ' -f1 "$1" | sort >"$1.normalized" &&
-	sort "$2" >"$2.normalized" &&
-	test_cmp "$1.normalized" "$2.normalized" &&
-	rm -f "$1.normalized" "$2.normalized"
-}
-
 # Tests for the hidden file attribute on Windows
 test_path_is_hidden () {
 	test_have_prereq MINGW ||
-- 
2.30.0.284.gd98b1dd5eaa7

Re: [PATCH 07/12] test-lib-functions: move function to lib-bitmap.sh

[SZEDER Gábor]

On Tue, Feb 09, 2021 at 10:41:54PM +0100, Ævar Arnfjörð Bjarmason wrote:
> Move a function added to test-lib-functions.sh in ea047a8eb4f (t5310:
> factor out bitmap traversal comparison, 2020-02-14) into a new
> lib-bitmap.sh.
> 
> The test-lib-functions.sh file should be for functions that are widely
> used across the test suite, if something's only used by a few tests it
> makes more sense to have it in a lib-*.sh file.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  t/lib-bitmap.sh                    | 26 ++++++++++++++++++++++++++
>  t/t5310-pack-bitmaps.sh            |  2 ++
>  t/t6113-rev-list-bitmap-filters.sh |  1 +
>  t/test-lib-functions.sh            | 27 ---------------------------
>  4 files changed, 29 insertions(+), 27 deletions(-)
>  create mode 100644 t/lib-bitmap.sh
> 
> diff --git a/t/lib-bitmap.sh b/t/lib-bitmap.sh
> new file mode 100644
> index 00000000000..fe3f98be24f
> --- /dev/null
> +++ b/t/lib-bitmap.sh
> @@ -0,0 +1,26 @@
> +# Compare a file containing rev-list bitmap traversal output to its non-bitmap
> +# counterpart. You can't just use test_cmp for this, because the two produce
> +# subtly different output:
> +#
> +#   - regular output is in traversal order, whereas bitmap is split by type,
> +#     with non-packed objects at the end
> +#
> +#   - regular output has a space and the pathname appended to non-commit
> +#     objects; bitmap output omits this
> +#
> +# This function normalizes and compares the two. The second file should
> +# always be the bitmap output.
> +test_bitmap_traversal () {
> +	if test "$1" = "--no-confirm-bitmaps"
> +	then
> +		shift
> +	elif cmp "$1" "$2"
> +	then
> +		echo >&2 "identical raw outputs; are you sure bitmaps were used?"
> +		return 1

Since you converted two 'error's to BUG earlier in this series...

This helper function was added in ea047a8eb4 (t5310: factor out bitmap
traversal comparison, 2020-02-14), and so I Cc: its author and quote
part of its commit message:

    While we're at it, let's also try to confirm that the bitmap output did
    indeed use bitmaps. Since the code internally falls back to the
    non-bitmap path in some cases, the tests are at risk of becoming trivial
    noops.
    
    This is a bit fragile, as not all outputs will differ (e.g., looking at
    only the commits from a fully-bitmapped pack will end up exactly the
    same as the normal traversal order, since it also matches the pack
    order). So we'll provide an escape hatch by which tests can disable this
    check (which should only be used after manually confirming that bitmaps
    kicked in).

Shouldn't this be a BUG as well?  I think this should be a BUG.

> +	fi &&
> +	cut -d' ' -f1 "$1" | sort >"$1.normalized" &&
> +	sort "$2" >"$2.normalized" &&
> +	test_cmp "$1.normalized" "$2.normalized" &&
> +	rm -f "$1.normalized" "$2.normalized"
> +}
> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
> index 5ba76031090..40b9f632441 100755
> --- a/t/t5310-pack-bitmaps.sh
> +++ b/t/t5310-pack-bitmaps.sh
> @@ -5,6 +5,8 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  
>  . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-bundle.sh
> +. "$TEST_DIRECTORY"/lib-bitmap.sh
>  
>  objpath () {
>  	echo ".git/objects/$(echo "$1" | sed -e 's|\(..\)|\1/|')"
> diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
> index 2b551e6fd0c..3f889949ca1 100755
> --- a/t/t6113-rev-list-bitmap-filters.sh
> +++ b/t/t6113-rev-list-bitmap-filters.sh
> @@ -2,6 +2,7 @@
>  
>  test_description='rev-list combining bitmaps and filters'
>  . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-bitmap.sh
>  
>  test_expect_success 'set up bitmapped repo' '
>  	# one commit will have bitmaps, the other will not
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 55d58b4a6ac..f228647c2b8 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -1602,33 +1602,6 @@ test_set_port () {
>  	eval $var=$port
>  }
>  
> -# Compare a file containing rev-list bitmap traversal output to its non-bitmap
> -# counterpart. You can't just use test_cmp for this, because the two produce
> -# subtly different output:
> -#
> -#   - regular output is in traversal order, whereas bitmap is split by type,
> -#     with non-packed objects at the end
> -#
> -#   - regular output has a space and the pathname appended to non-commit
> -#     objects; bitmap output omits this
> -#
> -# This function normalizes and compares the two. The second file should
> -# always be the bitmap output.
> -test_bitmap_traversal () {
> -	if test "$1" = "--no-confirm-bitmaps"
> -	then
> -		shift
> -	elif cmp "$1" "$2"
> -	then
> -		echo >&2 "identical raw outputs; are you sure bitmaps were used?"
> -		return 1
> -	fi &&
> -	cut -d' ' -f1 "$1" | sort >"$1.normalized" &&
> -	sort "$2" >"$2.normalized" &&
> -	test_cmp "$1.normalized" "$2.normalized" &&
> -	rm -f "$1.normalized" "$2.normalized"
> -}
> -
>  # Tests for the hidden file attribute on Windows
>  test_path_is_hidden () {
>  	test_have_prereq MINGW ||
> -- 
> 2.30.0.284.gd98b1dd5eaa7
> 

Re: [PATCH 07/12] test-lib-functions: move function to lib-bitmap.sh

[Jeff King]

On Wed, Feb 10, 2021 at 09:56:45PM +0100, SZEDER Gábor wrote:

> > +test_bitmap_traversal () {
> > +	if test "$1" = "--no-confirm-bitmaps"
> > +	then
> > +		shift
> > +	elif cmp "$1" "$2"
> > +	then
> > +		echo >&2 "identical raw outputs; are you sure bitmaps were used?"
> > +		return 1
> 
> Since you converted two 'error's to BUG earlier in this series...
> 
> This helper function was added in ea047a8eb4 (t5310: factor out bitmap
> traversal comparison, 2020-02-14), and so I Cc: its author and quote
> part of its commit message:
> 
>     While we're at it, let's also try to confirm that the bitmap output did
>     indeed use bitmaps. Since the code internally falls back to the
>     non-bitmap path in some cases, the tests are at risk of becoming trivial
>     noops.
>     
>     This is a bit fragile, as not all outputs will differ (e.g., looking at
>     only the commits from a fully-bitmapped pack will end up exactly the
>     same as the normal traversal order, since it also matches the pack
>     order). So we'll provide an escape hatch by which tests can disable this
>     check (which should only be used after manually confirming that bitmaps
>     kicked in).
> 
> Shouldn't this be a BUG as well?  I think this should be a BUG.

I dunno. I guess you are thinking of the case where somebody adds a new
caller but fails to invoke Git correctly. Which would be a bug in the
test script.

But it could also easily be due to Git changing how it behaves in a
certain circumstance. And maybe the solution is changing the test to
adapt, or maybe we just found a bug in Git.

TBH, I do not care that much either way. Either way, I think somebody
would notice the problem.

-Peff

Re: [PATCH 07/12] test-lib-functions: move function to lib-bitmap.sh

[SZEDER Gábor]

On Wed, Feb 10, 2021 at 04:10:58PM -0500, Jeff King wrote:
> On Wed, Feb 10, 2021 at 09:56:45PM +0100, SZEDER Gábor wrote:
> 
> > > +test_bitmap_traversal () {
> > > +	if test "$1" = "--no-confirm-bitmaps"
> > > +	then
> > > +		shift
> > > +	elif cmp "$1" "$2"
> > > +	then
> > > +		echo >&2 "identical raw outputs; are you sure bitmaps were used?"
> > > +		return 1
> > 
> > Since you converted two 'error's to BUG earlier in this series...
> > 
> > This helper function was added in ea047a8eb4 (t5310: factor out bitmap
> > traversal comparison, 2020-02-14), and so I Cc: its author and quote
> > part of its commit message:
> > 
> >     While we're at it, let's also try to confirm that the bitmap output did
> >     indeed use bitmaps. Since the code internally falls back to the
> >     non-bitmap path in some cases, the tests are at risk of becoming trivial
> >     noops.
> >     
> >     This is a bit fragile, as not all outputs will differ (e.g., looking at
> >     only the commits from a fully-bitmapped pack will end up exactly the
> >     same as the normal traversal order, since it also matches the pack
> >     order). So we'll provide an escape hatch by which tests can disable this
> >     check (which should only be used after manually confirming that bitmaps
> >     kicked in).
> > 
> > Shouldn't this be a BUG as well?  I think this should be a BUG.
> 
> I dunno. I guess you are thinking of the case where somebody adds a new
> caller but fails to invoke Git correctly. Which would be a bug in the
> test script.
> 
> But it could also easily be due to Git changing how it behaves in a
> certain circumstance. And maybe the solution is changing the test to
> adapt, or maybe we just found a bug in Git.

Well, I though that if the raw output is the same with and without
pack bitmaps, then the missing '--no-confirm-bitmaps' option is the
bug in the test script.

> TBH, I do not care that much either way. Either way, I think somebody
> would notice the problem.

Me neither, I just don't like test helper functions that simply return
in the middle, becauase that makes suppressing their trace output even
more awkward than it already is :)

[PATCH 08/12] t/.gitattributes: sort lines

[Ævar Arnfjörð Bjarmason]

Sort the lines starting with "/", the only out-of-place line was added
along with most of the file in 614f4f0f350 (Fix the remaining tests
that failed with core.autocrlf=true, 2017-05-09).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/.gitattributes | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/.gitattributes b/t/.gitattributes
index df05434d32c..d778bfad052 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -5,9 +5,9 @@ t[0-9][0-9][0-9][0-9]/* -whitespace
 /t3206/* eol=lf
 /t3900/*.txt eol=lf
 /t3901/*.txt eol=lf
-/t4034/*/* eol=lf
 /t4013/* eol=lf
 /t4018/* eol=lf
+/t4034/*/* eol=lf
 /t4051/* eol=lf
 /t4100/* eol=lf
 /t4101/* eol=lf
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH 09/12] test libs: rename "diff-lib" to "lib-diff"

[Ævar Arnfjörð Bjarmason]

Rename the "diff-lib" to "lib-diff". With this rename and preceding
commits there is no remaining t/*lib* which doesn't follow the
convention of being called t/lib-*.
---
 t/.gitattributes                      | 2 +-
 t/{diff-lib.sh => lib-diff.sh}        | 0
 t/{diff-lib => lib-diff}/COPYING      | 0
 t/{diff-lib => lib-diff}/README       | 0
 t/t4000-diff-format.sh                | 2 +-
 t/t4001-diff-rename.sh                | 2 +-
 t/t4003-diff-rename-1.sh              | 6 +++---
 t/t4004-diff-rename-symlink.sh        | 2 +-
 t/t4005-diff-rename-2.sh              | 6 +++---
 t/t4007-rename-3.sh                   | 6 +++---
 t/t4008-diff-break-rewrite.sh         | 6 +++---
 t/t4009-diff-rename-4.sh              | 6 +++---
 t/t4010-diff-pathspec.sh              | 2 +-
 t/t4011-diff-symlink.sh               | 2 +-
 t/t4013-diff-various.sh               | 2 +-
 t/t4015-diff-whitespace.sh            | 2 +-
 t/t4027-diff-submodule.sh             | 2 +-
 t/t4034-diff-words.sh                 | 2 +-
 t/t4038-diff-combined.sh              | 2 +-
 t/t4061-diff-indent.sh                | 2 +-
 t/t4206-log-follow-harder-copies.sh   | 2 +-
 t/t7501-commit-basic-functionality.sh | 2 +-
 t/t9300-fast-import.sh                | 2 +-
 23 files changed, 30 insertions(+), 30 deletions(-)
 rename t/{diff-lib.sh => lib-diff.sh} (100%)
 rename t/{diff-lib => lib-diff}/COPYING (100%)
 rename t/{diff-lib => lib-diff}/README (100%)

diff --git a/t/.gitattributes b/t/.gitattributes
index d778bfad052..dafa17c3e61 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -1,6 +1,6 @@
 t[0-9][0-9][0-9][0-9]/* -whitespace
 /chainlint/*.expect eol=lf
-/diff-lib/* eol=lf
+/lib-diff/* eol=lf
 /t0110/url-* binary
 /t3206/* eol=lf
 /t3900/*.txt eol=lf
diff --git a/t/diff-lib.sh b/t/lib-diff.sh
similarity index 100%
rename from t/diff-lib.sh
rename to t/lib-diff.sh
diff --git a/t/diff-lib/COPYING b/t/lib-diff/COPYING
similarity index 100%
rename from t/diff-lib/COPYING
rename to t/lib-diff/COPYING
diff --git a/t/diff-lib/README b/t/lib-diff/README
similarity index 100%
rename from t/diff-lib/README
rename to t/lib-diff/README
diff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
index e5116a76a1c..cce334981e1 100755
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -7,7 +7,7 @@ test_description='Test built-in diff output engine.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 echo >path0 'Line 1
 Line 2
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index c16486a9d41..2f9700742aa 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -7,7 +7,7 @@ test_description='Test rename detection in diff engine.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 test_expect_success 'setup' '
 	cat >path0 <<-\EOF &&
diff --git a/t/t4003-diff-rename-1.sh b/t/t4003-diff-rename-1.sh
index df2accb6555..db07ff3eb19 100755
--- a/t/t4003-diff-rename-1.sh
+++ b/t/t4003-diff-rename-1.sh
@@ -7,11 +7,11 @@ test_description='More rename detection
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 test_expect_success \
     'prepare reference tree' \
-    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+    'cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
      echo frotz >rezrov &&
     git update-index --add COPYING rezrov &&
     tree=$(git write-tree) &&
@@ -99,7 +99,7 @@ test_expect_success \
 
 test_expect_success \
     'prepare work tree once again' \
-    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+    'cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
      git update-index --add --remove COPYING COPYING.1'
 
 # tree has COPYING and rezrov.  work tree has COPYING and COPYING.1,
diff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh
index 6e562c80d12..3d495e37bb1 100755
--- a/t/t4004-diff-rename-symlink.sh
+++ b/t/t4004-diff-rename-symlink.sh
@@ -10,7 +10,7 @@ copy of symbolic links, but should not produce rename/copy followed
 by an edit for them.
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 test_expect_success SYMLINKS \
     'prepare reference tree' \
diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh
index d18a80493c2..86479061325 100755
--- a/t/t4005-diff-rename-2.sh
+++ b/t/t4005-diff-rename-2.sh
@@ -6,10 +6,10 @@
 test_description='Same rename detection as t4003 but testing diff-raw.'
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 test_expect_success 'setup reference tree' '
-	cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+	cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
 	echo frotz >rezrov &&
 	git update-index --add COPYING rezrov &&
 	tree=$(git write-tree) &&
@@ -64,7 +64,7 @@ test_expect_success 'validate output from rename/copy detection (#2)' '
 # nows how to say Copy.
 
 test_expect_success 'validate output from rename/copy detection (#3)' '
-	cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+	cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
 	git update-index --add --remove COPYING COPYING.1 &&
 
 	cat <<-EOF >expected &&
diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh
index b187b7f6c66..cbb9c62f535 100755
--- a/t/t4007-rename-3.sh
+++ b/t/t4007-rename-3.sh
@@ -7,17 +7,17 @@ test_description='Rename interaction with pathspec.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 test_expect_success 'prepare reference tree' '
 	mkdir path0 path1 &&
-	cp "$TEST_DIRECTORY"/diff-lib/COPYING path0/COPYING &&
+	cp "$TEST_DIRECTORY"/lib-diff/COPYING path0/COPYING &&
 	git update-index --add path0/COPYING &&
 	tree=$(git write-tree) &&
 	echo $tree
 '
 
-blob=$(git hash-object "$TEST_DIRECTORY/diff-lib/COPYING")
+blob=$(git hash-object "$TEST_DIRECTORY/lib-diff/COPYING")
 test_expect_success 'prepare work tree' '
 	cp path0/COPYING path1/COPYING &&
 	git update-index --add --remove path0/COPYING path1/COPYING
diff --git a/t/t4008-diff-break-rewrite.sh b/t/t4008-diff-break-rewrite.sh
index b1ccd4102e0..2299f27511b 100755
--- a/t/t4008-diff-break-rewrite.sh
+++ b/t/t4008-diff-break-rewrite.sh
@@ -22,11 +22,11 @@ With -B, this should be detected as two complete rewrites.
 Further, with -B and -M together, these should turn into two renames.
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 test_expect_success setup '
-	cat "$TEST_DIRECTORY"/diff-lib/README >file0 &&
-	cat "$TEST_DIRECTORY"/diff-lib/COPYING >file1 &&
+	cat "$TEST_DIRECTORY"/lib-diff/README >file0 &&
+	cat "$TEST_DIRECTORY"/lib-diff/COPYING >file1 &&
 	blob0_id=$(git hash-object file0) &&
 	blob1_id=$(git hash-object file1) &&
 	git update-index --add file0 file1 &&
diff --git a/t/t4009-diff-rename-4.sh b/t/t4009-diff-rename-4.sh
index b63bdf031f5..b1da807f169 100755
--- a/t/t4009-diff-rename-4.sh
+++ b/t/t4009-diff-rename-4.sh
@@ -7,11 +7,11 @@ test_description='Same rename detection as t4003 but testing diff-raw -z.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 test_expect_success \
     'prepare reference tree' \
-    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+    'cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
      echo frotz >rezrov &&
     git update-index --add COPYING rezrov &&
     orig=$(git hash-object COPYING) &&
@@ -81,7 +81,7 @@ test_expect_success \
 
 test_expect_success \
     'prepare work tree once again' \
-    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+    'cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
      git update-index --add --remove COPYING COPYING.1'
 
 git diff-index -z -C --find-copies-harder $tree >current
diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
index 65cc703c659..1bbced79ece 100755
--- a/t/t4010-diff-pathspec.sh
+++ b/t/t4010-diff-pathspec.sh
@@ -10,7 +10,7 @@ Prepare:
         path1/file1
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 test_expect_success \
     setup \
diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh
index 717034bb50b..5a25c259fe3 100755
--- a/t/t4011-diff-symlink.sh
+++ b/t/t4011-diff-symlink.sh
@@ -7,7 +7,7 @@ test_description='Test diff of symlinks.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 # Print the short OID of a symlink with the given name.
 symlink_oid () {
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index ce6aa3914fe..fcc30d8cc26 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -9,7 +9,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 test_expect_success setup '
 
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 8c574221b27..2c13b62d3c6 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -7,7 +7,7 @@ test_description='Test special whitespace in diff engine.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 test_expect_success "Ray Lehtiniemi's example" '
 	cat <<-\EOF >x &&
diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
index 894a11b224d..94ef77e1dfe 100755
--- a/t/t4027-diff-submodule.sh
+++ b/t/t4027-diff-submodule.sh
@@ -3,7 +3,7 @@
 test_description='difference in submodules'
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 test_expect_success setup '
 	test_tick &&
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 0c8fb39ceda..56f1e62a97b 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -3,7 +3,7 @@
 test_description='word diff colors'
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 cat >pre.simple <<-\EOF
 	h(4)
diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 09ad491a593..aeac203c424 100755
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -6,7 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 setup_helper () {
 	one=$1 branch=$2 side=$3 &&
diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
index bcf7493740a..7750b87ca16 100755
--- a/t/t4061-diff-indent.sh
+++ b/t/t4061-diff-indent.sh
@@ -7,7 +7,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 # Compare two diff outputs. Ignore "index" lines, because we don't
 # care about SHA-1s or file modes.
diff --git a/t/t4206-log-follow-harder-copies.sh b/t/t4206-log-follow-harder-copies.sh
index ad29e65fcba..4871a5dc92f 100755
--- a/t/t4206-log-follow-harder-copies.sh
+++ b/t/t4206-log-follow-harder-copies.sh
@@ -7,7 +7,7 @@ test_description='Test --follow should always find copies hard in git log.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 echo >path0 'Line 1
 Line 2
diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh
index 0f936182e4f..512ae2781fe 100755
--- a/t/t7501-commit-basic-functionality.sh
+++ b/t/t7501-commit-basic-functionality.sh
@@ -11,7 +11,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY/diff-lib.sh"
+. "$TEST_DIRECTORY/lib-diff.sh"
 
 author='The Real Author <someguy@his.email.org>'
 
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 3d17e932a0e..3d77701fd4d 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -8,7 +8,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 verify_packs () {
 	for p in .git/objects/pack/*.pack
-- 
2.30.0.284.gd98b1dd5eaa7

Re: [PATCH 09/12] test libs: rename "diff-lib" to "lib-diff"

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Rename the "diff-lib" to "lib-diff". With this rename and preceding
> commits there is no remaining t/*lib* which doesn't follow the
> convention of being called t/lib-*.
> ---

Seeing nothing (other than the OID length abstraction that prepares
us for SHA-256 world) has happened since 2009, I have no issue with
this change and no in-flight topics would, either.

Needs a sign-off, though.

Re: [PATCH 09/12] test libs: rename "diff-lib" to "lib-diff"

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 12217 bytes --]

Hi Ævar,

On Tue, 9 Feb 2021, Ævar Arnfjörð Bjarmason wrote:

> diff --git a/t/.gitattributes b/t/.gitattributes
> index d778bfad052..dafa17c3e61 100644
> --- a/t/.gitattributes
> +++ b/t/.gitattributes
> @@ -1,6 +1,6 @@
>  t[0-9][0-9][0-9][0-9]/* -whitespace
>  /chainlint/*.expect eol=lf
> -/diff-lib/* eol=lf
> +/lib-diff/* eol=lf
>  /t0110/url-* binary
>  /t3206/* eol=lf
>  /t3900/*.txt eol=lf

Lucky coincidence that this is still sorted ;-)

The patch looks good to me. Thanks!
Dscho

> diff --git a/t/diff-lib.sh b/t/lib-diff.sh
> similarity index 100%
> rename from t/diff-lib.sh
> rename to t/lib-diff.sh
> diff --git a/t/diff-lib/COPYING b/t/lib-diff/COPYING
> similarity index 100%
> rename from t/diff-lib/COPYING
> rename to t/lib-diff/COPYING
> diff --git a/t/diff-lib/README b/t/lib-diff/README
> similarity index 100%
> rename from t/diff-lib/README
> rename to t/lib-diff/README
> diff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
> index e5116a76a1c..cce334981e1 100755
> --- a/t/t4000-diff-format.sh
> +++ b/t/t4000-diff-format.sh
> @@ -7,7 +7,7 @@ test_description='Test built-in diff output engine.
>
>  '
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh
> +. "$TEST_DIRECTORY"/lib-diff.sh
>
>  echo >path0 'Line 1
>  Line 2
> diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
> index c16486a9d41..2f9700742aa 100755
> --- a/t/t4001-diff-rename.sh
> +++ b/t/t4001-diff-rename.sh
> @@ -7,7 +7,7 @@ test_description='Test rename detection in diff engine.
>
>  '
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh
> +. "$TEST_DIRECTORY"/lib-diff.sh
>
>  test_expect_success 'setup' '
>  	cat >path0 <<-\EOF &&
> diff --git a/t/t4003-diff-rename-1.sh b/t/t4003-diff-rename-1.sh
> index df2accb6555..db07ff3eb19 100755
> --- a/t/t4003-diff-rename-1.sh
> +++ b/t/t4003-diff-rename-1.sh
> @@ -7,11 +7,11 @@ test_description='More rename detection
>
>  '
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
> +. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
>
>  test_expect_success \
>      'prepare reference tree' \
> -    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
> +    'cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
>       echo frotz >rezrov &&
>      git update-index --add COPYING rezrov &&
>      tree=$(git write-tree) &&
> @@ -99,7 +99,7 @@ test_expect_success \
>
>  test_expect_success \
>      'prepare work tree once again' \
> -    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
> +    'cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
>       git update-index --add --remove COPYING COPYING.1'
>
>  # tree has COPYING and rezrov.  work tree has COPYING and COPYING.1,
> diff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh
> index 6e562c80d12..3d495e37bb1 100755
> --- a/t/t4004-diff-rename-symlink.sh
> +++ b/t/t4004-diff-rename-symlink.sh
> @@ -10,7 +10,7 @@ copy of symbolic links, but should not produce rename/copy followed
>  by an edit for them.
>  '
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh
> +. "$TEST_DIRECTORY"/lib-diff.sh
>
>  test_expect_success SYMLINKS \
>      'prepare reference tree' \
> diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh
> index d18a80493c2..86479061325 100755
> --- a/t/t4005-diff-rename-2.sh
> +++ b/t/t4005-diff-rename-2.sh
> @@ -6,10 +6,10 @@
>  test_description='Same rename detection as t4003 but testing diff-raw.'
>
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
> +. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
>
>  test_expect_success 'setup reference tree' '
> -	cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
> +	cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
>  	echo frotz >rezrov &&
>  	git update-index --add COPYING rezrov &&
>  	tree=$(git write-tree) &&
> @@ -64,7 +64,7 @@ test_expect_success 'validate output from rename/copy detection (#2)' '
>  # nows how to say Copy.
>
>  test_expect_success 'validate output from rename/copy detection (#3)' '
> -	cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
> +	cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
>  	git update-index --add --remove COPYING COPYING.1 &&
>
>  	cat <<-EOF >expected &&
> diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh
> index b187b7f6c66..cbb9c62f535 100755
> --- a/t/t4007-rename-3.sh
> +++ b/t/t4007-rename-3.sh
> @@ -7,17 +7,17 @@ test_description='Rename interaction with pathspec.
>
>  '
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
> +. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
>
>  test_expect_success 'prepare reference tree' '
>  	mkdir path0 path1 &&
> -	cp "$TEST_DIRECTORY"/diff-lib/COPYING path0/COPYING &&
> +	cp "$TEST_DIRECTORY"/lib-diff/COPYING path0/COPYING &&
>  	git update-index --add path0/COPYING &&
>  	tree=$(git write-tree) &&
>  	echo $tree
>  '
>
> -blob=$(git hash-object "$TEST_DIRECTORY/diff-lib/COPYING")
> +blob=$(git hash-object "$TEST_DIRECTORY/lib-diff/COPYING")
>  test_expect_success 'prepare work tree' '
>  	cp path0/COPYING path1/COPYING &&
>  	git update-index --add --remove path0/COPYING path1/COPYING
> diff --git a/t/t4008-diff-break-rewrite.sh b/t/t4008-diff-break-rewrite.sh
> index b1ccd4102e0..2299f27511b 100755
> --- a/t/t4008-diff-break-rewrite.sh
> +++ b/t/t4008-diff-break-rewrite.sh
> @@ -22,11 +22,11 @@ With -B, this should be detected as two complete rewrites.
>  Further, with -B and -M together, these should turn into two renames.
>  '
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
> +. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
>
>  test_expect_success setup '
> -	cat "$TEST_DIRECTORY"/diff-lib/README >file0 &&
> -	cat "$TEST_DIRECTORY"/diff-lib/COPYING >file1 &&
> +	cat "$TEST_DIRECTORY"/lib-diff/README >file0 &&
> +	cat "$TEST_DIRECTORY"/lib-diff/COPYING >file1 &&
>  	blob0_id=$(git hash-object file0) &&
>  	blob1_id=$(git hash-object file1) &&
>  	git update-index --add file0 file1 &&
> diff --git a/t/t4009-diff-rename-4.sh b/t/t4009-diff-rename-4.sh
> index b63bdf031f5..b1da807f169 100755
> --- a/t/t4009-diff-rename-4.sh
> +++ b/t/t4009-diff-rename-4.sh
> @@ -7,11 +7,11 @@ test_description='Same rename detection as t4003 but testing diff-raw -z.
>
>  '
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
> +. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
>
>  test_expect_success \
>      'prepare reference tree' \
> -    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
> +    'cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
>       echo frotz >rezrov &&
>      git update-index --add COPYING rezrov &&
>      orig=$(git hash-object COPYING) &&
> @@ -81,7 +81,7 @@ test_expect_success \
>
>  test_expect_success \
>      'prepare work tree once again' \
> -    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
> +    'cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
>       git update-index --add --remove COPYING COPYING.1'
>
>  git diff-index -z -C --find-copies-harder $tree >current
> diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
> index 65cc703c659..1bbced79ece 100755
> --- a/t/t4010-diff-pathspec.sh
> +++ b/t/t4010-diff-pathspec.sh
> @@ -10,7 +10,7 @@ Prepare:
>          path1/file1
>  '
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
> +. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
>
>  test_expect_success \
>      setup \
> diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh
> index 717034bb50b..5a25c259fe3 100755
> --- a/t/t4011-diff-symlink.sh
> +++ b/t/t4011-diff-symlink.sh
> @@ -7,7 +7,7 @@ test_description='Test diff of symlinks.
>
>  '
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh
> +. "$TEST_DIRECTORY"/lib-diff.sh
>
>  # Print the short OID of a symlink with the given name.
>  symlink_oid () {
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index ce6aa3914fe..fcc30d8cc26 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -9,7 +9,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh
> +. "$TEST_DIRECTORY"/lib-diff.sh
>
>  test_expect_success setup '
>
> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
> index 8c574221b27..2c13b62d3c6 100755
> --- a/t/t4015-diff-whitespace.sh
> +++ b/t/t4015-diff-whitespace.sh
> @@ -7,7 +7,7 @@ test_description='Test special whitespace in diff engine.
>
>  '
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh
> +. "$TEST_DIRECTORY"/lib-diff.sh
>
>  test_expect_success "Ray Lehtiniemi's example" '
>  	cat <<-\EOF >x &&
> diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
> index 894a11b224d..94ef77e1dfe 100755
> --- a/t/t4027-diff-submodule.sh
> +++ b/t/t4027-diff-submodule.sh
> @@ -3,7 +3,7 @@
>  test_description='difference in submodules'
>
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh
> +. "$TEST_DIRECTORY"/lib-diff.sh
>
>  test_expect_success setup '
>  	test_tick &&
> diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
> index 0c8fb39ceda..56f1e62a97b 100755
> --- a/t/t4034-diff-words.sh
> +++ b/t/t4034-diff-words.sh
> @@ -3,7 +3,7 @@
>  test_description='word diff colors'
>
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh
> +. "$TEST_DIRECTORY"/lib-diff.sh
>
>  cat >pre.simple <<-\EOF
>  	h(4)
> diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
> index 09ad491a593..aeac203c424 100755
> --- a/t/t4038-diff-combined.sh
> +++ b/t/t4038-diff-combined.sh
> @@ -6,7 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh
> +. "$TEST_DIRECTORY"/lib-diff.sh
>
>  setup_helper () {
>  	one=$1 branch=$2 side=$3 &&
> diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
> index bcf7493740a..7750b87ca16 100755
> --- a/t/t4061-diff-indent.sh
> +++ b/t/t4061-diff-indent.sh
> @@ -7,7 +7,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh
> +. "$TEST_DIRECTORY"/lib-diff.sh
>
>  # Compare two diff outputs. Ignore "index" lines, because we don't
>  # care about SHA-1s or file modes.
> diff --git a/t/t4206-log-follow-harder-copies.sh b/t/t4206-log-follow-harder-copies.sh
> index ad29e65fcba..4871a5dc92f 100755
> --- a/t/t4206-log-follow-harder-copies.sh
> +++ b/t/t4206-log-follow-harder-copies.sh
> @@ -7,7 +7,7 @@ test_description='Test --follow should always find copies hard in git log.
>
>  '
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh
> +. "$TEST_DIRECTORY"/lib-diff.sh
>
>  echo >path0 'Line 1
>  Line 2
> diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh
> index 0f936182e4f..512ae2781fe 100755
> --- a/t/t7501-commit-basic-functionality.sh
> +++ b/t/t7501-commit-basic-functionality.sh
> @@ -11,7 +11,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY/diff-lib.sh"
> +. "$TEST_DIRECTORY/lib-diff.sh"
>
>  author='The Real Author <someguy@his.email.org>'
>
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index 3d17e932a0e..3d77701fd4d 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -8,7 +8,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
>  . ./test-lib.sh
> -. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
> +. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
>
>  verify_packs () {
>  	for p in .git/objects/pack/*.pack
> --
> 2.30.0.284.gd98b1dd5eaa7
>
>

Re: [PATCH 09/12] test libs: rename "diff-lib" to "lib-diff"

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Ævar,
>
> On Tue, 9 Feb 2021, Ævar Arnfjörð Bjarmason wrote:
>
>> diff --git a/t/.gitattributes b/t/.gitattributes
>> index d778bfad052..dafa17c3e61 100644
>> --- a/t/.gitattributes
>> +++ b/t/.gitattributes
>> @@ -1,6 +1,6 @@
>>  t[0-9][0-9][0-9][0-9]/* -whitespace
>>  /chainlint/*.expect eol=lf
>> -/diff-lib/* eol=lf
>> +/lib-diff/* eol=lf
>>  /t0110/url-* binary
>>  /t3206/* eol=lf
>>  /t3900/*.txt eol=lf
>
> Lucky coincidence that this is still sorted ;-)
>
> The patch looks good to me. Thanks!
> Dscho

Yup.  It still needs to be signed off.

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

[Ævar Arnfjörð Bjarmason]

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

[PATCH 11/12] test-lib-functions: assert correct parameter count

[Ævar Arnfjörð Bjarmason]

Add assertions of the correct parameter count of various functions, in
particularly the wrappers for the shell "test" built-in.

In an earlier commit we fixed a bug with an incorrect number of
arguments being passed to "test_path_is_{file,missing}". Let's also
guard other similar functions from the same sort of misuse.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib-functions.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 109d1b548ce..8ec06eb67a0 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -728,6 +728,7 @@ test_path_is_file () {
 }
 
 test_path_is_dir () {
+	test "$#" -ne 1 && BUG "1 param"
 	if ! test -d "$1"
 	then
 		echo "Directory $1 doesn't exist"
@@ -746,6 +747,7 @@ test_path_exists () {
 
 # Check if the directory exists and is empty as expected, barf otherwise.
 test_dir_is_empty () {
+	test "$#" -ne 1 && BUG "1 param"
 	test_path_is_dir "$1" &&
 	if test -n "$(ls -a1 "$1" | egrep -v '^\.\.?$')"
 	then
@@ -757,6 +759,7 @@ test_dir_is_empty () {
 
 # Check if the file exists and has a size greater than zero
 test_file_not_empty () {
+	test "$#" = 2 && BUG "2 param"
 	if ! test -s "$1"
 	then
 		echo "'$1' is not a non-empty file."
@@ -765,6 +768,7 @@ test_file_not_empty () {
 }
 
 test_path_is_missing () {
+	test "$#" -ne 1 && BUG "1 param"
 	if test -e "$1"
 	then
 		echo "Path exists:"
@@ -801,6 +805,7 @@ test_line_count () {
 }
 
 test_file_size () {
+	test "$#" -ne 1 && BUG "1 param"
 	test-tool path-utils file-size "$1"
 }
 
@@ -973,6 +978,7 @@ test_expect_code () {
 # - not all diff versions understand "-u"
 
 test_cmp () {
+	test "$#" -ne 2 && BUG "2 param"
 	eval "$GIT_TEST_CMP" '"$@"'
 }
 
@@ -1002,6 +1008,7 @@ test_cmp_config () {
 # test_cmp_bin - helper to compare binary files
 
 test_cmp_bin () {
+	test "$#" -ne 2 && BUG "2 param"
 	cmp "$@"
 }
 
@@ -1071,6 +1078,7 @@ verbose () {
 # otherwise.
 
 test_must_be_empty () {
+	test "$#" -ne 1 && BUG "1 param"
 	test_path_is_file "$1" &&
 	if test -s "$1"
 	then
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH 12/12] test-lib-functions: split out {debug,path,text} helpers

[Ævar Arnfjörð Bjarmason]

Split out the debug, path and text miscellaneous helpers into their
own files. The "test -[efsd]" etc. helpers were not all in one place
in test-lib-functions.sh, I think moving them into their own file
makes it easier to follow the code.

The same goes for the miscellaneous text munging helpers, and the
debug code that was early in the file.

As viewing this commit with diff.colorMoved=true will show there's no
functional changes here. The only added/removed lines are comments and
whitespace. The rest is all moving of existing code into new files.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib-functions.sh       | 267 +---------------------------------
 t/test-lib-functions/debug.sh |  99 +++++++++++++
 t/test-lib-functions/path.sh  | 137 +++++++++++++++++
 t/test-lib-functions/text.sh  |  33 +++++
 4 files changed, 275 insertions(+), 261 deletions(-)
 create mode 100644 t/test-lib-functions/debug.sh
 create mode 100644 t/test-lib-functions/path.sh
 create mode 100644 t/test-lib-functions/text.sh

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8ec06eb67a0..a21ba83d83c 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -16,6 +16,12 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see http://www.gnu.org/licenses/ .
 
+# Include sub-libraries of test-lib-functions.sh, split out for
+# readability.
+. "$TEST_DIRECTORY/test-lib-functions/debug.sh"
+. "$TEST_DIRECTORY/test-lib-functions/path.sh"
+. "$TEST_DIRECTORY/test-lib-functions/text.sh"
+
 # The semantics of the editor variables are that of invoking
 # sh -c "$EDITOR \"$@\"" files ...
 #
@@ -79,37 +85,7 @@ test_decode_color () {
 	'
 }
 
-lf_to_nul () {
-	perl -pe 'y/\012/\000/'
-}
-
-nul_to_q () {
-	perl -pe 'y/\000/Q/'
-}
-
-q_to_nul () {
-	perl -pe 'y/Q/\000/'
-}
-
-q_to_cr () {
-	tr Q '\015'
-}
-
-q_to_tab () {
-	tr Q '\011'
-}
-
-qz_to_tab_space () {
-	tr QZ '\011\040'
-}
-
-append_cr () {
-	sed -e 's/$/Q/' | tr Q '\015'
-}
 
-remove_cr () {
-	tr '\015' Q | sed -e 's/Q$//'
-}
 
 # In some bourne shell implementations, the "unset" builtin returns
 # nonzero status when a variable to be unset was not set in the first
@@ -134,38 +110,6 @@ test_tick () {
 	export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
 }
 
-# Stop execution and start a shell. This is useful for debugging tests.
-#
-# Be sure to remove all invocations of this command before submitting.
-
-test_pause () {
-	"$SHELL_PATH" <&6 >&5 2>&7
-}
-
-# Wrap git with a debugger. Adding this to a command can make it easier
-# to understand what is going on in a failing test.
-#
-# Examples:
-#     debug git checkout master
-#     debug --debugger=nemiver git $ARGS
-#     debug -d "valgrind --tool=memcheck --track-origins=yes" git $ARGS
-debug () {
-	case "$1" in
-	-d)
-		GIT_DEBUGGER="$2" &&
-		shift 2
-		;;
-	--debugger=*)
-		GIT_DEBUGGER="${1#*=}" &&
-		shift 1
-		;;
-	*)
-		GIT_DEBUGGER=1
-		;;
-	esac &&
-	GIT_DEBUGGER="${GIT_DEBUGGER}" "$@" <&6 >&5 2>&7
-}
-
 # Usage: test_commit [options] <message> [<file> [<contents> [<tag>]]]
 #   -C <dir>:
 #	Run all git commands in directory <dir>
@@ -369,25 +313,6 @@ test_commit_bulk () {
 
 }
 
-# This function helps systems where core.filemode=false is set.
-# Use it instead of plain 'chmod +x' to set or unset the executable bit
-# of a file in the working directory and add it to the index.
-
-test_chmod () {
-	chmod "$@" &&
-	git update-index --add "--chmod=$@"
-}
-
-# Get the modebits from a file or directory, ignoring the setgid bit (g+s).
-# This bit is inherited by subdirectories at their creation. So we remove it
-# from the returning string to prevent callers from having to worry about the
-# state of the bit in the test directory.
-#
-test_modebits () {
-	ls -ld "$1" | sed -e 's|^\(..........\).*|\1|' \
-			  -e 's|^\(......\)S|\1-|' -e 's|^\(......\)s|\1x|'
-}
-
 # Unset a configuration variable, but don't fail if it doesn't exist.
 test_unconfig () {
 	config_dir=
@@ -716,99 +641,6 @@ test_external_without_stderr () {
 	fi
 }
 
-# debugging-friendly alternatives to "test [-f|-d|-e]"
-# 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"
-		false
-	fi
-}
-
-test_path_is_dir () {
-	test "$#" -ne 1 && BUG "1 param"
-	if ! test -d "$1"
-	then
-		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"
-		false
-	fi
-}
-
-# Check if the directory exists and is empty as expected, barf otherwise.
-test_dir_is_empty () {
-	test "$#" -ne 1 && BUG "1 param"
-	test_path_is_dir "$1" &&
-	if test -n "$(ls -a1 "$1" | egrep -v '^\.\.?$')"
-	then
-		echo "Directory '$1' is not empty, it contains:"
-		ls -la "$1"
-		return 1
-	fi
-}
-
-# Check if the file exists and has a size greater than zero
-test_file_not_empty () {
-	test "$#" = 2 && BUG "2 param"
-	if ! test -s "$1"
-	then
-		echo "'$1' is not a non-empty file."
-		false
-	fi
-}
-
-test_path_is_missing () {
-	test "$#" -ne 1 && BUG "1 param"
-	if test -e "$1"
-	then
-		echo "Path exists:"
-		ls -ld "$1"
-		if test $# -ge 1
-		then
-			echo "$*"
-		fi
-		false
-	fi
-}
-
-# test_line_count checks that a file has the number of lines it
-# ought to. For example:
-#
-#	test_expect_success 'produce exactly one line of output' '
-#		do something >output &&
-#		test_line_count = 1 output
-#	'
-#
-# is like "test $(wc -l <output) = 1" except that it passes the
-# output through when the number of lines is wrong.
-
-test_line_count () {
-	if test $# != 3
-	then
-		BUG "not 3 parameters to test_line_count"
-	elif ! test $(wc -l <"$3") "$1" "$2"
-	then
-		echo "test_line_count: line count for $3 !$1 $2"
-		cat "$3"
-		return 1
-	fi
-}
-
-test_file_size () {
-	test "$#" -ne 1 && BUG "1 param"
-	test-tool path-utils file-size "$1"
-}
-
 # Returns success if a comma separated string of keywords ($1) contains a
 # given keyword ($2).
 # Examples:
@@ -1074,20 +906,6 @@ verbose () {
 	return 1
 }
 
-# Check if the file expected to be empty is indeed empty, and barfs
-# otherwise.
-
-test_must_be_empty () {
-	test "$#" -ne 1 && BUG "1 param"
-	test_path_is_file "$1" &&
-	if test -s "$1"
-	then
-		echo "'$1' is not empty, it contains:"
-		cat "$1"
-		return 1
-	fi
-}
-
 # Tests that its two parameters refer to the same revision, or if '!' is
 # provided first, that its other two parameters refer to different
 # revisions.
@@ -1411,21 +1229,6 @@ test_match_signal () {
 	return 1
 }
 
-# Read up to "$1" bytes (or to EOF) from stdin and write them to stdout.
-test_copy_bytes () {
-	perl -e '
-		my $len = $ARGV[1];
-		while ($len > 0) {
-			my $s;
-			my $nread = sysread(STDIN, $s, $len);
-			die "cannot read: $!" unless defined($nread);
-			last unless $nread;
-			print $s;
-			$len -= $nread;
-		}
-	' - "$1"
-}
-
 # run "$@" inside a non-git directory
 nongit () {
 	test -d non-repo ||
@@ -1440,54 +1243,6 @@ nongit () {
 	)
 } 7>&2 2>&4
 
-# convert function arguments or stdin (if not arguments given) to pktline
-# representation. If multiple arguments are given, they are separated by
-# whitespace and put in a single packet. Note that data containing NULs must be
-# given on stdin, and that empty input becomes an empty packet, not a flush
-# packet (for that you can just print 0000 yourself).
-packetize () {
-	if test $# -gt 0
-	then
-		packet="$*"
-		printf '%04x%s' "$((4 + ${#packet}))" "$packet"
-	else
-		perl -e '
-			my $packet = do { local $/; <STDIN> };
-			printf "%04x%s", 4 + length($packet), $packet;
-		'
-	fi
-}
-
-# Parse the input as a series of pktlines, writing the result to stdout.
-# Sideband markers are removed automatically, and the output is routed to
-# stderr if appropriate.
-#
-# NUL bytes are converted to "\\0" for ease of parsing with text tools.
-depacketize () {
-	perl -e '
-		while (read(STDIN, $len, 4) == 4) {
-			if ($len eq "0000") {
-				print "FLUSH\n";
-			} else {
-				read(STDIN, $buf, hex($len) - 4);
-				$buf =~ s/\0/\\0/g;
-				if ($buf =~ s/^[\x2\x3]//) {
-					print STDERR $buf;
-				} else {
-					$buf =~ s/^\x1//;
-					print $buf;
-				}
-			}
-		}
-	'
-}
-
-# Converts base-16 data into base-8. The output is given as a sequence of
-# escaped octals, suitable for consumption by 'printf'.
-hex2oct () {
-	perl -ne 'printf "\\%03o", hex for /../g'
-}
-
 # Set the hash algorithm in use to $1.  Only useful when testing the testsuite.
 test_set_hash () {
 	test_hash_algo="$1"
@@ -1611,16 +1366,6 @@ test_set_port () {
 	eval $var=$port
 }
 
-# Tests for the hidden file attribute on Windows
-test_path_is_hidden () {
-	test_have_prereq MINGW ||
-	BUG "test_path_is_hidden can only be used on Windows"
-
-	# Use the output of `attrib`, ignore the absolute path
-	case "$("$SYSTEMROOT"/system32/attrib "$1")" in *H*?:*) return 0;; esac
-	return 1
-}
-
 # Check that the given command was invoked as part of the
 # trace2-format trace on stdin.
 #
diff --git a/t/test-lib-functions/debug.sh b/t/test-lib-functions/debug.sh
new file mode 100644
index 00000000000..4393bf82f02
--- /dev/null
+++ b/t/test-lib-functions/debug.sh
@@ -0,0 +1,99 @@
+# Included by test-lib.sh via test-lib-functions.sh
+#
+# Text munging functions, e.g. wrappers for perl, tr, sed
+# etc. one-liners.
+
+lf_to_nul () {
+	perl -pe 'y/\012/\000/'
+}
+
+nul_to_q () {
+	perl -pe 'y/\000/Q/'
+}
+
+q_to_nul () {
+	perl -pe 'y/Q/\000/'
+}
+
+q_to_cr () {
+	tr Q '\015'
+}
+
+q_to_tab () {
+	tr Q '\011'
+}
+
+qz_to_tab_space () {
+	tr QZ '\011\040'
+}
+
+append_cr () {
+	sed -e 's/$/Q/' | tr Q '\015'
+}
+
+remove_cr () {
+	tr '\015' Q | sed -e 's/Q$//'
+}
+
+# Converts base-16 data into base-8. The output is given as a sequence of
+# escaped octals, suitable for consumption by 'printf'.
+hex2oct () {
+	perl -ne 'printf "\\%03o", hex for /../g'
+}
+
+# convert function arguments or stdin (if not arguments given) to pktline
+# representation. If multiple arguments are given, they are separated by
+# whitespace and put in a single packet. Note that data containing NULs must be
+# given on stdin, and that empty input becomes an empty packet, not a flush
+# packet (for that you can just print 0000 yourself).
+packetize () {
+	if test $# -gt 0
+	then
+		packet="$*"
+		printf '%04x%s' "$((4 + ${#packet}))" "$packet"
+	else
+		perl -e '
+			my $packet = do { local $/; <STDIN> };
+			printf "%04x%s", 4 + length($packet), $packet;
+		'
+	fi
+}
+
+# Parse the input as a series of pktlines, writing the result to stdout.
+# Sideband markers are removed automatically, and the output is routed to
+# stderr if appropriate.
+#
+# NUL bytes are converted to "\\0" for ease of parsing with text tools.
+depacketize () {
+	perl -e '
+		while (read(STDIN, $len, 4) == 4) {
+			if ($len eq "0000") {
+				print "FLUSH\n";
+			} else {
+				read(STDIN, $buf, hex($len) - 4);
+				$buf =~ s/\0/\\0/g;
+				if ($buf =~ s/^[\x2\x3]//) {
+					print STDERR $buf;
+				} else {
+					$buf =~ s/^\x1//;
+					print $buf;
+				}
+			}
+		}
+	'
+}
+
+# Read up to "$1" bytes (or to EOF) from stdin and write them to stdout.
+test_copy_bytes () {
+	perl -e '
+		my $len = $ARGV[1];
+		while ($len > 0) {
+			my $s;
+			my $nread = sysread(STDIN, $s, $len);
+			die "cannot read: $!" unless defined($nread);
+			last unless $nread;
+			print $s;
+			$len -= $nread;
+		}
+	' - "$1"
+}
diff --git a/t/test-lib-functions/path.sh b/t/test-lib-functions/path.sh
new file mode 100644
index 00000000000..af1dd140c27
--- /dev/null
+++ b/t/test-lib-functions/path.sh
@@ -0,0 +1,137 @@
+# Included by test-lib.sh via test-lib-functions.sh
+#
+# File functions, e.g. wrappers for "test [-e|-s|-f|...]", "wc -l"
+# etc.
+
+# ... test -f
+test_path_is_file () {
+	test "$#" -ne 1 && BUG "1 param"
+	if ! test -f "$1"
+	then
+		echo "File $1 doesn't exist"
+		false
+	fi
+}
+
+# ... test -d
+test_path_is_dir () {
+	test "$#" -ne 1 && BUG "1 param"
+	if ! test -d "$1"
+	then
+		echo "Directory $1 doesn't exist"
+		false
+	fi
+}
+
+# test -d && is_empty()
+test_dir_is_empty () {
+	test "$#" -ne 1 && BUG "1 param"
+	test_path_is_dir "$1" &&
+	if test -n "$(ls -a1 "$1" | egrep -v '^\.\.?$')"
+	then
+		echo "Directory '$1' is not empty, it contains:"
+		ls -la "$1"
+		return 1
+	fi
+}
+
+# ... test -e
+test_path_exists () {
+	test "$#" -ne 1 && BUG "1 param"
+	if ! test -e "$1"
+	then
+		echo "Path $1 doesn't exist"
+		false
+	fi
+}
+
+# ... ! test -e
+test_path_is_missing () {
+	test "$#" -ne 1 && BUG "1 param"
+	if test -e "$1"
+	then
+		echo "Path exists:"
+		ls -ld "$1"
+		if test $# -ge 1
+		then
+			echo "$*"
+		fi
+		false
+	fi
+}
+
+# ... test -s
+test_must_be_empty () {
+	test "$#" -ne 1 && BUG "1 param"
+	test_path_is_file "$1" &&
+	if test -s "$1"
+	then
+		echo "'$1' is not empty, it contains:"
+		cat "$1"
+		return 1
+	fi
+}
+
+# ... ! test -s
+test_file_not_empty () {
+	test "$#" = 2 && BUG "2 param"
+	if ! test -s "$1"
+	then
+		echo "'$1' is not a non-empty file."
+		false
+	fi
+}
+
+test_file_size () {
+	test "$#" -ne 1 && BUG "1 param"
+	test-tool path-utils file-size "$1"
+}
+
+# This function helps systems where core.filemode=false is set.
+# Use it instead of plain 'chmod +x' to set or unset the executable bit
+# of a file in the working directory and add it to the index.
+test_chmod () {
+	chmod "$@" &&
+	git update-index --add "--chmod=$@"
+}
+
+# Get the modebits from a file or directory, ignoring the setgid bit (g+s).
+# This bit is inherited by subdirectories at their creation. So we remove it
+# from the returning string to prevent callers from having to worry about the
+# state of the bit in the test directory.
+test_modebits () {
+	ls -ld "$1" | sed -e 's|^\(..........\).*|\1|' \
+			  -e 's|^\(......\)S|\1-|' -e 's|^\(......\)s|\1x|'
+}
+
+# test_line_count checks that a file has the number of lines it
+# ought to. For example:
+#
+#	test_expect_success 'produce exactly one line of output' '
+#		do something >output &&
+#		test_line_count = 1 output
+#	'
+#
+# is like "test $(wc -l <output) = 1" except that it passes the
+# output through when the number of lines is wrong.
+test_line_count () {
+	if test $# != 3
+	then
+		BUG "not 3 parameters to test_line_count"
+	elif ! test $(wc -l <"$3") "$1" "$2"
+	then
+		echo "test_line_count: line count for $3 !$1 $2"
+		cat "$3"
+		return 1
+	fi
+}
+
+# Tests for the hidden file attribute on Windows
+test_path_is_hidden () {
+	test_have_prereq MINGW ||
+	BUG "test_path_is_hidden can only be used on Windows"
+
+	# Use the output of `attrib`, ignore the absolute path
+	case "$("$SYSTEMROOT"/system32/attrib "$1")" in *H*?:*) return 0;; esac
+	return 1
+}
diff --git a/t/test-lib-functions/text.sh b/t/test-lib-functions/text.sh
new file mode 100644
index 00000000000..e5f438ffd31
--- /dev/null
+++ b/t/test-lib-functions/text.sh
@@ -0,0 +1,33 @@
+# Included by test-lib.sh via test-lib-functions.sh
+#
+# Debugging functions, not intended to be present in submitted
+# patches. Be sure to remove their use before submitting.
+
+# Stop execution and start a shell.
+test_pause () {
+	"$SHELL_PATH" <&6 >&5 2>&7
+}
+
+# Wrap git with a debugger. Adding this to a command can make it easier
+# to understand what is going on in a failing test.
+#
+# Examples:
+#     debug git checkout master
+#     debug --debugger=nemiver git $ARGS
+#     debug -d "valgrind --tool=memcheck --track-origins=yes" git $ARGS
+debug () {
+	case "$1" in
+	-d)
+		GIT_DEBUGGER="$2" &&
+		shift 2
+		;;
+	--debugger=*)
+		GIT_DEBUGGER="${1#*=}" &&
+		shift 1
+		;;
+	*)
+		GIT_DEBUGGER=1
+		;;
+	esac &&
+	GIT_DEBUGGER="${GIT_DEBUGGER}" "$@" <&6 >&5 2>&7
+}
-- 
2.30.0.284.gd98b1dd5eaa7

Re: [PATCH 12/12] test-lib-functions: split out {debug,path,text} helpers

[Denton Liu]

Hi Ævar,

Just a quick drive-by comment:

On Tue, Feb 09, 2021 at 10:41:59PM +0100, Ævar Arnfjörð Bjarmason wrote:
> diff --git a/t/test-lib-functions/debug.sh b/t/test-lib-functions/debug.sh
> new file mode 100644
> index 00000000000..4393bf82f02
> --- /dev/null
> +++ b/t/test-lib-functions/debug.sh

Is this supposed to be text.sh?

> @@ -0,0 +1,99 @@
> +# Included by test-lib.sh via test-lib-functions.sh
> +#
> +# Text munging functions, e.g. wrappers for perl, tr, sed
> +# etc. one-liners.

[...]

> diff --git a/t/test-lib-functions/text.sh b/t/test-lib-functions/text.sh
> new file mode 100644
> index 00000000000..e5f438ffd31
> --- /dev/null
> +++ b/t/test-lib-functions/text.sh

And is this one supposed to be debug.sh?

> @@ -0,0 +1,33 @@
> +# Included by test-lib.sh via test-lib-functions.sh
> +#
> +# Debugging functions, not intended to be present in submitted
> +# patches. Be sure to remove their use before submitting.
> +
> +# Stop execution and start a shell.
> +test_pause () {
> +	"$SHELL_PATH" <&6 >&5 2>&7
> +}
> +
> +# Wrap git with a debugger. Adding this to a command can make it easier
> +# to understand what is going on in a failing test.
> +#
> +# Examples:
> +#     debug git checkout master
> +#     debug --debugger=nemiver git $ARGS
> +#     debug -d "valgrind --tool=memcheck --track-origins=yes" git $ARGS
> +debug () {
> +	case "$1" in
> +	-d)
> +		GIT_DEBUGGER="$2" &&
> +		shift 2
> +		;;
> +	--debugger=*)
> +		GIT_DEBUGGER="${1#*=}" &&
> +		shift 1
> +		;;
> +	*)
> +		GIT_DEBUGGER=1
> +		;;
> +	esac &&
> +	GIT_DEBUGGER="${GIT_DEBUGGER}" "$@" <&6 >&5 2>&7
> +}
> -- 
> 2.30.0.284.gd98b1dd5eaa7
> 

Re: [PATCH 12/12] test-lib-functions: split out {debug,path,text} helpers

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Split out the debug, path and text miscellaneous helpers into their
> own files. The "test -[efsd]" etc. helpers were not all in one place
> in test-lib-functions.sh, I think moving them into their own file
> makes it easier to follow the code.

I'd prefer not to see this done.  It would be fine to move them in
different sections but still in the same file to group related
helpers together, but I am having enough trouble when deciding to
look into which between test-lib.sh and test-lib-functions.sh in the
current organization already.  And it will make the situation even
worse if we did this, no?

Re: [PATCH 12/12] test-lib-functions: split out {debug,path,text} helpers

[SZEDER Gábor]

On Tue, Feb 09, 2021 at 04:06:59PM -0800, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
> 
> > Split out the debug, path and text miscellaneous helpers into their
> > own files. The "test -[efsd]" etc. helpers were not all in one place
> > in test-lib-functions.sh, I think moving them into their own file
> > makes it easier to follow the code.
> 
> I'd prefer not to see this done.  It would be fine to move them in
> different sections but still in the same file to group related
> helpers together, but I am having enough trouble when deciding to
> look into which between test-lib.sh and test-lib-functions.sh in the
> current organization already.  And it will make the situation even
> worse if we did this, no?

Indeed, I'd rather not see this one, either.

Re: [PATCH 12/12] test-lib-functions: split out {debug,path,text} helpers

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

Hi,

On Tue, 9 Feb 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
> > Split out the debug, path and text miscellaneous helpers into their
> > own files. The "test -[efsd]" etc. helpers were not all in one place
> > in test-lib-functions.sh, I think moving them into their own file
> > makes it easier to follow the code.
>
> I'd prefer not to see this done.  It would be fine to move them in
> different sections but still in the same file to group related
> helpers together, but I am having enough trouble when deciding to
> look into which between test-lib.sh and test-lib-functions.sh in the
> current organization already.  And it will make the situation even
> worse if we did this, no?

As much as I would love to see a better organization, I agree with Junio
that this split would make it more tedious to find the correct function.

I had a look over the rest of the series and like what I saw, leaving only
one or two comments.

Thanks,
Dscho

Re: [PATCH 12/12] test-lib-functions: split out {debug,path,text} helpers

[Ævar Arnfjörð Bjarmason]

On Thu, Feb 11 2021, Johannes Schindelin wrote:

> Hi,
>
> On Tue, 9 Feb 2021, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>>
>> > Split out the debug, path and text miscellaneous helpers into their
>> > own files. The "test -[efsd]" etc. helpers were not all in one place
>> > in test-lib-functions.sh, I think moving them into their own file
>> > makes it easier to follow the code.
>>
>> I'd prefer not to see this done.  It would be fine to move them in
>> different sections but still in the same file to group related
>> helpers together, but I am having enough trouble when deciding to
>> look into which between test-lib.sh and test-lib-functions.sh in the
>> current organization already.  And it will make the situation even
>> worse if we did this, no?
>
> As much as I would love to see a better organization, I agree with Junio
> that this split would make it more tedious to find the correct function.
>
> I had a look over the rest of the series and like what I saw, leaving only
> one or two comments.

Junio / SZEDER / Johannes: Yeah I agree that that 12/12 sucked in its
current form.

Having eyeballed test-lib.sh and test-lib-functions.sh as part of this
though there seems to be a pretty obvious way to split this up which I
think would increase the readability:

 * test-lib.sh: main entry, reading/setting global vars/state,
   command-line parsing (and other CLI / Makefile interface), loading
   test-lib-functions.sh, set global prereqs, actual setup for the test
   about to be run.

 * test-lib-functions.sh: the top-level "library" used everywhere,
   i.e. test_expect_success/test_expect_failure etc, Hopefully we could
   move the internal functions like test_ok_, test_failure_ etc. from
   test-lib.sh to test-lib-functions.sh (they need some global variables
   in test-lib.sh now).

   Also things part of core test "flow" like test_config,
   test_when_finished, nongit etc.

 * lib-common.sh: what I was trying to start with in this 12/12,
   i.e. utility functions common enough that we want them
   "everywhere". We'd just have test-lib-functions.sh load this.

That seems like a much more obvious split & organization to me than the
mis-match with a bit of everything everyhere we have right now.

[PATCH v2 00/11] test-lib: misc improvements

[Ævar Arnfjörð Bjarmason]

Fixed a missing SOB and dropped the 12th patch.

Ævar Arnfjörð Bjarmason (11):
  test-lib: remove check_var_migration
  test lib: change "error" to "BUG" as appropriate
  test-lib-functions: move test_set_index_version() to its user
  test-lib-functions: remove generate_zero_bytes() wrapper
  test libs: rename bundle helper to "lib-bundle.sh"
  test libs: rename gitweb-lib.sh to lib-gitweb.sh
  test-lib-functions: move function to lib-bitmap.sh
  t/.gitattributes: sort lines
  test libs: rename "diff-lib" to "lib-diff"
  test-lib-functions: remove bug-inducing "diagnostics" helper param
  test-lib-functions: assert correct parameter count

 t/.gitattributes                              |  4 +-
 t/README                                      |  8 +--
 t/lib-bitmap.sh                               | 26 ++++++++
 t/{test-bundle-functions.sh => lib-bundle.sh} |  0
 t/{diff-lib.sh => lib-diff.sh}                |  0
 t/{diff-lib => lib-diff}/COPYING              |  0
 t/{diff-lib => lib-diff}/README               |  0
 t/{gitweb-lib.sh => lib-gitweb.sh}            |  0
 t/t2104-update-index-skip-worktree.sh         |  5 ++
 t/t3404-rebase-interactive.sh                 |  3 +-
 t/t4000-diff-format.sh                        |  2 +-
 t/t4001-diff-rename.sh                        |  2 +-
 t/t4003-diff-rename-1.sh                      |  6 +-
 t/t4004-diff-rename-symlink.sh                |  2 +-
 t/t4005-diff-rename-2.sh                      |  6 +-
 t/t4007-rename-3.sh                           |  6 +-
 t/t4008-diff-break-rewrite.sh                 |  6 +-
 t/t4009-diff-rename-4.sh                      |  6 +-
 t/t4010-diff-pathspec.sh                      |  2 +-
 t/t4011-diff-symlink.sh                       |  2 +-
 t/t4013-diff-various.sh                       |  2 +-
 t/t4015-diff-whitespace.sh                    |  2 +-
 t/t4027-diff-submodule.sh                     |  2 +-
 t/t4034-diff-words.sh                         |  2 +-
 t/t4038-diff-combined.sh                      |  2 +-
 t/t4061-diff-indent.sh                        |  2 +-
 t/t4206-log-follow-harder-copies.sh           |  2 +-
 t/t5310-pack-bitmaps.sh                       |  2 +
 t/t5318-commit-graph.sh                       |  2 +-
 t/t5510-fetch.sh                              |  2 +-
 t/t6020-bundle-misc.sh                        |  2 +-
 t/t6113-rev-list-bitmap-filters.sh            |  1 +
 t/t6426-merge-skip-unneeded-updates.sh        | 16 +++--
 t/t6500-gc.sh                                 |  4 +-
 t/t7501-commit-basic-functionality.sh         |  2 +-
 t/t9300-fast-import.sh                        |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh        |  2 +-
 t/t9501-gitweb-standalone-http-status.sh      |  2 +-
 t/t9502-gitweb-standalone-parse-output.sh     |  2 +-
 t/test-lib-functions.sh                       | 62 +++++--------------
 t/test-lib.sh                                 | 30 ---------
 41 files changed, 107 insertions(+), 124 deletions(-)
 create mode 100644 t/lib-bitmap.sh
 rename t/{test-bundle-functions.sh => lib-bundle.sh} (100%)
 rename t/{diff-lib.sh => lib-diff.sh} (100%)
 rename t/{diff-lib => lib-diff}/COPYING (100%)
 rename t/{diff-lib => lib-diff}/README (100%)
 rename t/{gitweb-lib.sh => lib-gitweb.sh} (100%)

Range-diff:
 1:  422ffbfd16a =  1:  5243499bf03 test-lib: remove check_var_migration
 2:  47b4bf0e49d =  2:  aa67ff1e6b9 test lib: change "error" to "BUG" as appropriate
 3:  e8cb488e124 =  3:  56cff1fb482 test-lib-functions: move test_set_index_version() to its user
 4:  d4ef2a8fd5b =  4:  8960a2578da test-lib-functions: remove generate_zero_bytes() wrapper
 5:  9a164f4d49b =  5:  8bb54931234 test libs: rename bundle helper to "lib-bundle.sh"
 6:  d91be650b42 =  6:  a72b1d02225 test libs: rename gitweb-lib.sh to lib-gitweb.sh
 7:  0ffc90200f8 =  7:  ab8769ca71f test-lib-functions: move function to lib-bitmap.sh
 8:  57a258c1414 =  8:  0dfe871261d t/.gitattributes: sort lines
 9:  843a8718658 !  9:  ffde3c43099 test libs: rename "diff-lib" to "lib-diff"
    @@ Commit message
         commits there is no remaining t/*lib* which doesn't follow the
         convention of being called t/lib-*.
     
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
    +
      ## t/.gitattributes ##
     @@
      t[0-9][0-9][0-9][0-9]/* -whitespace
10:  65b2a41a784 = 10:  ea732f64704 test-lib-functions: remove bug-inducing "diagnostics" helper param
11:  9424b533f02 = 11:  3932ed40712 test-lib-functions: assert correct parameter count
12:  c649939196d <  -:  ----------- test-lib-functions: split out {debug,path,text} helpers
-- 
2.30.0.284.gd98b1dd5eaa7

Re: [PATCH v2 00/11] test-lib: misc improvements

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Fixed a missing SOB and dropped the 12th patch.
>
> Ævar Arnfjörð Bjarmason (11):
>   test-lib: remove check_var_migration
>   test lib: change "error" to "BUG" as appropriate
>   test-lib-functions: move test_set_index_version() to its user
>   test-lib-functions: remove generate_zero_bytes() wrapper
>   test libs: rename bundle helper to "lib-bundle.sh"
>   test libs: rename gitweb-lib.sh to lib-gitweb.sh
>   test-lib-functions: move function to lib-bitmap.sh
>   t/.gitattributes: sort lines
>   test libs: rename "diff-lib" to "lib-diff"
>   test-lib-functions: remove bug-inducing "diagnostics" helper param
>   test-lib-functions: assert correct parameter count

Will replace and queue.  Let's declare a victory and move on.

[PATCH v2 01/11] test-lib: remove check_var_migration

[Ævar Arnfjörð Bjarmason]

Remove the check_var_migration() migration helper. This was added back
in [1], [2] and [3] to warn users to migrate from e.g. the
"GIT_FSMONITOR_TEST" name to "GIT_TEST_FSMONITOR".

I daresay that having been warning about this since late 2018 (or
v2.20.0) was sufficient time to give everyone interested a heads-up
about moving to the new names.

I don't see the need for going through the "do this later" codepath
anticipated in [1], let's just remove this instead.

1. 4cb54d0aa8e (fsmonitor: update GIT_TEST_FSMONITOR support,
   2018-09-18)
2. 1f357b045b5 (read-cache: update TEST_GIT_INDEX_VERSION support,
   2018-09-18)
3. 5765d97b71d (preload-index: update GIT_FORCE_PRELOAD_TEST support,
   2018-09-18)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh | 30 ------------------------------
 1 file changed, 30 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 431adba0fb3..987a6c1936d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -453,36 +453,6 @@ export GIT_DEFAULT_HASH
 GIT_TRACE_BARE=1
 export GIT_TRACE_BARE
 
-check_var_migration () {
-	# the warnings and hints given from this helper depends
-	# on end-user settings, which will disrupt the self-test
-	# done on the test framework itself.
-	case "$GIT_TEST_FRAMEWORK_SELFTEST" in
-	t)	return ;;
-	esac
-
-	old_name=$1 new_name=$2
-	eval "old_isset=\${${old_name}:+isset}"
-	eval "new_isset=\${${new_name}:+isset}"
-
-	case "$old_isset,$new_isset" in
-	isset,)
-		echo >&2 "warning: $old_name is now $new_name"
-		echo >&2 "hint: set $new_name too during the transition period"
-		eval "$new_name=\$$old_name"
-		;;
-	isset,isset)
-		# do this later
-		# echo >&2 "warning: $old_name is now $new_name"
-		# echo >&2 "hint: remove $old_name"
-		;;
-	esac
-}
-
-check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
-check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
-check_var_migration GIT_FORCE_PRELOAD_TEST GIT_TEST_PRELOAD_INDEX
-
 # Use specific version of the index file format
 if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
 then
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH v2 02/11] test lib: change "error" to "BUG" as appropriate

[Ævar Arnfjörð Bjarmason]

Change two uses of "error" in test-lib-functions.sh to "BUG".

In the first instance in "test_cmp_rev" the author of the "BUG"
function added in [1] had another in-flight patch adding this in [2],
and the two were never consolidated.

In the second case in "test_atexit" added in [3] that we could have
instead used "BUG" appears to have been missed.

1. 165293af3ce (tests: send "bug in the test script" errors to the
   script's stderr, 2018-11-19)

2. 30d0b6dccbc (test-lib-functions: make 'test_cmp_rev' more
   informative on failure, 2018-11-19)

3. 900721e15c4 (test-lib: introduce 'test_atexit', 2019-03-13)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib-functions.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 07976af81c8..4b7e09db63a 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1096,7 +1096,7 @@ test_cmp_rev () {
 	fi
 	if test $# != 2
 	then
-		error "bug in the test script: test_cmp_rev requires two revisions, but got $#"
+		BUG "test_cmp_rev requires two revisions, but got $#"
 	else
 		local r1 r2
 		r1=$(git rev-parse --verify "$1") &&
@@ -1207,7 +1207,7 @@ test_atexit () {
 	# doing so on Bash is better than nothing (the test will
 	# silently pass on other shells).
 	test "${BASH_SUBSHELL-0}" = 0 ||
-	error "bug in test script: test_atexit does nothing in a subshell"
+	BUG "test_atexit does nothing in a subshell"
 	test_atexit_cleanup="{ $*
 		} && (exit \"\$eval_ret\"); eval_ret=\$?; $test_atexit_cleanup"
 }
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH v2 03/11] test-lib-functions: move test_set_index_version() to its user

[Ævar Arnfjörð Bjarmason]

Move the test_set_index_version() function to its only user. This
function has only been used in one place since its addition in
5d9fc888b48 (test-lib: allow setting the index format version,
2014-02-23). Let's have that test script define it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t2104-update-index-skip-worktree.sh | 5 +++++
 t/test-lib-functions.sh               | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t2104-update-index-skip-worktree.sh b/t/t2104-update-index-skip-worktree.sh
index 7e2e7dd4ae5..30666fc70d2 100755
--- a/t/t2104-update-index-skip-worktree.sh
+++ b/t/t2104-update-index-skip-worktree.sh
@@ -9,6 +9,11 @@ test_description='skip-worktree bit test'
 
 sane_unset GIT_TEST_SPLIT_INDEX
 
+test_set_index_version () {
+    GIT_INDEX_VERSION="$1"
+    export GIT_INDEX_VERSION
+}
+
 test_set_index_version 3
 
 cat >expect.full <<EOF
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 4b7e09db63a..82ad0b91743 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -32,11 +32,6 @@ test_set_editor () {
 	export EDITOR
 }
 
-test_set_index_version () {
-    GIT_INDEX_VERSION="$1"
-    export GIT_INDEX_VERSION
-}
-
 test_decode_color () {
 	awk '
 		function name(n) {
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH v2 04/11] test-lib-functions: remove generate_zero_bytes() wrapper

[Ævar Arnfjörð Bjarmason]

Since d5cfd142ec1 (tests: teach the test-tool to generate NUL bytes
and use it, 2019-02-14) the generate_zero_bytes() functions has been a
thin wrapper for "test-tool genzeros". Let's have its only user call
that directly instead.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t5318-commit-graph.sh | 2 +-
 t/test-lib-functions.sh | 7 -------
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 2ed0c1544da..368d1cfb448 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -529,7 +529,7 @@ corrupt_graph_and_verify() {
 	zero_pos=${4:-${orig_size}} &&
 	printf "$data" | dd of="$objdir/info/commit-graph" bs=1 seek="$pos" conv=notrunc &&
 	dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" if=/dev/null &&
-	generate_zero_bytes $(($orig_size - $zero_pos)) >>"$objdir/info/commit-graph" &&
+	test-tool genzeros $(($orig_size - $zero_pos)) >>"$objdir/info/commit-graph" &&
 	corrupt_graph_verify "$grepstr"
 
 }
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 82ad0b91743..71d4e806f6d 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -111,13 +111,6 @@ remove_cr () {
 	tr '\015' Q | sed -e 's/Q$//'
 }
 
-# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes).
-# If $1 is 'infinity', output forever or until the receiving pipe stops reading,
-# whichever comes first.
-generate_zero_bytes () {
-	test-tool genzeros "$@"
-}
-
 # In some bourne shell implementations, the "unset" builtin returns
 # nonzero status when a variable to be unset was not set in the first
 # place.
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH v2 05/11] test libs: rename bundle helper to "lib-bundle.sh"

[Ævar Arnfjörð Bjarmason]

Rename the recently introduced test-bundle-functions.sh to be
consistent with other lib-*.sh files, which is the convention for
these sorts of shared test library functions.

The new test-bundle-functions.sh was introduced in 9901164d81d (test:
add helper functions for git-bundle, 2021-01-11). It was the only
test-*.sh of this nature.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/{test-bundle-functions.sh => lib-bundle.sh} | 0
 t/t5510-fetch.sh                              | 2 +-
 t/t6020-bundle-misc.sh                        | 2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename t/{test-bundle-functions.sh => lib-bundle.sh} (100%)

diff --git a/t/test-bundle-functions.sh b/t/lib-bundle.sh
similarity index 100%
rename from t/test-bundle-functions.sh
rename to t/lib-bundle.sh
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 42f55030047..c1e0eb39231 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -9,7 +9,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/test-bundle-functions.sh
+. "$TEST_DIRECTORY"/lib-bundle.sh
 
 D=$(pwd)
 
diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh
index 6249420a806..da5fe1ba6de 100755
--- a/t/t6020-bundle-misc.sh
+++ b/t/t6020-bundle-misc.sh
@@ -9,7 +9,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/test-bundle-functions.sh
+. "$TEST_DIRECTORY"/lib-bundle.sh
 
 # Create a commit or tag and set the variable with the object ID.
 test_commit_setvar () {
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH v2 06/11] test libs: rename gitweb-lib.sh to lib-gitweb.sh

[Ævar Arnfjörð Bjarmason]

Rename gitweb-lib.sh to lib-gitweb.sh for consistency with other test
library files.

When it was introduced in 05526071cb5 (gitweb: split test suite into
library and tests, 2009-08-27) this naming pattern was more
common.

Since then all but one other such library which didn't start with
"lib-*.sh" such as t6000lib.sh has been been renamed, see
e.g. 9d488eb40e2 (Move t6000lib.sh to lib-*, 2010-05-07).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/{gitweb-lib.sh => lib-gitweb.sh}        | 0
 t/t9500-gitweb-standalone-no-errors.sh    | 2 +-
 t/t9501-gitweb-standalone-http-status.sh  | 2 +-
 t/t9502-gitweb-standalone-parse-output.sh | 2 +-
 4 files changed, 3 insertions(+), 3 deletions(-)
 rename t/{gitweb-lib.sh => lib-gitweb.sh} (100%)

diff --git a/t/gitweb-lib.sh b/t/lib-gitweb.sh
similarity index 100%
rename from t/gitweb-lib.sh
rename to t/lib-gitweb.sh
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index ee8c6e30e67..0333065d4d6 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -13,7 +13,7 @@ or warnings to log.'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
-. ./gitweb-lib.sh
+. ./lib-gitweb.sh
 
 # ----------------------------------------------------------------------
 # no commits (empty, just initialized repository)
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
index 141610de546..32814e75df5 100755
--- a/t/t9501-gitweb-standalone-http-status.sh
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -13,7 +13,7 @@ code and message.'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
-. ./gitweb-lib.sh
+. ./lib-gitweb.sh
 
 #
 # Gitweb only provides the functionality tested by the 'modification times'
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
index 9cf7ab30a8d..3167473b303 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -13,7 +13,7 @@ in the HTTP header or the actual script output.'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
-. ./gitweb-lib.sh
+. ./lib-gitweb.sh
 
 # ----------------------------------------------------------------------
 # snapshot file name and prefix
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH v2 07/11] test-lib-functions: move function to lib-bitmap.sh

[Ævar Arnfjörð Bjarmason]

Move a function added to test-lib-functions.sh in ea047a8eb4f (t5310:
factor out bitmap traversal comparison, 2020-02-14) into a new
lib-bitmap.sh.

The test-lib-functions.sh file should be for functions that are widely
used across the test suite, if something's only used by a few tests it
makes more sense to have it in a lib-*.sh file.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/lib-bitmap.sh                    | 26 ++++++++++++++++++++++++++
 t/t5310-pack-bitmaps.sh            |  2 ++
 t/t6113-rev-list-bitmap-filters.sh |  1 +
 t/test-lib-functions.sh            | 27 ---------------------------
 4 files changed, 29 insertions(+), 27 deletions(-)
 create mode 100644 t/lib-bitmap.sh

diff --git a/t/lib-bitmap.sh b/t/lib-bitmap.sh
new file mode 100644
index 00000000000..fe3f98be24f
--- /dev/null
+++ b/t/lib-bitmap.sh
@@ -0,0 +1,26 @@
+# Compare a file containing rev-list bitmap traversal output to its non-bitmap
+# counterpart. You can't just use test_cmp for this, because the two produce
+# subtly different output:
+#
+#   - regular output is in traversal order, whereas bitmap is split by type,
+#     with non-packed objects at the end
+#
+#   - regular output has a space and the pathname appended to non-commit
+#     objects; bitmap output omits this
+#
+# This function normalizes and compares the two. The second file should
+# always be the bitmap output.
+test_bitmap_traversal () {
+	if test "$1" = "--no-confirm-bitmaps"
+	then
+		shift
+	elif cmp "$1" "$2"
+	then
+		echo >&2 "identical raw outputs; are you sure bitmaps were used?"
+		return 1
+	fi &&
+	cut -d' ' -f1 "$1" | sort >"$1.normalized" &&
+	sort "$2" >"$2.normalized" &&
+	test_cmp "$1.normalized" "$2.normalized" &&
+	rm -f "$1.normalized" "$2.normalized"
+}
diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index 5ba76031090..40b9f632441 100755
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -5,6 +5,8 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-bundle.sh
+. "$TEST_DIRECTORY"/lib-bitmap.sh
 
 objpath () {
 	echo ".git/objects/$(echo "$1" | sed -e 's|\(..\)|\1/|')"
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index 2b551e6fd0c..3f889949ca1 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -2,6 +2,7 @@
 
 test_description='rev-list combining bitmaps and filters'
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-bitmap.sh
 
 test_expect_success 'set up bitmapped repo' '
 	# one commit will have bitmaps, the other will not
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 71d4e806f6d..473d2e283e6 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1593,33 +1593,6 @@ test_set_port () {
 	eval $var=$port
 }
 
-# Compare a file containing rev-list bitmap traversal output to its non-bitmap
-# counterpart. You can't just use test_cmp for this, because the two produce
-# subtly different output:
-#
-#   - regular output is in traversal order, whereas bitmap is split by type,
-#     with non-packed objects at the end
-#
-#   - regular output has a space and the pathname appended to non-commit
-#     objects; bitmap output omits this
-#
-# This function normalizes and compares the two. The second file should
-# always be the bitmap output.
-test_bitmap_traversal () {
-	if test "$1" = "--no-confirm-bitmaps"
-	then
-		shift
-	elif cmp "$1" "$2"
-	then
-		echo >&2 "identical raw outputs; are you sure bitmaps were used?"
-		return 1
-	fi &&
-	cut -d' ' -f1 "$1" | sort >"$1.normalized" &&
-	sort "$2" >"$2.normalized" &&
-	test_cmp "$1.normalized" "$2.normalized" &&
-	rm -f "$1.normalized" "$2.normalized"
-}
-
 # Tests for the hidden file attribute on Windows
 test_path_is_hidden () {
 	test_have_prereq MINGW ||
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH v2 08/11] t/.gitattributes: sort lines

[Ævar Arnfjörð Bjarmason]

Sort the lines starting with "/", the only out-of-place line was added
along with most of the file in 614f4f0f350 (Fix the remaining tests
that failed with core.autocrlf=true, 2017-05-09).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/.gitattributes | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/.gitattributes b/t/.gitattributes
index df05434d32c..d778bfad052 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -5,9 +5,9 @@ t[0-9][0-9][0-9][0-9]/* -whitespace
 /t3206/* eol=lf
 /t3900/*.txt eol=lf
 /t3901/*.txt eol=lf
-/t4034/*/* eol=lf
 /t4013/* eol=lf
 /t4018/* eol=lf
+/t4034/*/* eol=lf
 /t4051/* eol=lf
 /t4100/* eol=lf
 /t4101/* eol=lf
-- 
2.30.0.284.gd98b1dd5eaa7

[PATCH v2 09/11] test libs: rename "diff-lib" to "lib-diff"

[Ævar Arnfjörð Bjarmason]

Rename the "diff-lib" to "lib-diff". With this rename and preceding
commits there is no remaining t/*lib* which doesn't follow the
convention of being called t/lib-*.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/.gitattributes                      | 2 +-
 t/{diff-lib.sh => lib-diff.sh}        | 0
 t/{diff-lib => lib-diff}/COPYING      | 0
 t/{diff-lib => lib-diff}/README       | 0
 t/t4000-diff-format.sh                | 2 +-
 t/t4001-diff-rename.sh                | 2 +-
 t/t4003-diff-rename-1.sh              | 6 +++---
 t/t4004-diff-rename-symlink.sh        | 2 +-
 t/t4005-diff-rename-2.sh              | 6 +++---
 t/t4007-rename-3.sh                   | 6 +++---
 t/t4008-diff-break-rewrite.sh         | 6 +++---
 t/t4009-diff-rename-4.sh              | 6 +++---
 t/t4010-diff-pathspec.sh              | 2 +-
 t/t4011-diff-symlink.sh               | 2 +-
 t/t4013-diff-various.sh               | 2 +-
 t/t4015-diff-whitespace.sh            | 2 +-
 t/t4027-diff-submodule.sh             | 2 +-
 t/t4034-diff-words.sh                 | 2 +-
 t/t4038-diff-combined.sh              | 2 +-
 t/t4061-diff-indent.sh                | 2 +-
 t/t4206-log-follow-harder-copies.sh   | 2 +-
 t/t7501-commit-basic-functionality.sh | 2 +-
 t/t9300-fast-import.sh                | 2 +-
 23 files changed, 30 insertions(+), 30 deletions(-)
 rename t/{diff-lib.sh => lib-diff.sh} (100%)
 rename t/{diff-lib => lib-diff}/COPYING (100%)
 rename t/{diff-lib => lib-diff}/README (100%)

diff --git a/t/.gitattributes b/t/.gitattributes
index d778bfad052..dafa17c3e61 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -1,6 +1,6 @@
 t[0-9][0-9][0-9][0-9]/* -whitespace
 /chainlint/*.expect eol=lf
-/diff-lib/* eol=lf
+/lib-diff/* eol=lf
 /t0110/url-* binary
 /t3206/* eol=lf
 /t3900/*.txt eol=lf
diff --git a/t/diff-lib.sh b/t/lib-diff.sh
similarity index 100%
rename from t/diff-lib.sh
rename to t/lib-diff.sh
diff --git a/t/diff-lib/COPYING b/t/lib-diff/COPYING
similarity index 100%
rename from t/diff-lib/COPYING
rename to t/lib-diff/COPYING
diff --git a/t/diff-lib/README b/t/lib-diff/README
similarity index 100%
rename from t/diff-lib/README
rename to t/lib-diff/README
diff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
index e5116a76a1c..cce334981e1 100755
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -7,7 +7,7 @@ test_description='Test built-in diff output engine.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 echo >path0 'Line 1
 Line 2
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index c16486a9d41..2f9700742aa 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -7,7 +7,7 @@ test_description='Test rename detection in diff engine.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 test_expect_success 'setup' '
 	cat >path0 <<-\EOF &&
diff --git a/t/t4003-diff-rename-1.sh b/t/t4003-diff-rename-1.sh
index df2accb6555..db07ff3eb19 100755
--- a/t/t4003-diff-rename-1.sh
+++ b/t/t4003-diff-rename-1.sh
@@ -7,11 +7,11 @@ test_description='More rename detection
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 test_expect_success \
     'prepare reference tree' \
-    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+    'cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
      echo frotz >rezrov &&
     git update-index --add COPYING rezrov &&
     tree=$(git write-tree) &&
@@ -99,7 +99,7 @@ test_expect_success \
 
 test_expect_success \
     'prepare work tree once again' \
-    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+    'cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
      git update-index --add --remove COPYING COPYING.1'
 
 # tree has COPYING and rezrov.  work tree has COPYING and COPYING.1,
diff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh
index 6e562c80d12..3d495e37bb1 100755
--- a/t/t4004-diff-rename-symlink.sh
+++ b/t/t4004-diff-rename-symlink.sh
@@ -10,7 +10,7 @@ copy of symbolic links, but should not produce rename/copy followed
 by an edit for them.
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 test_expect_success SYMLINKS \
     'prepare reference tree' \
diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh
index d18a80493c2..86479061325 100755
--- a/t/t4005-diff-rename-2.sh
+++ b/t/t4005-diff-rename-2.sh
@@ -6,10 +6,10 @@
 test_description='Same rename detection as t4003 but testing diff-raw.'
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 test_expect_success 'setup reference tree' '
-	cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+	cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
 	echo frotz >rezrov &&
 	git update-index --add COPYING rezrov &&
 	tree=$(git write-tree) &&
@@ -64,7 +64,7 @@ test_expect_success 'validate output from rename/copy detection (#2)' '
 # nows how to say Copy.
 
 test_expect_success 'validate output from rename/copy detection (#3)' '
-	cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+	cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
 	git update-index --add --remove COPYING COPYING.1 &&
 
 	cat <<-EOF >expected &&
diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh
index b187b7f6c66..cbb9c62f535 100755
--- a/t/t4007-rename-3.sh
+++ b/t/t4007-rename-3.sh
@@ -7,17 +7,17 @@ test_description='Rename interaction with pathspec.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 test_expect_success 'prepare reference tree' '
 	mkdir path0 path1 &&
-	cp "$TEST_DIRECTORY"/diff-lib/COPYING path0/COPYING &&
+	cp "$TEST_DIRECTORY"/lib-diff/COPYING path0/COPYING &&
 	git update-index --add path0/COPYING &&
 	tree=$(git write-tree) &&
 	echo $tree
 '
 
-blob=$(git hash-object "$TEST_DIRECTORY/diff-lib/COPYING")
+blob=$(git hash-object "$TEST_DIRECTORY/lib-diff/COPYING")
 test_expect_success 'prepare work tree' '
 	cp path0/COPYING path1/COPYING &&
 	git update-index --add --remove path0/COPYING path1/COPYING
diff --git a/t/t4008-diff-break-rewrite.sh b/t/t4008-diff-break-rewrite.sh
index b1ccd4102e0..2299f27511b 100755
--- a/t/t4008-diff-break-rewrite.sh
+++ b/t/t4008-diff-break-rewrite.sh
@@ -22,11 +22,11 @@ With -B, this should be detected as two complete rewrites.
 Further, with -B and -M together, these should turn into two renames.
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 test_expect_success setup '
-	cat "$TEST_DIRECTORY"/diff-lib/README >file0 &&
-	cat "$TEST_DIRECTORY"/diff-lib/COPYING >file1 &&
+	cat "$TEST_DIRECTORY"/lib-diff/README >file0 &&
+	cat "$TEST_DIRECTORY"/lib-diff/COPYING >file1 &&
 	blob0_id=$(git hash-object file0) &&
 	blob1_id=$(git hash-object file1) &&
 	git update-index --add file0 file1 &&
diff --git a/t/t4009-diff-rename-4.sh b/t/t4009-diff-rename-4.sh
index b63bdf031f5..b1da807f169 100755
--- a/t/t4009-diff-rename-4.sh
+++ b/t/t4009-diff-rename-4.sh
@@ -7,11 +7,11 @@ test_description='Same rename detection as t4003 but testing diff-raw -z.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 test_expect_success \
     'prepare reference tree' \
-    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+    'cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
      echo frotz >rezrov &&
     git update-index --add COPYING rezrov &&
     orig=$(git hash-object COPYING) &&
@@ -81,7 +81,7 @@ test_expect_success \
 
 test_expect_success \
     'prepare work tree once again' \
-    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+    'cat "$TEST_DIRECTORY"/lib-diff/COPYING >COPYING &&
      git update-index --add --remove COPYING COPYING.1'
 
 git diff-index -z -C --find-copies-harder $tree >current
diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
index 65cc703c659..1bbced79ece 100755
--- a/t/t4010-diff-pathspec.sh
+++ b/t/t4010-diff-pathspec.sh
@@ -10,7 +10,7 @@ Prepare:
         path1/file1
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 test_expect_success \
     setup \
diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh
index 717034bb50b..5a25c259fe3 100755
--- a/t/t4011-diff-symlink.sh
+++ b/t/t4011-diff-symlink.sh
@@ -7,7 +7,7 @@ test_description='Test diff of symlinks.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 # Print the short OID of a symlink with the given name.
 symlink_oid () {
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index ce6aa3914fe..fcc30d8cc26 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -9,7 +9,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 test_expect_success setup '
 
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 8c574221b27..2c13b62d3c6 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -7,7 +7,7 @@ test_description='Test special whitespace in diff engine.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 test_expect_success "Ray Lehtiniemi's example" '
 	cat <<-\EOF >x &&
diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
index 894a11b224d..94ef77e1dfe 100755
--- a/t/t4027-diff-submodule.sh
+++ b/t/t4027-diff-submodule.sh
@@ -3,7 +3,7 @@
 test_description='difference in submodules'
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 test_expect_success setup '
 	test_tick &&
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 0c8fb39ceda..56f1e62a97b 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -3,7 +3,7 @@
 test_description='word diff colors'
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 cat >pre.simple <<-\EOF
 	h(4)
diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 09ad491a593..aeac203c424 100755
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -6,7 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 setup_helper () {
 	one=$1 branch=$2 side=$3 &&
diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
index bcf7493740a..7750b87ca16 100755
--- a/t/t4061-diff-indent.sh
+++ b/t/t4061-diff-indent.sh
@@ -7,7 +7,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 # Compare two diff outputs. Ignore "index" lines, because we don't
 # care about SHA-1s or file modes.
diff --git a/t/t4206-log-follow-harder-copies.sh b/t/t4206-log-follow-harder-copies.sh
index ad29e65fcba..4871a5dc92f 100755
--- a/t/t4206-log-follow-harder-copies.sh
+++ b/t/t4206-log-follow-harder-copies.sh
@@ -7,7 +7,7 @@ test_description='Test --follow should always find copies hard in git log.
 
 '
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
+. "$TEST_DIRECTORY"/lib-diff.sh
 
 echo >path0 'Line 1
 Line 2
diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh
index 0f936182e4f..512ae2781fe 100755
--- a/t/t7501-commit-basic-functionality.sh
+++ b/t/t7501-commit-basic-functionality.sh
@@ -11,7 +11,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY/diff-lib.sh"
+. "$TEST_DIRECTORY/lib-diff.sh"
 
 author='The Real Author <someguy@his.email.org>'
 
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 3d17e932a0e..3d77701fd4d 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -8,7 +8,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
+. "$TEST_DIRECTORY"/lib-diff.sh ;# test-lib chdir's into trash
 
 verify_packs () {
 	for p in .git/objects/pack/*.pack
-- 
2.30.0.284.gd98b1dd5eaa7

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

[Ævar Arnfjörð Bjarmason]

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 1d80fb70041..98d4164a4e6 100644
--- a/t/README
+++ b/t/README
@@ -911,13 +911,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 473d2e283e6..a5f63c0af6b 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

[PATCH v2 11/11] test-lib-functions: assert correct parameter count

[Ævar Arnfjörð Bjarmason]

Add assertions of the correct parameter count of various functions, in
particularly the wrappers for the shell "test" built-in.

In an earlier commit we fixed a bug with an incorrect number of
arguments being passed to "test_path_is_{file,missing}". Let's also
guard other similar functions from the same sort of misuse.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib-functions.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index a5f63c0af6b..a52eb16846c 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -728,6 +728,7 @@ test_path_is_file () {
 }
 
 test_path_is_dir () {
+	test "$#" -ne 1 && BUG "1 param"
 	if ! test -d "$1"
 	then
 		echo "Directory $1 doesn't exist"
@@ -746,6 +747,7 @@ test_path_exists () {
 
 # Check if the directory exists and is empty as expected, barf otherwise.
 test_dir_is_empty () {
+	test "$#" -ne 1 && BUG "1 param"
 	test_path_is_dir "$1" &&
 	if test -n "$(ls -a1 "$1" | egrep -v '^\.\.?$')"
 	then
@@ -757,6 +759,7 @@ test_dir_is_empty () {
 
 # Check if the file exists and has a size greater than zero
 test_file_not_empty () {
+	test "$#" = 2 && BUG "2 param"
 	if ! test -s "$1"
 	then
 		echo "'$1' is not a non-empty file."
@@ -765,6 +768,7 @@ test_file_not_empty () {
 }
 
 test_path_is_missing () {
+	test "$#" -ne 1 && BUG "1 param"
 	if test -e "$1"
 	then
 		echo "Path exists:"
@@ -801,6 +805,7 @@ test_line_count () {
 }
 
 test_file_size () {
+	test "$#" -ne 1 && BUG "1 param"
 	test-tool path-utils file-size "$1"
 }
 
@@ -973,6 +978,7 @@ test_expect_code () {
 # - not all diff versions understand "-u"
 
 test_cmp () {
+	test "$#" -ne 2 && BUG "2 param"
 	eval "$GIT_TEST_CMP" '"$@"'
 }
 
@@ -1002,6 +1008,7 @@ test_cmp_config () {
 # test_cmp_bin - helper to compare binary files
 
 test_cmp_bin () {
+	test "$#" -ne 2 && BUG "2 param"
 	cmp "$@"
 }
 
@@ -1062,6 +1069,7 @@ verbose () {
 # otherwise.
 
 test_must_be_empty () {
+	test "$#" -ne 1 && BUG "1 param"
 	test_path_is_file "$1" &&
 	if test -s "$1"
 	then
-- 
2.30.0.284.gd98b1dd5eaa7