Re: [PATCH v4 2/4] worktree: add --[no-]track option to the add subcommand

[Thomas Gummerer <t.gummerer@gmail.com>]

On 11/24, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> > index b5c47ac602..53042ce565 100755
> > --- a/t/t2025-worktree-add.sh
> > +++ b/t/t2025-worktree-add.sh
> > @@ -313,5 +313,60 @@ test_expect_success 'checkout a branch under bisect' '
> >  test_expect_success 'rename a branch under bisect not allowed' '
> >  	test_must_fail git branch -M under-bisect bisect-with-new-name
> >  '
> > +# Is branch "refs/heads/$1" set to pull from "$2/$3"?
> > +test_branch_upstream () {
> > +	printf "%s\n" "$2" "refs/heads/$3" >expect.upstream &&
> > +	{
> > +		git config "branch.$1.remote" &&
> > +		git config "branch.$1.merge"
> > +	} >actual.upstream &&
> > +	test_cmp expect.upstream actual.upstream
> > +}
> 
> OK.
> 
> > +test_expect_success '--track sets up tracking' '
> > +	test_when_finished rm -rf track &&
> > +	git worktree add --track -b track track master &&
> > +	git config "branch.track.merge" &&
> > +	(
> > +		test_branch_upstream track . master
> > +	)
> > +'
> 
> Is this "git config" necessary, or is it a remnant of a debugging
> session?  It is tested in the helper that branch.track.merge is set
> to something, and otherwise the helper would fail the same way as
> this standalnoe "git config" would, no?

It's a remnant of a debugging session, sorry.  It would indeed fail in
the same way, so just leaving the 'test_branch_upstream' is enough.
Also looking at that, there's no need for it to be in a subshell, will
fix that as well.


> > +# setup remote repository $1 and repository $2 with $1 set up as
> > +# remote.  The remote has two branches, master and foo.
> > +setup_remote_repo () {
> > +	git init $1 &&
> > +	(
> > +		cd $1 &&
> > +		test_commit $1_master &&
> > +		git checkout -b foo &&
> > +		test_commit upstream_foo
> > +	) &&
> > +	git init $2 &&
> > +	(
> > +		cd $2 &&
> > +		test_commit $2_master &&
> > +		git remote add $1 ../$1 &&
> > +		git config remote.$1.fetch \
> > +			"refs/heads/*:refs/remotes/$1/*" &&
> > +		git fetch --all
> > +	)
> > +}
> > +
> > +test_expect_success '--no-track avoids setting up tracking' '
> > +	test_when_finished rm -rf repo_upstream repo_local foo &&
> > +	setup_remote_repo repo_upstream repo_local &&
> > +	(
> > +		cd repo_local &&
> > +		git worktree add --no-track -b foo ../foo repo_upstream/foo
> > +	) &&
> > +	(
> > +		cd foo &&
> > +		! test_branch_upstream foo repo_upstream foo &&
> 
> It is true that this test helper must yield failure.  But what you
> expect probably is more than that, no?  For example, the test helper
> would fail even if branch.foo.remote is set to the upstream as long
> as branch.foo.merge is not set to point at their foo, but what you
> really want to make sure is that neither configuration variable is
> set.

Yeah you're right, this test is a bit too loose.  Will fix that in the
re-roll.  Thanks!

> > +		git rev-parse repo_upstream/foo >expect &&
> > +		git rev-parse foo >actual &&
> > +		test_cmp expect actual
> > +	)
> > +'
> >  
> >  test_done