Re: [RFC PATCH 2/2] t2402: add test to locked linked worktree marker

> Rafael Silva <rafaeloliveira.cs@gmail.com> writes:
> 
> > Test the output of the `worktree list` command to show when
> > a linked worktree is locked and test to not mistakenly
> > mark main or unlocked worktrees.
> >
> > Signed-off-by: Rafael Silva <rafaeloliveira.cs@gmail.com>
> > ---
> >  t/t2402-worktree-list.sh | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> 
> I think this should be part of [1/2], as the change necessary to
> implement the new feature is small enough that there is no reason
> to split the test part out.

> 
> > diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
> > index 52585ec2aa..07bd9a3350 100755
> > --- a/t/t2402-worktree-list.sh
> > +++ b/t/t2402-worktree-list.sh
> > @@ -61,6 +61,19 @@ test_expect_success '"list" all worktrees --porcelain' '
> >  	test_cmp expect actual
> >  '
> >  
> > +test_expect_success 'show locked worktree with (locked)' '
> > +	echo "$(git rev-parse --show-toplevel) $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]" >expect &&
> > +	test_when_finished "rm -rf locked unlocked out actual expect && git worktree prune" &&
> > +	git worktree add --detach locked master &&
> > +	git worktree add --detach unlocked master &&
> > +	git worktree lock locked &&
> > +	echo "$(git -C locked rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD) (locked)" >>expect &&
> > +	echo "$(git -C unlocked rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
> > +	git worktree list >out &&
> > +	sed "s/  */ /g" <out >actual &&
> > +	test_cmp expect actual
> > +'
> 
> This seems to prescribe the output from the command too strictly
> (you do avoid being overly too strict by removing the indentation
> with 's/ */ /g' though).  
> 
> If the leading path to the $TRASH_DIRECTORY has two or more
> consecutive SPs (and that is not something under our control), the
> 'expect' file would keep such a double-SP, but such a double-SP in
> 'out' would have been squashed out in the 'actual' file.
> 
> I wonder if
> 
> 	grep '/locked  *[0-9a-f].* (locked)' out &&
> 	! grep '/unlocked  *[0-9a-f].* (locked)' out
> 
> might be a better way to test?  That is
> 
>  - we do not care what the leading directories are called
>  - we do not care what branch is checked out or how they are presented
>  - we care the one that ends with /locked is (locked)
>  - we care the one that ends with /unlocked is not (locked)
> 
> After all, this new test piece is not about verifying that the
> object name or branch name is correct.