git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rafael Silva <rafaeloliveira.cs@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC PATCH 2/2] t2402: add test to locked linked worktree marker
Date: Tue, 29 Sep 2020 21:37:47 +0000	[thread overview]
Message-ID: <20200929213747.GC1336@contrib-buster.localdomain> (raw)
In-Reply-To: <xmqq3631lg8f.fsf@gitster.c.googlers.com>

On Mon, Sep 28, 2020 at 02:54:24PM -0700, Junio C Hamano wrote:
> 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.

Sounds good

> 
> > 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.

Sounds good and I agree with the all, and seems even easier
to understand the test code this way. Will address this change on the next patch,
although I'm not sure whether somebody could run into any issues when
running the `grep` command in other platforms. I'll look into it.

  reply	other threads:[~2020-09-29 21:37 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-28 15:49 [RFC PATCH 0/2] teach `worktree list` to mark locked worktrees Rafael Silva
2020-09-28 15:49 ` [RFC PATCH 1/2] worktree: teach `list` to mark locked worktree Rafael Silva
2020-09-28 21:37   ` Junio C Hamano
2020-09-29 21:36     ` Rafael Silva
2020-09-30  7:35     ` Eric Sunshine
2020-09-28 15:49 ` [RFC PATCH 2/2] t2402: add test to locked linked worktree marker Rafael Silva
2020-09-28 21:54   ` Junio C Hamano
2020-09-29 21:37     ` Rafael Silva [this message]
2020-09-30  8:06   ` Eric Sunshine
2020-09-28 21:19 ` [RFC PATCH 0/2] teach `worktree list` to mark locked worktrees Junio C Hamano
2020-09-29 21:35   ` Rafael Silva
2020-09-30  7:19 ` Eric Sunshine
2020-10-02 16:28   ` Rafael Silva
2020-10-09 22:50     ` Eric Sunshine
2020-10-10 19:06       ` Rafael Silva
2020-10-10 18:55 ` [PATCH v2 0/1] Teach "worktree list" to annotate " Rafael Silva
2020-10-10 18:55   ` [PATCH v2 1/1] worktree: teach `list` to annotate locked worktree Rafael Silva
2020-10-11  6:26     ` Eric Sunshine
2020-10-11  6:34       ` Eric Sunshine
2020-10-11 10:04       ` Rafael Silva
2020-10-11 10:11   ` [PATCH v3 0/1] Teach "worktree list" to annotate locked worktrees Rafael Silva
2020-10-11 10:11     ` [PATCH v3] worktree: teach `list` to annotate locked worktree Rafael Silva
2020-10-12  2:24       ` Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200929213747.GC1336@contrib-buster.localdomain \
    --to=rafaeloliveira.cs@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).