Subject: Re: [PATCH 2/2] git p4: add support for 'p4 move' in P4Submit

From: Pete Wyckoff <[email-masked]>
Date: Mon, 9 Jul 2012 06:56:29 -0400

[email-masked] wrote on Thu, 05 Jul 2012 23:28 -0700:
> Pete Wyckoff <[email-masked]> 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