test-lib-functions: remove bug-inducing "diagnostics" helper param

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/README b/t/README
index c730a7077057d4104a8d4f18737309a2e2dcbdf4..df38d8d6be3ef3610b9ea9e3b10bc73285fc46da 100644 (file)
--- 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 1e738df81d574e2e68d343a67afae266443e39c6..28c2d15d8071207234e2f3a137346097e2d7af34 100755 (executable)
--- 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 d7eeee431061c4195b91a86581fd8462551ceb42..7b5f1c1dcd1f68c74646a5983b6b0d53bd9b4a92 100755 (executable)
--- 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 4a3b8f48ac638d05759377524c0e9566e36e94a9..7879a6b8c51a28a34af3f393a3d02bd985c30d79 100755 (executable)
--- 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 f228647c2b8b88f501d32445e7c2340dab9f8c43..109d1b548ce4ed4798e332fa31ddc404e1b49fc0 100644 (file)
--- 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
 }