Re: [PATCH/RFC] branch: add tests for new copy branch feature

On Mon, May 29, 2017 at 12:56 AM, Sahil Dua [off-list ref] wrote:

quoted

New feature - copying a branch along with its config section.

Aim is to have an option -c for copying a branch just like -m option for
renaming a branch.

This commit adds a few basic tests for getting any suggestions/feedback
about expected behavior for this new feature.

Signed-off-by: Sahil Dua <redacted>
---
 t/t3200-branch.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fe62e7c775da..2c95ed6ebf3c 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh

@@ -341,6 +341,59 @@ test_expect_success 'config information was renamed, too' '
        test_must_fail git config branch.s/s/dummy
 '

+test_expect_success 'git branch -c dumps usage' '
+       test_expect_code 128 git branch -c 2>err &&
+       test_i18ngrep "branch name required" err
+'
+
+git config branch.d.dummy Hello
+
+test_expect_success 'git branch -c d e should work' '
+       git branch -l d &&
+       git reflog exists refs/heads/d &&
+       git branch -c d e &&
+       git reflog exists refs/heads/d &&
+       git reflog exists refs/heads/e
+'
+
+test_expect_success 'config information was copied, too' '
+       test $(git config branch.e.dummy) = Hello &&
+       test $(git config branch.d.dummy) = Hello
+'
+
+git config branch.f/f.dummy Hello
+
+test_expect_success 'git branch -c f/f g/g should work' '
+       git branch -l f/f &&
+       git reflog exists refs/heads/f/f &&
+       git branch -c f/f g/g &&
+       git reflog exists refs/heads/f/f &&
+       git reflog exists refs/heads/g/g
+'
+
+test_expect_success 'config information was copied, too' '
+       test $(git config branch.f/f.dummy) = Hello &&
+       test $(git config branch.g/g.dummy) = Hello
+'
+
+test_expect_success 'git branch -c m2 m2 should work' '
+       git branch -l m2 &&
+       git reflog exists refs/heads/m2 &&
+       git branch -c m2 m2 &&
+       git reflog exists refs/heads/m2
+'
+
+test_expect_success 'git branch -c a a/a should fail' '
+       git branch -l a &&
+       git reflog exists refs/heads/a &&
+       test_must_fail git branch -c a a/a
+'
+
+test_expect_success 'git branch -c b/b b should fail' '
+       git branch -l b/b &&
+       test_must_fail git branch -c b/b b
+'
+
 test_expect_success 'deleting a symref' '
        git branch target &&
        git symbolic-ref refs/heads/symref refs/heads/target &&

This looks good to me. Comments, in no particular order:

* There should be a test for `git branch -c <newbranch>`, i.e. that
should implicitly copy from HEAD just like `git branch -m <newbranch>`
does. However, when looking at this I can see there's actually no test
for one-argument `git branch -m`, i.e. if you apply this:

--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -699,8 +699,6 @@ int cmd_branch(int argc, const char **argv, const

char *prefix)
        } else if (rename) {
                if (!argc)
                        die(_("branch name required"));
-               else if (argc == 1)
-                       rename_branch(head, argv[0], rename > 1);
                else if (argc == 2)
                        rename_branch(argv[0], argv[1], rename > 1);
                else

The only test that fails is a `git branch -M master`, i.e.
one-argument -M is tested for, but not -m, same codepath though, but
while you're at it a patch/series like this could start by adding a
one-arg -m test, then follow-up by copying that for the new `-c`.