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

From: Junio C Hamano <[email-masked]>
Date: Mon, 28 Sep 2020 14:54:24 -0700

Rafael Silva <[email-masked]> 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 <[email-masked]>
> ---
>  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.