Re: [PATCH v3 6/7] t/t1517: move verify-commit -h test to t1517

[Junio C Hamano <gitster@pobox.com>]

Usman Akinyemi <usmanakinyemi202@gmail.com> writes:

>> But in the longer run, we are very much likely that we'd want to
>> test something that needs things that require prerequisites (like
>> "do this only where XYZ is installed") but ought to work outside a
>> repository, which means t1517 would need to pull in things like
>> lib-gpg.sh only because it has a few tests about verify-blah
>> command.  These tend to accumulate over time.
>
> I understand the concern and I felt we should at least decide where to
> put the "verify -h" because, right now, we have some of them in the
> t1517 and also some in their respective test files.

If t1517 were only about "git subcmd -h outside a repository" for
various subcommands, that would be a happy arrangement.  I think we
even have a way to iterate over all Git subcommands, current or
future, so your patch may become "we've sprinkled 'subcmd -h' tests
in various scripts, but now t1517 will do that automatically so
anybody who add a new command do not have to do anything".

But if t1517 currently (before your series) already has other things
tested, that changes the story somewhat.  Especially if we aim for
the automated solution, we may want to move existing tests in 1517
that is not about "subcmd -h" out to different scripts.  Obvious
two choices are:

 - We spread them to existing test scripts for the command being
   tested (e.g. "does patch-id work correctly outside a repo?"
   moves to t4204-patch-id, and "does update-server-info work OK
   inside and outside a repo?" can be split and one half moves to
   t5200-update-server-info).

 - We move them all to a new test script that is dedicated for "do
   various subcommands work outside a repo to do things other than
   responding to '-h'?".

and I would favour the former.

The only reason why you moved these to t1517 is because the set-up
part of that script sets up the ceiling just once properly and let
its tests do as if they are running outsie a repository, and having
to arrange the ceiling correctly to add a few test in various
scripts so that each of these scripts can test their single
subcommand pretending that it is running outside a repository looked
cumbersome, right?  And I do agree with you, if that was the reason,
that it is annoying to have to set up the ceiling manually in each
test script.  But then can we do it less annoying?  We already made
a nongit test helper and it may be good enough to help existing
tests in t1517.

As an illustration, here is what the beginning of the former
approach may look like.


 t/t1517-outside-repo.sh       | 27 +++++++++++++++++++++------
 t/t5200-update-server-info.sh |  5 +++++
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git c/t/t1517-outside-repo.sh w/t/t1517-outside-repo.sh
index 6824581317..c1294d5761 100755
--- c/t/t1517-outside-repo.sh
+++ w/t/t1517-outside-repo.sh
@@ -107,11 +107,26 @@ test_expect_success LIBCURL 'remote-http outside repository' '
 	test_grep "^error: remote-curl" actual
 '
 
-test_expect_success 'update-server-info does not crash with -h' '
-	test_expect_code 129 git update-server-info -h >usage &&
-	test_grep "[Uu]sage: git update-server-info " usage &&
-	test_expect_code 129 nongit git update-server-info -h >usage &&
-	test_grep "[Uu]sage: git update-server-info " usage
-'
+for cmd in $(git --list-cmds=main)
+do
+	cmd=${cmd%.*} # strip .sh, .perl, etc.
+	case "$cmd" in
+	archimport | cvsexportcommit | cvsimport | cvsserver | daemon | \
+	difftool--helper | filter-branch | fsck-objects | get-tar-commit-id | \
+	http-backend | http-fetch | http-push | init-db | instaweb.sh | \
+	merge-octopus | merge-one-file | merge-resolve | mergetool | \
+	mktag | p4 | p4.py | pickaxe | quiltimport | remote-ftp | remote-ftps | \
+	remote-http | remote-https | replay | request-pull | send-email | \
+	sh-i18n--envsubst | shell | show | stage | submodule | svn | \
+	upload-archive--writer | upload-pack | web--browse | whatchanged)
+		expect_outcome=expect_failure ;;
+	*)
+		expect_outcome=expect_success ;;
+	esac
+	test_$expect_outcome "'git $cmd -h' outside a repository" '
+		test_expect_code 129 nongit git $cmd -h >usage &&
+		test_grep "[Uu]sage: git $cmd " usage
+	'
+done
 
 test_done
diff --git c/t/t5200-update-server-info.sh w/t/t5200-update-server-info.sh
index 8365907055..a551e955b5 100755
--- c/t/t5200-update-server-info.sh
+++ w/t/t5200-update-server-info.sh
@@ -46,4 +46,9 @@ test_expect_success 'midx does not create duplicate pack entries' '
 	test_must_be_empty dups
 '
 
+test_expect_success 'update-server-info does not crash with -h' '
+	test_expect_code 129 git update-server-info -h >usage &&
+	test_grep "[Uu]sage: git update-server-info " usage
+'
+
 test_done