On 2025-03-31 at 13:14:12, SZEDER Gábor wrote: > This test succeeds without checking everything it is supposed to: > > + git clone --upload-pack=GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack --no-local a nonlocal-otheruser > + repo_is_hardlinked nonlocal-otheruser > + find nonlocal-otheruser/objects -type f -links 1 > find: 'nonlocal-otheruser/objects': No such file or directory > + git -C nonlocal-otheruser rev-parse --show-toplevel > /home/szeder/src/git/t/trash directory.t5605-clone-local/nonlocal-otheruser > + test_grep detected dubious ownership err > error: 'grep detected dubious ownership err' didn't find a match in: > Cloning into 'nonlocal-otheruser'... > warning: remote HEAD refers to nonexistent ref, unable to checkout > ok 21 - cloning a local path with --no-local from a different user succeeds > > The 'repo_is_hardlinked' helper function expects the path to the .git > directory as parameter, but in this case it gets the top-level path of > the repository instead, which results in 'find' rightfully complaining > about the non-existing 'objects' directory. Thanks to the &&-chain in > 'repo_is_hardlinked' this error then causes the helper function to > return with non-zero error code, just what this test case expected. > > All other tests using this helper function create bare clones, and > they all pass the right path as parameter. > > The trivial fix would be to either use a bare clone in this test as > well, or to pass the right path to 'repo_is_hardlinked', i.e. the path > to the '.git' directory. > > The right fix, in my opinion, is to teach 'repo_is_hardlinked' a > negation parameter, so tests expecting the repo to be not hardlinked > could invoke it as 'repo_is_hardlinked ! <path>'. An error from > 'find' within the helper function should then always result in an > error of the helper function, i.e. both with and without that '!' > parameter. > > Furthermore, this test should use 'test_grep ! ...' instead of '! > test_grep ...', so we don't get that misleading error message in the > test's output. > > And there is a stray empty line at the end of the test as well. Thanks for the report. I've opted not to teach `repo_is_hardlinked` a negation parameter, but I have fixed the three items you've mentioned. Here's a patch. -- >% -- Subject: [PATCH] t5605: fix test for cloning from a different user This test currently passes, but for the wrong reason. The repo_is_hardlinked function expects a .git directory or a bare repository and currently fails because it cannot find the objects directory. One solution is to use the --bare argument, but then --show-toplevel won't work. We could change that, but there's no need to, so just add the missing .git directory. In addition, use the built-in negation functionality of test_grep to avoid mishandling real errors (such as a missing file) and, as a final fix, remove the extra newline. Reported-by: SZEDER Gábor <szeder.dev@xxxxxxxxx> Signed-off-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> --- t/t5605-clone-local.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh index 4605703496..2397f8fa61 100755 --- a/t/t5605-clone-local.sh +++ b/t/t5605-clone-local.sh @@ -156,11 +156,10 @@ test_expect_success 'cloning a local path with --no-local does not hardlink' ' test_expect_success 'cloning a local path with --no-local from a different user succeeds' ' git clone --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \ --no-local a nonlocal-otheruser 2>err && - ! repo_is_hardlinked nonlocal-otheruser && + ! repo_is_hardlinked nonlocal-otheruser/.git && # Verify that this is a git repository. git -C nonlocal-otheruser rev-parse --show-toplevel && - ! test_grep "detected dubious ownership" err - + test_grep ! "detected dubious ownership" err ' test_expect_success 'cloning locally respects "-u" for fetching refs' ' -- 2.49.0.395.g12beb8f557c -- >% -- -- brian m. carlson (they/them) Toronto, Ontario, CA
Attachment:
signature.asc
Description: PGP signature
