Re: [PATCH v3 06/30] subtree: t7900: use 'test' for string equality
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-04-30 09:57:44
On Tue, Apr 27 2021, Luke Shumaker wrote:
quoted hunk ↗ jump to hunk
From: Luke Shumaker <redacted> t7900-subtree.sh defines its own `check_equal A B` function, instead of just using `test A = B` like all of the other tests. Don't be special, get rid of `check_equal` in favor of `test`. Signed-off-by: Luke Shumaker <redacted> --- contrib/subtree/t/t7900-subtree.sh | 60 ++++++++++++------------------ 1 file changed, 24 insertions(+), 36 deletions(-)diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index 12b8cb03c7..76183153c9 100755 --- a/contrib/subtree/t/t7900-subtree.sh +++ b/contrib/subtree/t/t7900-subtree.sh@@ -28,18 +28,6 @@ create () { git add "$1" } -check_equal () { - test_debug 'echo' - test_debug "echo \"check a:\" \"{$1}\"" - test_debug "echo \" b:\" \"{$2}\"" - if test "$1" = "$2" - then - return 0 - else - return 1 - fi -}
It looks like the reason this was used because when this fails just
having the "test" makes for bad debugging. I.e. if the values don't
match the $1 and $2 are not aligned, so it's hard to eyeball what went
wrong.
These days this is more idiomatic:
echo "Add [...]" >expected
last_commit_message >actual &&
test_cmp expected actual
So I think in this case a better narrower improvement would be to keep
the check_equal function. I wonder if we shouldn't just in general in
t/test-lib.sh have a test_cmp_str for this use-case. I.e. a trivial
wrapper that echos the two strings to a file for you, before running
diff(1).