All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pete Wyckoff <pw@padd.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Gary Gibbons <ggibbons@perforce.com>
Subject: Re: [PATCH 2/2] git p4: add support for 'p4 move' in P4Submit
Date: Mon, 9 Jul 2012 06:56:29 -0400	[thread overview]
Message-ID: <20120709105629.GA23746@padd.com> (raw)
In-Reply-To: <7v7guhpfmn.fsf@alter.siamese.dyndns.org>

gitster@pobox.com wrote on Thu, 05 Jul 2012 23:28 -0700:
> Pete Wyckoff <pw@padd.com> writes:
> 
> > diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
> > index 84fffb3..8be74b6 100755
> > --- a/t/t9814-git-p4-rename.sh
> > +++ b/t/t9814-git-p4-rename.sh
> > @@ -77,16 +77,16 @@ test_expect_success 'detect renames' '
> >  		git commit -a -m "Rename file1 to file4" &&
> >  		git diff-tree -r -M HEAD &&
> >  		git p4 submit &&
> > -		p4 filelog //depot/file4 &&
> > -		p4 filelog //depot/file4 | test_must_fail grep -q "branch from" &&
> > +		p4 filelog //depot/file4 | tee filelog &&
> > +		! grep -q " from //depot" filelog &&
> 
> I am not a huge fan of using "tee" in our test scripts, especially
> as it means piping output of another command whose output (and
> presumably the behaviour) we care about, hiding its exit status.
> 
> Fixing the incorrect use of piping to "test_must_fail grep" is a
> good change, but is there anything wrong to do the above like this?
> 
> 	p4 filelog //depot/file4 >filelog &&
> 	! grep -q " from //depot" filelog &&

I'd started growing fond of "tee" as it shows all the
output, and isolates the grep as a separate step.  Much
easier to see the bad output when a test fails.

I'll switch around to your approach, adding a "cat filelog" line
for interesting cases.

		-- Pete

      reply	other threads:[~2012-07-09 10:56 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-04 13:40 [PATCH 0/2] git p4: use "move" command for renames Pete Wyckoff
2012-07-04 13:40 ` [PATCH 1/2] git p4: refactor diffOpts calculation Pete Wyckoff
2012-07-04 13:40 ` [PATCH 2/2] git p4: add support for 'p4 move' in P4Submit Pete Wyckoff
2012-07-06  6:28   ` Junio C Hamano
2012-07-09 10:56     ` Pete Wyckoff [this message]

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=20120709105629.GA23746@padd.com \
    --to=pw@padd.com \
    --cc=ggibbons@perforce.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.