On 22/11/04 02:37AM, Ævar Arnfjörð Bjarmason wrote: > > On Fri, Nov 04 2022, Jacob Abel wrote: > > We usually add tests along with the feature, so this should be squashed > into your 3/4. > > > Signed-off-by: Jacob Abel <jacobabel@xxxxxxxxxx> > > --- > > t/t2400-worktree-add.sh | 54 +++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 54 insertions(+) > > > > diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh > > index d587e0b20d..064e1336e2 100755 > > --- a/t/t2400-worktree-add.sh > > +++ b/t/t2400-worktree-add.sh > > @@ -310,6 +310,26 @@ test_expect_success '"add" -B/--detach mutually exclusive' ' > > test_must_fail git worktree add -B poodle --detach bamboo main > > ' > > > > +test_expect_success '"add" --orphan/-b mutually exclusive' ' > > + test_must_fail git worktree add --orphan poodle -b poodle bamboo main > > +' > > + > > +test_expect_success '"add" --orphan/-B mutually exclusive' ' > > + test_must_fail git worktree add --orphan poodle -B poodle bamboo main > > +' > > + > > +test_expect_success '"add" --orphan/--detach mutually exclusive' ' > > + test_must_fail git worktree add --orphan poodle --detach bamboo main > > +' > > + > > +test_expect_success '"add" --orphan/--no-checkout mutually exclusive' ' > > + test_must_fail git worktree add --orphan poodle --no-checkout bamboo main > > +' > > + > > +test_expect_success '"add" -B/--detach mutually exclusive' ' > > + test_must_fail git worktree add -B poodle --detach bamboo main > > +' > > + > > [...] > > +test_expect_success '"add --orphan"' ' > > + git worktree add --orphan neworphan orphandir main && > > + git -C orphandir symbolic-ref HEAD >actual && > > + echo refs/heads/neworphan >expected && > > + test_cmp expected actual && > > nit: we usually do: > > echo ... > git ... > test_cmp > > So there's never doubt that the "expected" is before the command > (i.e. not impacted by it). > > > + ( > > + cd orphandir && > > + git diff main > > + ) > > Here you can avoid the sub-shell, with: > > git -C orphandir diff main > > > +' > > + > > +test_expect_success '"add --orphan" fails if the branch already exists' ' > > + git worktree add -b existingbranch orphandir2 main && > > + test_must_fail git worktree add --orphan existingbranch orphandir3 main && > > + [ ! -d orphandir3 ] > > Don't use "[", use "test", but in this case use one of the "test -d", > "test -f" etc. wrappers in test-lib-functions.sh. > > > +' > > + > > +test_expect_success '"add --orphan" fails if the commit-ish doesnt exist' ' > > + test_must_fail git worktree add --orphan badcommitish orphandir4 eee2222 && > > + [ ! -d orphandir4 ] > > ditto. Noted. Will make the above requested changes in v2. > > +' > > + > > +test_expect_success '"add --orphan" with empty repository' ' > > + ( > > + mkdir emptyorphanrepo && > > + cd emptyorphanrepo && > > + GIT_DIR=".git" git init --bare && > > + git worktree add --orphan newbranch worktreedir && > > + git -C worktreedir symbolic-ref HEAD >actual && > > + echo refs/heads/newbranch >expected && > > + test_cmp expected actual > > + ) > > Ditto we can avoid sub-shelling here, also when using sub-shells, make > the "cd" the first command in it, if you keep it the "mkdir" should go > outside of it. > > But isn't this (untested, maybe I'm missing a subtlety): > > test_when_finished "rm -rf repo" && > GIT_DIR=.git git init --bare repo && > git -C worktree ... > git -C worktree/subdir .. > echo refs ... > test_cmp ... > > I.e. do we need to create the dir manually before "git init --bare"? Yes, these should be essentially equivalent. I'll update the tests accordingly.