Skip to content

Commit a48a880

Browse files
szedergitster
authored andcommitted
tests: make 'test_oid' print trailing newline
Unlike other test helper functions, 'test_oid' doesn't terminate its output with a LF, but, alas, the reason for this, if any, is not mentioned in 2c02b11 (t: add test functions to translate hash-related values, 2018-09-13)). Now, in the vast majority of cases 'test_oid' is invoked in a command substitution that is part of a heredoc or supplies an argument to a command or the value to a variable, and the command substitution would chop off any trailing LFs, so in these cases the lack or presence of a trailing LF in its output doesn't matter. However: - There appear to be only three cases where 'test_oid' is not invoked in a command substitution: $ git grep '\stest_oid ' -- ':/t/*.sh' t0000-basic.sh: test_oid zero >actual && t0000-basic.sh: test_oid zero >actual && t0000-basic.sh: test_oid zero >actual && These are all in test cases checking that 'test_oid' actually works, and that the size of its output matches the size of the corresponding hash function with conditions like test $(wc -c <actual) -eq 40 In these cases the lack of trailing LF does actually matter, though they could be trivially updated to account for the presence of a trailing LF. - There are also a few cases where the lack of trailing LF in 'test_oid's output actually hurts, because tests need to compare its output with LF terminated file contents, forcing developers to invoke it as 'echo $(test_oid ...)' to append the missing LF: $ git grep 'echo "\?$(test_oid ' -- ':/t/*.sh' t1302-repo-version.sh: echo $(test_oid version) >expect && t1500-rev-parse.sh: echo "$(test_oid algo)" >expect && t4044-diff-index-unique-abbrev.sh: echo "$(test_oid val1)" > foo && t4044-diff-index-unique-abbrev.sh: echo "$(test_oid val2)" > foo && t5313-pack-bounds-checks.sh: echo $(test_oid oidfff) >file && And there is yet another similar case in an in-flight topic at: https://public-inbox.org/git/813e81a058227bd373cec802e443fcd677042fb4.1670862677.git.gitgitgadget@gmail.com/ Arguably we would be better off if 'test_oid' terminated its output with a LF. So let's update 'test_oid' accordingly, update its tests in t0000 to account for the extra character in those size tests, and remove the now unnecessary 'echo $(...)' command substitutions around 'test_oid' invocations as well. Signed-off-by: SZEDER Gábor <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 57e2c6e commit a48a880

File tree

6 files changed

+10
-9
lines changed

6 files changed

+10
-9
lines changed

t/t0000-basic.sh

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,8 @@ test_expect_success 'test_oid provides sane info by default' '
815815
grep "^00*\$" actual &&
816816
rawsz="$(test_oid rawsz)" &&
817817
hexsz="$(test_oid hexsz)" &&
818-
test "$hexsz" -eq $(wc -c <actual) &&
818+
# +1 accounts for the trailing newline
819+
test $(( $hexsz + 1)) -eq $(wc -c <actual) &&
819820
test $(( $rawsz * 2)) -eq "$hexsz"
820821
'
821822

@@ -826,7 +827,7 @@ test_expect_success 'test_oid can look up data for SHA-1' '
826827
grep "^00*\$" actual &&
827828
rawsz="$(test_oid rawsz)" &&
828829
hexsz="$(test_oid hexsz)" &&
829-
test $(wc -c <actual) -eq 40 &&
830+
test $(wc -c <actual) -eq 41 &&
830831
test "$rawsz" -eq 20 &&
831832
test "$hexsz" -eq 40
832833
'
@@ -838,7 +839,7 @@ test_expect_success 'test_oid can look up data for SHA-256' '
838839
grep "^00*\$" actual &&
839840
rawsz="$(test_oid rawsz)" &&
840841
hexsz="$(test_oid hexsz)" &&
841-
test $(wc -c <actual) -eq 64 &&
842+
test $(wc -c <actual) -eq 65 &&
842843
test "$rawsz" -eq 32 &&
843844
test "$hexsz" -eq 64
844845
'

t/t1302-repo-version.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ test_expect_success 'setup' '
2727
'
2828

2929
test_expect_success 'gitdir selection on normal repos' '
30-
echo $(test_oid version) >expect &&
30+
test_oid version >expect &&
3131
git config core.repositoryformatversion >actual &&
3232
git -C test config core.repositoryformatversion >actual2 &&
3333
test_cmp expect actual &&

t/t1500-rev-parse.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ test_expect_success 'rev-parse --is-shallow-repository in non-shallow repo' '
195195
'
196196

197197
test_expect_success 'rev-parse --show-object-format in repo' '
198-
echo "$(test_oid algo)" >expect &&
198+
test_oid algo >expect &&
199199
git rev-parse --show-object-format >actual &&
200200
test_cmp expect actual &&
201201
git rev-parse --show-object-format=storage >actual &&

t/t4044-diff-index-unique-abbrev.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,12 @@ test_expect_success 'setup' '
3434
100644 blob $(test_oid hash2) foo
3535
EOF
3636
37-
echo "$(test_oid val1)" > foo &&
37+
test_oid val1 > foo &&
3838
git add foo &&
3939
git commit -m "initial" &&
4040
git cat-file -p HEAD: > actual &&
4141
test_cmp expect_initial actual &&
42-
echo "$(test_oid val2)" > foo &&
42+
test_oid val2 > foo &&
4343
git commit -a -m "update" &&
4444
git cat-file -p HEAD: > actual &&
4545
test_cmp expect_update actual

t/t5313-pack-bounds-checks.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ test_expect_success 'setup' '
5959
test_expect_success 'set up base packfile and variables' '
6060
# the hash of this content starts with ff, which
6161
# makes some later computations much simpler
62-
echo $(test_oid oidfff) >file &&
62+
test_oid oidfff >file &&
6363
git add file &&
6464
git commit -m base &&
6565
git repack -ad &&

t/test-lib-functions.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1682,7 +1682,7 @@ test_oid () {
16821682
then
16831683
BUG "undefined key '$1'"
16841684
fi &&
1685-
eval "printf '%s' \"\${$var}\""
1685+
eval "printf '%s\n' \"\${$var}\""
16861686
}
16871687

16881688
# Insert a slash into an object ID so it can be used to reference a location

0 commit comments

Comments
 (0)