On Tue, Feb 27, 2024 at 09:21:48AM +0100, Patrick Steinhardt wrote: > > +test_expect_success '__git_complete_worktree_paths - not a git repository' ' > > + ( > > + cd non-repo && > > + GIT_CEILING_DIRECTORIES="$ROOT" && > > + export GIT_CEILING_DIRECTORIES && > > + test_completion "git worktree remove " "" 2>err && > > + test_must_be_empty err > > + ) > > +' > > If I understand correctly, we assume that the repo isn't detected here, > and thus we will fail to complete the command. We don't want an error > message though, which we assert. Correct. > But do we also want to assert that > there is no output on stdout? To me, the check makes sense; to notice if we leak a message in such a circumstance, for instance. I can drop it if you think it does not add value. The test for stderr is my main goal here. > > > + > > +test_expect_success '__git_complete_worktree_paths with -C' ' > > + test_when_finished "rm -rf to_delete" && > > What does this delete? I don't see "to_delete" being created as part of > this test. Good eyes. It's noise. I'll drop this line. Thanks. > > > + git -C otherrepo worktree add --orphan otherrepo_wt && > > + run_completion "git -C otherrepo worktree remove " && > > + grep otherrepo_wt out > > And as far as I can see, we don't write to "out" in this test, either. > So I think we're accidentally relying on state by the first test here. The function run_completion leaves the result of the completion in the file "out". So we're checking here if "otherrepo_wt" is present in what "git -C otherrepo worktree remove <TAB>" returns. Maybe a new function: grep_completion, similar to test_completion, could make this clearer? > > Patrick Thanks. > > > +' > > + > > test_expect_success 'git switch - with no options, complete local branches and unique remote branch names for DWIM logic' ' ow > > test_completion "git switch " <<-\EOF > > branch-in-other Z > > -- > > 2.44.0.1.g0da3aa8f7f > >
