# [PATCH 0/3] branch and t3200 cleanup

6 messages from 2020-06-15 to 2020-06-17. Participants: Denton Liu, Junio C Hamano.
Thread: https://gitlist.dev/t/53680

## Denton Liu, 2020-06-15 11:53

Subject: [PATCH 0/3] branch and t3200 cleanup
Message-ID: <cover.1592221875.git.liu.denton@gmail.com>
URL: https://gitlist.dev/e/cover.1592221875.git.liu.denton%40gmail.com

```
I was looking through some old patches that I had sitting around. In one
set of these, I implemented `git branch --set-remote-to` but I
ultimately decided that the idea was stupid. However, these patches are
salvageable so I'll send them in.

Denton Liu (3):
  t3200: rename "expected" to "expect"
  t3200: test for specific errors
  branch: don't mix --edit-description

 builtin/branch.c  |  2 +-
 t/t3200-branch.sh | 67 ++++++++++++++++++++++++++++++-----------------
 2 files changed, 44 insertions(+), 25 deletions(-)

-- 
2.27.0.132.g321788e831


```

## Denton Liu, 2020-06-15 11:53

Subject: [PATCH 1/3] t3200: rename "expected" to "expect"
Message-ID: <5fa405d7e1672ef18442cdcd514698c92e139a8e.1592221875.git.liu.denton@gmail.com>
URL: https://gitlist.dev/e/5fa405d7e1672ef18442cdcd514698c92e139a8e.1592221875.git.liu.denton%40gmail.com
In-Reply-To: <cover.1592221875.git.liu.denton@gmail.com>

```
Clean up style of test by changing some filenames from "expected" to
"expect", which follows typical test convention.

Also, change a space-indent into a tab-indent.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3200-branch.sh | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 411a70b0ce..da3659c3d3 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -323,11 +323,11 @@ test_expect_success 'git branch --list -v with --abbrev' '
 
 test_expect_success 'git branch --column' '
 	COLUMNS=81 git branch --column=column >actual &&
-	cat >expected <<\EOF &&
+	cat >expect <<\EOF &&
   a/b/c     bam       foo       l       * master    mb        o/o       q
   abc       bar       j/k       m/m       master2   n         o/p       r
 EOF
-	test_cmp expected actual
+	test_cmp expect actual
 '
 
 test_expect_success 'git branch --column with an extremely long branch name' '
@@ -336,7 +336,7 @@ test_expect_success 'git branch --column with an extremely long branch name' '
 	test_when_finished "git branch -d $long" &&
 	git branch $long &&
 	COLUMNS=80 git branch --column=column >actual &&
-	cat >expected <<EOF &&
+	cat >expect <<EOF &&
   a/b/c
   abc
   bam
@@ -355,7 +355,7 @@ test_expect_success 'git branch --column with an extremely long branch name' '
   r
   $long
 EOF
-	test_cmp expected actual
+	test_cmp expect actual
 '
 
 test_expect_success 'git branch with column.*' '
@@ -364,11 +364,11 @@ test_expect_success 'git branch with column.*' '
 	COLUMNS=80 git branch >actual &&
 	git config --unset column.branch &&
 	git config --unset column.ui &&
-	cat >expected <<\EOF &&
+	cat >expect <<\EOF &&
   a/b/c   bam   foo   l   * master    mb   o/o   q
   abc     bar   j/k   m/m   master2   n    o/p   r
 EOF
-	test_cmp expected actual
+	test_cmp expect actual
 '
 
 test_expect_success 'git branch --column -v should fail' '
@@ -379,7 +379,7 @@ test_expect_success 'git branch -v with column.ui ignored' '
 	git config column.ui column &&
 	COLUMNS=80 git branch -v | cut -c -10 | sed "s/ *$//" >actual &&
 	git config --unset column.ui &&
-	cat >expected <<\EOF &&
+	cat >expect <<\EOF &&
   a/b/c
   abc
   bam
@@ -397,7 +397,7 @@ test_expect_success 'git branch -v with column.ui ignored' '
   q
   r
 EOF
-	test_cmp expected actual
+	test_cmp expect actual
 '
 
 mv .git/config .git/config-saved
@@ -922,17 +922,17 @@ test_expect_success 'test --unset-upstream on a particular branch' '
 '
 
 test_expect_success 'disabled option --set-upstream fails' '
-    test_must_fail git branch --set-upstream origin/master
+	test_must_fail git branch --set-upstream origin/master
 '
 
 test_expect_success '--set-upstream-to notices an error to set branch as own upstream' '
 	git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
-	cat >expected <<-\EOF &&
+	cat >expect <<-\EOF &&
 	warning: Not setting branch my13 as its own upstream.
 	EOF
 	test_expect_code 1 git config branch.my13.remote &&
 	test_expect_code 1 git config branch.my13.merge &&
-	test_i18ncmp expected actual
+	test_i18ncmp expect actual
 '
 
 # Keep this test last, as it changes the current branch
-- 
2.27.0.132.g321788e831


```

## Denton Liu, 2020-06-15 11:53

Subject: [PATCH 2/3] t3200: test for specific errors
Message-ID: <e787b581c3a86b9de51dc21dc90bd1685309313e.1592221875.git.liu.denton@gmail.com>
URL: https://gitlist.dev/e/e787b581c3a86b9de51dc21dc90bd1685309313e.1592221875.git.liu.denton%40gmail.com
In-Reply-To: <cover.1592221875.git.liu.denton@gmail.com>

```
In the "--set-upstream-to" and "--unset-upstream" tests, specific error
conditions are being tested. However, there is no way of ensuring that a
test case is failing because of some specific error.

Check stderr of failing commands to ensure that they are failing in the
expected way.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3200-branch.sh | 45 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index da3659c3d3..1fd03cae80 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -835,32 +835,42 @@ test_expect_success 'branch from tag w/--track causes failure' '
 '
 
 test_expect_success '--set-upstream-to fails on multiple branches' '
-	test_must_fail git branch --set-upstream-to master a b c
+	echo "fatal: too many arguments to set new upstream" >expect &&
+	test_must_fail git branch --set-upstream-to master a b c 2>err &&
+	test_i18ncmp expect err
 '
 
 test_expect_success '--set-upstream-to fails on detached HEAD' '
 	git checkout HEAD^{} &&
-	test_must_fail git branch --set-upstream-to master &&
-	git checkout -
+	test_when_finished git checkout - &&
+	echo "fatal: could not set upstream of HEAD to master when it does not point to any branch." >expect &&
+	test_must_fail git branch --set-upstream-to master 2>err &&
+	test_i18ncmp expect err
 '
 
 test_expect_success '--set-upstream-to fails on a missing dst branch' '
-	test_must_fail git branch --set-upstream-to master does-not-exist
+	echo "fatal: branch '"'"'does-not-exist'"'"' does not exist" >expect &&
+	test_must_fail git branch --set-upstream-to master does-not-exist 2>err &&
+	test_i18ncmp expect err
 '
 
 test_expect_success '--set-upstream-to fails on a missing src branch' '
-	test_must_fail git branch --set-upstream-to does-not-exist master
+	test_must_fail git branch --set-upstream-to does-not-exist master 2>err &&
+	test_i18ngrep "the requested upstream branch '"'"'does-not-exist'"'"' does not exist" err
 '
 
 test_expect_success '--set-upstream-to fails on a non-ref' '
-	test_must_fail git branch --set-upstream-to HEAD^{}
+	echo "fatal: Cannot setup tracking information; starting point '"'"'HEAD^{}'"'"' is not a branch." >expect &&
+	test_must_fail git branch --set-upstream-to HEAD^{} 2>err &&
+	test_i18ncmp expect err
 '
 
 test_expect_success '--set-upstream-to fails on locked config' '
 	test_when_finished "rm -f .git/config.lock" &&
 	>.git/config.lock &&
 	git branch locked &&
-	test_must_fail git branch --set-upstream-to locked
+	test_must_fail git branch --set-upstream-to locked 2>err &&
+	test_i18ngrep "could not lock config file .git/config: File exists" err
 '
 
 test_expect_success 'use --set-upstream-to modify HEAD' '
@@ -881,14 +891,17 @@ test_expect_success 'use --set-upstream-to modify a particular branch' '
 '
 
 test_expect_success '--unset-upstream should fail if given a non-existent branch' '
-	test_must_fail git branch --unset-upstream i-dont-exist
+	echo "fatal: Branch '"'"'i-dont-exist'"'"' has no upstream information" >expect &&
+	test_must_fail git branch --unset-upstream i-dont-exist 2>err &&
+	test_i18ncmp expect err
 '
 
 test_expect_success '--unset-upstream should fail if config is locked' '
 	test_when_finished "rm -f .git/config.lock" &&
 	git branch --set-upstream-to locked &&
 	>.git/config.lock &&
-	test_must_fail git branch --unset-upstream
+	test_must_fail git branch --unset-upstream 2>err &&
+	test_i18ngrep "could not lock config file .git/config: File exists" err
 '
 
 test_expect_success 'test --unset-upstream on HEAD' '
@@ -900,17 +913,23 @@ test_expect_success 'test --unset-upstream on HEAD' '
 	test_must_fail git config branch.master.remote &&
 	test_must_fail git config branch.master.merge &&
 	# fail for a branch without upstream set
-	test_must_fail git branch --unset-upstream
+	echo "fatal: Branch '"'"'master'"'"' has no upstream information" >expect &&
+	test_must_fail git branch --unset-upstream 2>err &&
+	test_i18ncmp expect err
 '
 
 test_expect_success '--unset-upstream should fail on multiple branches' '
-	test_must_fail git branch --unset-upstream a b c
+	echo "fatal: too many arguments to unset upstream" >expect &&
+	test_must_fail git branch --unset-upstream a b c 2>err &&
+	test_i18ncmp expect err
 '
 
 test_expect_success '--unset-upstream should fail on detached HEAD' '
 	git checkout HEAD^{} &&
-	test_must_fail git branch --unset-upstream &&
-	git checkout -
+	test_when_finished git checkout - &&
+	echo "fatal: could not unset upstream of HEAD when it does not point to any branch." >expect &&
+	test_must_fail git branch --unset-upstream 2>err &&
+	test_i18ncmp expect err
 '
 
 test_expect_success 'test --unset-upstream on a particular branch' '
-- 
2.27.0.132.g321788e831


```

## Denton Liu, 2020-06-15 11:53

Subject: [PATCH 3/3] branch: don't mix --edit-description
Message-ID: <1d0eebd1ecf24db0241801af31d904a19ad391a8.1592221875.git.liu.denton@gmail.com>
URL: https://gitlist.dev/e/1d0eebd1ecf24db0241801af31d904a19ad391a8.1592221875.git.liu.denton%40gmail.com
In-Reply-To: <cover.1592221875.git.liu.denton@gmail.com>

```
`git branch` accepts `--edit-description` in conjunction with other
arguments. However, `--edit-description` is its own mode, similar to
`--set-upstream-to`, which is also made mutually exclusive with other
modes. Prevent `--edit-description` from being mixed with other modes.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 builtin/branch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index accb61b1aa..99633ad004 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -693,7 +693,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		list = 1;
 
 	if (!!delete + !!rename + !!copy + !!new_upstream + !!show_current +
-	    list + unset_upstream > 1)
+	    list + edit_description + unset_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
 	if (filter.abbrev == -1)
-- 
2.27.0.132.g321788e831


```

## Junio C Hamano, 2020-06-17 18:09

Subject: Re: [PATCH 3/3] branch: don't mix --edit-description
Message-ID: <xmqqv9jpva1j.fsf@gitster.c.googlers.com>
URL: https://gitlist.dev/e/xmqqv9jpva1j.fsf%40gitster.c.googlers.com
In-Reply-To: <1d0eebd1ecf24db0241801af31d904a19ad391a8.1592221875.git.liu.denton@gmail.com>

```
Denton Liu <liu.denton@gmail.com> writes:

> `git branch` accepts `--edit-description` in conjunction with other
> arguments. However, `--edit-description` is its own mode, similar to
> `--set-upstream-to`, which is also made mutually exclusive with other
> modes. Prevent `--edit-description` from being mixed with other modes.

Makes sense.  Do we have some "these options are mutually exclusive"
test?

It makes me wonder if this can/should be switched to OPT_CMDMODE()
so that we do not have to write this if statement in the first
place.


> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  builtin/branch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index accb61b1aa..99633ad004 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -693,7 +693,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>  		list = 1;
>  
>  	if (!!delete + !!rename + !!copy + !!new_upstream + !!show_current +
> -	    list + unset_upstream > 1)
> +	    list + edit_description + unset_upstream > 1)
>  		usage_with_options(builtin_branch_usage, options);
>  
>  	if (filter.abbrev == -1)

```

## Junio C Hamano, 2020-06-17 18:12

Subject: Re: [PATCH 0/3] branch and t3200 cleanup
Message-ID: <xmqqr1udv9wh.fsf@gitster.c.googlers.com>
URL: https://gitlist.dev/e/xmqqr1udv9wh.fsf%40gitster.c.googlers.com
In-Reply-To: <cover.1592221875.git.liu.denton@gmail.com>

```
Denton Liu <liu.denton@gmail.com> writes:

> I was looking through some old patches that I had sitting around. In one
> set of these, I implemented `git branch --set-remote-to` but I
> ultimately decided that the idea was stupid. However, these patches are
> salvageable so I'll send them in.
>
> Denton Liu (3):
>   t3200: rename "expected" to "expect"
>   t3200: test for specific errors
>   branch: don't mix --edit-description
>
>  builtin/branch.c  |  2 +-
>  t/t3200-branch.sh | 67 ++++++++++++++++++++++++++++++-----------------
>  2 files changed, 44 insertions(+), 25 deletions(-)

Thnaks, will queue.

```