Re: [PATCH] t3200: add test for single parameter passed to -m option

2 messages, 2 authors, 2017-06-06 · open the first message on its own page

Re: [PATCH] t3200: add test for single parameter passed to -m option

From: Junio C Hamano <hidden>
Date: 2017-06-06 01:27:12

Sahil Dua [off-list ref] writes:
quoted hunk
Adds a test for the case when only one parameter is passed to '-m'
(move/rename) option.

For example - if 'git branch -m bbb' is run, it should rename the
currently checked out branch to bbb. There was no test for this
particular case with only one parameter for -m option. However,
there's one similar test case for -M option.

Signed-off-by: Sahil Dua <redacted>
---
 t/t3200-branch.sh | 8 ++++++++
 1 file changed, 8 insertions(+)
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fe62e7c775da6..7504f14bc52f8 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -100,6 +100,14 @@ test_expect_success 'git branch -m n/n n should work' '
 	git reflog exists refs/heads/n
 '
 
+test_expect_success 'git branch -m bbb should rename checked out branch' '
+	test_when_finished git branch -d bbb &&
+	test_when_finished git checkout master &&
+	git checkout -b aaa &&
+	git branch -m bbb &&
+	git reflog exists refs/heads/bbb
+'
Looks almost good.  Don't we want to also make sure that HEAD points
at bbb branch, e.g.

	git symbolic-ref HEAD >actual &&
	echo refs/heads/bbb >expect &&
	test_cmp expect actual &&

after the "do we have reflog for the bbb branch?" test?

Also, I suspect that we care about the reflog that is moved to 'bbb'
retains entries created for 'aaa'.  So perhaps "reflog exists" needs
to be replaced by something like

	git checkout -b aaa &&
	git commit --allow-empty -m "a new commit" &&
	git rev-parse aaa@{1} >expect &&
	git branch -m bbb &&
	# the topmost entry is about branch creation
	git rev-parse bbb@{2} >actual

no?
 test_expect_success 'git branch -m o/o o should fail when o/p exists' '
 	git branch o/o &&
 	git branch o/p &&

--
https://github.com/git/git/pull/371

Re: [PATCH] t3200: add test for single parameter passed to -m option

From: Sahil Dua <hidden>
Date: 2017-06-06 08:51:34

On Tue, Jun 6, 2017 at 3:27 AM, Junio C Hamano [off-list ref] wrote:
Sahil Dua [off-list ref] writes:
quoted
Adds a test for the case when only one parameter is passed to '-m'
(move/rename) option.

For example - if 'git branch -m bbb' is run, it should rename the
currently checked out branch to bbb. There was no test for this
particular case with only one parameter for -m option. However,
there's one similar test case for -M option.

Signed-off-by: Sahil Dua <redacted>
---
 t/t3200-branch.sh | 8 ++++++++
 1 file changed, 8 insertions(+)
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fe62e7c775da6..7504f14bc52f8 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -100,6 +100,14 @@ test_expect_success 'git branch -m n/n n should work' '
      git reflog exists refs/heads/n
 '

+test_expect_success 'git branch -m bbb should rename checked out branch' '
+     test_when_finished git branch -d bbb &&
+     test_when_finished git checkout master &&
+     git checkout -b aaa &&
+     git branch -m bbb &&
+     git reflog exists refs/heads/bbb
+'
Looks almost good.  Don't we want to also make sure that HEAD points
at bbb branch, e.g.

        git symbolic-ref HEAD >actual &&
        echo refs/heads/bbb >expect &&
        test_cmp expect actual &&

after the "do we have reflog for the bbb branch?" test?

Also, I suspect that we care about the reflog that is moved to 'bbb'
retains entries created for 'aaa'.  So perhaps "reflog exists" needs
to be replaced by something like

        git checkout -b aaa &&
        git commit --allow-empty -m "a new commit" &&
        git rev-parse aaa@{1} >expect &&
        git branch -m bbb &&
        # the topmost entry is about branch creation
        git rev-parse bbb@{2} >actual

no?
Both of these changes make complete sense and in my opinion will
increase the quality of the tests if applied to the other tests too. I
notice that no other test for this (-m|-M) option checks if the branch
is actually checked out. Similarly, no other test checks if the
operation retains reflog entries. They just check if the new reflog
exists.

Do you think I should make similar changes to the other tests for
(-m|-M) option as well?
quoted
 test_expect_success 'git branch -m o/o o should fail when o/p exists' '
      git branch o/o &&
      git branch o/p &&

--
https://github.com/git/git/pull/371
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help