[v3 PATCH 2/2] reset: add tests for git reset -

[v3 PATCH 2/2] reset: add tests for git reset -

[Sundararajan R]

The failure case which occurs on teaching git is taught the '-' shorthand
is when there exists no branch pointed to by '@{-1}'. But, if there is a file
named - in the working tree, the user can be unambiguously assumed to be 
referring to it while issuing this command.

The ambiguous case occurs when the @{-1} branch exists and file named '-' also
exists in the working tree. This are also treated as a failure case but here 
the user is given advice as to how he can proceed.

Another potentially tricky case is when the file '@{-1}' exists. In this case,
the command should succeed as the user doesn't mention the file '@{-1}' and can
be safely assumed to be referring to the @{-1} branch.

Add tests to check the handling of these cases.
Also add a test to verify that reset - behaves like reset @{-1} when none
of the above cases are true.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Signed-off-by: Sundararajan R <dyoucme@gmail.com>
---
Thank you for your feedback Torsten and Eric. I have made modifications suggested 
by you. I have also acted on Matthieu's suggestions on the archive.
Please let me know if there is something else I should add.
I have also removed one irrelevant test from  which we come to know of nothing new.

 t/t7102-reset.sh | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..f5a8e76 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,79 @@ test_expect_success 'reset --mixed sets up work tree' '
 	test_cmp expect actual
 '
 
+test_expect_success 'reset - with no @{-1} branch should fail' '
+	test_when_finished rm -rf new &&
+	git init new &&
+	(
+		cd new &&
+		test_must_fail git reset - 2>../actual
+	) && 
+	test_i18ngrep "bad flag" actual 
+'
+
+test_expect_success 'reset - with no @{-1} branch and file named - should succeed' '
+	test_when_finished rm -rf new &&
+	>expected &&
+	git init new &&
+	(
+		cd new &&
+		echo "Hello" >- &&
+		git add - &&
+		git reset - >../actual 
+	) &&
+	test_cmp expected actual
+'
+
+test_expect_success 'reset - with @{-1} branch and file named - should fail' '
+	test_when_finished rm -rf new &&
+	git init new &&
+	(
+		cd new && 
+		echo "Hello" >- &&
+		git add - &&
+		git commit -m "first_commit" &&
+		git checkout -b new_branch &&
+		>- &&
+		git add - &&
+		test_must_fail git reset - 2>../actual 
+	) &&
+	test_i18ngrep "ambiguous argument" actual 
+'
+
+test_expect_success 'reset - with @{-1} branch and file named @{-1} should succeed' '
+	test_when_finished rm -rf new &&
+	git init new &&
+	(
+		cd new && 
+		echo "Hello" >@{-1} &&
+		git add @{-1} &&
+		git commit -m "first_commit" &&
+		git checkout -b new_branch &&
+		>@{-1} &&
+		git add @{-1} &&
+		git reset - >../actual 
+	) &&
+	test_i18ngrep "Unstaged" actual 
+'
+
+test_expect_success 'reset - with @{-1} branch and no file named - should succeed' '
+	test_when_finished rm -rf new &&
+	git init new &&
+	(
+		cd new &&
+		echo "Hey" >new_file &&
+		git add new_file &&
+		git commit -m "first_commit" &&
+		git checkout -b new_branch &&
+		>new_file &&
+		git add new_file &&
+		git reset - &&
+		git status -uno >actual &&
+		git add new_file &&
+		git reset @{-1} &&
+		git status -uno >expected &&
+		test_cmp actual expected 
+	)
+'
+
 test_done
-- 
2.1.0

Re: [v3 PATCH 2/2] reset: add tests for git reset -

[Matthieu Moy]

Sundararajan R <dyoucme@gmail.com> writes:

> Subject: [v3 PATCH 2/2] reset: add tests for git reset -

This should be [PATCH v3 2/2].

"git send-email -v2" can do this for you.

Sundararajan R <dyoucme@gmail.com> writes:

> +test_expect_success 'reset - with no @{-1} branch and file named - should succeed' '
> +	test_when_finished rm -rf new &&
> +	>expected &&
> +	git init new &&
> +	(
> +		cd new &&
> +		echo "Hello" >- &&
> +		git add - &&
> +		git reset - >../actual 
> +	) &&
> +	test_cmp expected actual
> +'

test_must_be_empty actual would be easier to read than ">expected ...
test_cmp expected" IMHO.

> +test_expect_success 'reset - with @{-1} branch and no file named - should succeed' '
> +	test_when_finished rm -rf new &&
> +	git init new &&
> +	(
> +		cd new &&
> +		echo "Hey" >new_file &&
> +		git add new_file &&
> +		git commit -m "first_commit" &&
> +		git checkout -b new_branch &&
> +		>new_file &&
> +		git add new_file &&
> +		git reset - &&
> +		git status -uno >actual &&
> +		git add new_file &&
> +		git reset @{-1} &&
> +		git status -uno >expected &&
> +		test_cmp actual expected 
> +	)
> +'

Better use "git status --porcelain" here as its format is meant to be
stable and unambiguous. The non-porcelain should work two because you're
comparing the output on two identical states, but who knows.

With or without my suggested change, the series looks good to me.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [v3 PATCH 2/2] reset: add tests for git reset -

[Kevin D]

On Wed, Mar 18, 2015 at 02:05:09PM +0530, Sundararajan R wrote:
> The failure case which occurs on teaching git is taught the '-' shorthand
> is when there exists no branch pointed to by '@{-1}'. But, if there is a file
> named - in the working tree, the user can be unambiguously assumed to be 
> referring to it while issuing this command.
> 

The first line is hard to read. I think the "is taught" part is
redundant.

Re: [v3 PATCH 2/2] reset: add tests for git reset -

[Kevin D]

On Fri, Mar 20, 2015 at 04:02:38AM +0530, Sundararajan R wrote:
> Yes. I made a mistake while framing the sentence. I should have proof read
> the commit message more thoroughly.  Should I submit a new patch with the
> corrected commit message?

Yeah, you can combine that with the comments from Matthieu.