Hi Gábor,
Le 2021-01-06 à 08:16, SZEDER Gábor a écrit :
On Mon, Dec 28, 2020 at 07:41:44PM +0000, Philippe Blain via GitGitGadget wrote:
To prevent future regressions, add a simple test that counts the number
of tools shown by 'git mergetool --tool-help', irrespective of their
installed status, by making use of the fact that mergetools are listed
by 'git mergetool --tool-help' on lines starting with tabs. Prefix the
`git config` commands used at the beginning of the test to remove the
fake tools used by the previous test with 'test_might_fail' so that the
test can be run independantly of the previous test without failing.
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 70afdd06fa7..ebd3af139e5 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -828,4 +828,14 @@ test_expect_success 'mergetool -Oorder-file is honored' '
test_cmp expect actual
'
+test_expect_success 'mergetool --tool-help shows all recognized tools' '
+ # Remove fake tools added in previous tests
+ test_might_fail git config --unset merge.tool &&
+ test_might_fail git config --remove-section mergetool.mytool &&
+ test_might_fail git config --remove-section mergetool.mybase &&
+ git mergetool --tool-help >output &&
+ grep "$(printf "\t")" output >mergetools &&
+ test_line_count = 30 mergetools
+'
This new test fails when the topic 'pb/mergetool-tool-help-fix' is
built and tested in isolation, because 'git mergetool --tool-help'
lists only 29 tools instead of the expected 30. The reason is that
'pb/mergetool-tool-help-fix' doesn't include commit 6bc9082c0f
(mergetools/bc: add `bc4` to the alias list for Beyond Compare,
2020-11-11), which added that 30th tool (and is already part of
v2.30.0).
Indeed. The branch I submitted is based on v2.30.0-rc2, but Junio based
pb/mergetool-tool-help-fix on v2.29.0-rc0~165^2, so the number of supported
tools is "wrong".
It also makes me wonder whether this is the right way to test this
fix, because we'll need to adjust this test case every time we add
support for a new merge tool (which arguably doesn't happen that
often, but since we are already at 30 it doesn't seem to be that rare
either).
Yes, it does. I thought about that, and I came to the conclusion that it
was the easiest way to prevent a regression like the one this patch is fixing.
I tought about doing it a different way, like having the test count the available
mergetools itself, and comparing the number of tools shown by '--tool-help' with
that number, but I chose not to do that because it seemed to be more complicated
(and would end up kind of re-implementing what '--tool-help' does, in a way...)
If you think of another of how we could test it, I can try it.
Cheers,
Philippe.
