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

[Thomas Gummerer]

On 11/24, Junio C Hamano wrote:
> Thomas Gummerer  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

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

[Junio C Hamano]

Thomas Gummerer  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?

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

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