@matheusbsbs / matheus-bagliano / commits / a48a88019b

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 2c02b110da (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 <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

SZEDER Gábor committed Dec 18, 2022 at 17:29 UTC a48a88019b408157cf3b76e1a198afd824f4f0a3
6 files changed +10 -9
t/t0000-basic.sh
+4 -3
@@ -815,7 +815,8 @@ test_expect_success 'test_oid provides sane info by default' '
815 grep "^00*\$" actual &&
816 rawsz="$(test_oid rawsz)" &&
817 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) &&
820 test $(( $rawsz * 2)) -eq "$hexsz"
821 '
822
@@ -826,7 +827,7 @@ test_expect_success 'test_oid can look up data for SHA-1' '
827 grep "^00*\$" actual &&
828 rawsz="$(test_oid rawsz)" &&
829 hexsz="$(test_oid hexsz)" &&
829 - test $(wc -c <actual) -eq 40 &&
830 + test $(wc -c <actual) -eq 41 &&
831 test "$rawsz" -eq 20 &&
832 test "$hexsz" -eq 40
833 '
@@ -838,7 +839,7 @@ test_expect_success 'test_oid can look up data for SHA-256' '
839 grep "^00*\$" actual &&
840 rawsz="$(test_oid rawsz)" &&
841 hexsz="$(test_oid hexsz)" &&
841 - test $(wc -c <actual) -eq 64 &&
842 + test $(wc -c <actual) -eq 65 &&
843 test "$rawsz" -eq 32 &&
844 test "$hexsz" -eq 64
845 '
t/t1302-repo-version.sh
+1 -1
@@ -27,7 +27,7 @@ test_expect_success 'setup' '
27 '
28
29 test_expect_success 'gitdir selection on normal repos' '
30 - echo $(test_oid version) >expect &&
30 + test_oid version >expect &&
31 git config core.repositoryformatversion >actual &&
32 git -C test config core.repositoryformatversion >actual2 &&
33 test_cmp expect actual &&
t/t1500-rev-parse.sh
+1 -1
@@ -195,7 +195,7 @@ test_expect_success 'rev-parse --is-shallow-repository in non-shallow repo' '
195 '
196
197 test_expect_success 'rev-parse --show-object-format in repo' '
198 - echo "$(test_oid algo)" >expect &&
198 + test_oid algo >expect &&
199 git rev-parse --show-object-format >actual &&
200 test_cmp expect actual &&
201 git rev-parse --show-object-format=storage >actual &&
t/t4044-diff-index-unique-abbrev.sh
+2 -2
@@ -34,12 +34,12 @@ test_expect_success 'setup' '
34 100644 blob $(test_oid hash2) foo
35 EOF
36
37 - echo "$(test_oid val1)" > foo &&
37 + test_oid val1 > foo &&
38 git add foo &&
39 git commit -m "initial" &&
40 git cat-file -p HEAD: > actual &&
41 test_cmp expect_initial actual &&
42 - echo "$(test_oid val2)" > foo &&
42 + test_oid val2 > foo &&
43 git commit -a -m "update" &&
44 git cat-file -p HEAD: > actual &&
45 test_cmp expect_update actual
t/t5313-pack-bounds-checks.sh
+1 -1
@@ -59,7 +59,7 @@ test_expect_success 'setup' '
59 test_expect_success 'set up base packfile and variables' '
60 # the hash of this content starts with ff, which
61 # makes some later computations much simpler
62 - echo $(test_oid oidfff) >file &&
62 + test_oid oidfff >file &&
63 git add file &&
64 git commit -m base &&
65 git repack -ad &&
t/test-lib-functions.sh
+1 -1
@@ -1682,7 +1682,7 @@ test_oid () {
1682 then
1683 BUG "undefined key '$1'"
1684 fi &&
1685 - eval "printf '%s' \"\${$var}\""
1685 + eval "printf '%s\n' \"\${$var}\""
1686 }
1687
1688 # Insert a slash into an object ID so it can be used to reference a location