Re: [PATCH v3 6/7] t/t1517: move verify-commit -h test to t1517
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 11, 2025, 16:37 UTC
- Message-ID
- <xmqqcya63cqx.fsf@gitster.g>
- In-Reply-To
- <CAPSxiM_ZZrbFpgvxqYgZ8oeTbRs+HW=rM+9Dud0G_Qr7eq3=FA@mail.gmail.com>
Usman Akinyemi <usmanakinyemi202@gmail.com> writes:
Show 10 quoted lines
>> 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