Skip to content

Commit 45a2686

Browse files
avargitster
authored andcommitted
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 2caf20c (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 ae4e89e (gc: add --keep-largest-pack option, 2018-04-15), and c04ba51 (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ebd73f5 commit 45a2686

File tree

5 files changed

+26
-16
lines changed

5 files changed

+26
-16
lines changed

t/README

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -917,13 +917,13 @@ library for your script to use.
917917

918918
Check whether a file has the length it is expected to.
919919

920-
- test_path_is_file <path> [<diagnosis>]
921-
test_path_is_dir <path> [<diagnosis>]
922-
test_path_is_missing <path> [<diagnosis>]
920+
- test_path_is_file <path>
921+
test_path_is_dir <path>
922+
test_path_is_missing <path>
923923

924924
Check if the named path is a file, if the named path is a
925925
directory, or if the named path does not exist, respectively,
926-
and fail otherwise, showing the <diagnosis> text.
926+
and fail otherwise.
927927

928928
- test_when_finished <script>
929929

t/t3404-rebase-interactive.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ test_expect_success 'rebase -i with the exec command' '
101101
) &&
102102
test_path_is_file touch-one &&
103103
test_path_is_file touch-two &&
104-
test_path_is_missing touch-three " (should have stopped before)" &&
104+
# Missing because we should have stopped by now.
105+
test_path_is_missing touch-three &&
105106
test_cmp_rev C HEAD &&
106107
git rebase --continue &&
107108
test_path_is_file touch-three &&

t/t6426-merge-skip-unneeded-updates.sh

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,9 @@ test_expect_success '3a-L: bq_1->foo/bq_2 on A, foo/->bar/ on B' '
492492
test_cmp expect actual &&
493493
494494
test_must_fail git rev-parse HEAD:bq HEAD:foo/bq &&
495-
test_path_is_missing bq foo/bq foo/whatever
495+
test_path_is_missing bq &&
496+
test_path_is_missing foo/bq &&
497+
test_path_is_missing foo/whatever
496498
)
497499
'
498500

@@ -522,7 +524,9 @@ test_expect_success '3a-R: bq_1->foo/bq_2 on A, foo/->bar/ on B' '
522524
test_cmp expect actual &&
523525
524526
test_must_fail git rev-parse HEAD:bq HEAD:foo/bq &&
525-
test_path_is_missing bq foo/bq foo/whatever
527+
test_path_is_missing bq &&
528+
test_path_is_missing foo/bq &&
529+
test_path_is_missing foo/whatever
526530
)
527531
'
528532

@@ -588,7 +592,9 @@ test_expect_success '3b-L: bq_1->foo/bq_2 on A, foo/->bar/ on B' '
588592
test_cmp expect actual &&
589593
590594
test_must_fail git rev-parse HEAD:bq HEAD:foo/bq &&
591-
test_path_is_missing bq foo/bq foo/whatever
595+
test_path_is_missing bq &&
596+
test_path_is_missing foo/bq &&
597+
test_path_is_missing foo/whatever
592598
)
593599
'
594600

@@ -618,7 +624,9 @@ test_expect_success '3b-R: bq_1->foo/bq_2 on A, foo/->bar/ on B' '
618624
test_cmp expect actual &&
619625
620626
test_must_fail git rev-parse HEAD:bq HEAD:foo/bq &&
621-
test_path_is_missing bq foo/bq foo/whatever
627+
test_path_is_missing bq &&
628+
test_path_is_missing foo/bq &&
629+
test_path_is_missing foo/whatever
622630
)
623631
'
624632

t/t6500-gc.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ test_expect_success 'gc --keep-largest-pack' '
7878
git gc &&
7979
( cd .git/objects/pack && ls *.pack ) >pack-list &&
8080
test_line_count = 1 pack-list &&
81-
BASE_PACK=.git/objects/pack/pack-*.pack &&
81+
cp pack-list base-pack-list &&
8282
test_commit four &&
8383
git repack -d &&
8484
test_commit five &&
@@ -90,7 +90,7 @@ test_expect_success 'gc --keep-largest-pack' '
9090
test_line_count = 2 pack-list &&
9191
awk "/^P /{print \$2}" <.git/objects/info/packs >pack-info &&
9292
test_line_count = 2 pack-info &&
93-
test_path_is_file $BASE_PACK &&
93+
test_path_is_file .git/objects/pack/$(cat base-pack-list) &&
9494
git fsck
9595
)
9696
'

t/test-lib-functions.sh

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -717,28 +717,29 @@ test_external_without_stderr () {
717717
}
718718

719719
# debugging-friendly alternatives to "test [-f|-d|-e]"
720-
# The commands test the existence or non-existence of $1. $2 can be
721-
# given to provide a more precise diagnosis.
720+
# The commands test the existence or non-existence of $1
722721
test_path_is_file () {
722+
test "$#" -ne 1 && BUG "1 param"
723723
if ! test -f "$1"
724724
then
725-
echo "File $1 doesn't exist. $2"
725+
echo "File $1 doesn't exist"
726726
false
727727
fi
728728
}
729729

730730
test_path_is_dir () {
731731
if ! test -d "$1"
732732
then
733-
echo "Directory $1 doesn't exist. $2"
733+
echo "Directory $1 doesn't exist"
734734
false
735735
fi
736736
}
737737

738738
test_path_exists () {
739+
test "$#" -ne 1 && BUG "1 param"
739740
if ! test -e "$1"
740741
then
741-
echo "Path $1 doesn't exist. $2"
742+
echo "Path $1 doesn't exist"
742743
false
743744
fi
744745
}

0 commit comments

Comments
 (0)