Re: [PATCH 06/21] git p4 test: use client_view in t9806

From: Pete Wyckoff <pw@padd.com>
Date: Jan 27, 2013, 01:51 UTC
Message-ID: <20130127015135.GA29157@padd.com>
In-Reply-To: <7v4nmiklbt.fsf@alter.siamese.dyndns.org>

Yes, this really is four months later. Somehow I forgot all about this series.

gitster@pobox.com wrote on Fri, 28 Sep 2012 12:11 -0700:

Show 61 quoted lines

> Pete Wyckoff <pw@padd.com> writes:
> 
> > Use the standard client_view function from lib-git-p4.sh
> > instead of building one by hand.  This requires a bit of
> > rework, using the current value of $P4CLIENT for the client
> > name.  It also reorganizes the test to isolate changes to
> > $P4CLIENT and $cli in a subshell.
> >
> > Signed-off-by: Pete Wyckoff <pw@padd.com>
> > ---
> >  t/lib-git-p4.sh           |  4 ++--
> >  t/t9806-git-p4-options.sh | 50 ++++++++++++++++++++++-------------------------
> >  2 files changed, 25 insertions(+), 29 deletions(-)
> >
> > diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
> > index 890ee60..d558dd0 100644
> > --- a/t/lib-git-p4.sh
> > +++ b/t/lib-git-p4.sh
> > @@ -116,8 +116,8 @@ marshal_dump() {
> >  client_view() {
> >  	(
> >  		cat <<-EOF &&
> > -		Client: client
> > -		Description: client
> > +		Client: $P4CLIENT
> > +		Description: $P4CLIENT
> >  		Root: $cli
> >  		View:
> >  		EOF
> > diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
> > index fa40cc8..37ca30a 100755
> > --- a/t/t9806-git-p4-options.sh
> > +++ b/t/t9806-git-p4-options.sh
> > @@ -126,37 +126,33 @@ test_expect_success 'clone --use-client-spec' '
> >  		exec >/dev/null &&
> >  		test_must_fail git p4 clone --dest="$git" --use-client-spec
> >  	) &&
> > -	cli2=$(test-path-utils real_path "$TRASH_DIRECTORY/cli2") &&
> > +	# build a different client
> > +	cli2="$TRASH_DIRECTORY/cli2" &&
> >  	mkdir -p "$cli2" &&
> >  	test_when_finished "rmdir \"$cli2\"" &&
> >  	test_when_finished cleanup_git &&
> > ...
> > -	# same thing again, this time with variable instead of option
> >  	(
> > ...
> > +		# group P4CLIENT and cli changes in a sub-shell
> > +		P4CLIENT=client2 &&
> > +		cli="$cli2" &&
> > +		client_view "//depot/sub/... //client2/bus/..." &&
> > +		git p4 clone --dest="$git" --use-client-spec //depot/... &&
> > +		(
> > +			cd "$git" &&
> > +			test_path_is_file bus/dir/f4 &&
> > +			test_path_is_missing file1
> > +		) &&
> > +		cleanup_git &&
> 
> Hmm, the use of "test-path-utils real_path" to form cli2 in the
> original was not necessary at all?

Thanks, I will make this removal more explicit, putting it in with 8/21 where it belongs, with explanation.

Show 16 quoted lines

> > +		# same thing again, this time with variable instead of option
> > +		(
> > +			cd "$git" &&
> > +			git init &&
> > +			git config git-p4.useClientSpec true &&
> > +			git p4 sync //depot/... &&
> > +			git checkout -b master p4/master &&
> > +			test_path_is_file bus/dir/f4 &&
> > +			test_path_is_missing file1
> > +		)
> 
> Do you need a separate sub-shell inside a sub-shell we are already
> in that you called client_view in?
> 
> >  	)
> >  '

The first subshell is to hide P4CLIENT and cli variable changes from the rest of the tests.

The second is to keep the "cd $git" from changing behavior of the following "cleanup_git" call. That does "rm -rf $git" which would fail on some file systems if cwd is still in there. With just one subshell it would look like:

	(
		P4CLIENT=client2 &&
		git p4 clone .. &&
		cd "$git" &&
		... do test
		cd "$TRASH_DIRECTORY" &&
		cleanup_git &&

		cd "$git" &&
		... more test
	)

It's a bit easier to understand with an extra level of shell, and sticks to the pattern used in the rest of the t98*.

		-- Pete