* [v3 PATCH 2/2] reset: add tests for git reset -
@ 2015-03-18 8:35 Sundararajan R
2015-03-18 16:54 ` Matthieu Moy
2015-03-19 22:16 ` Kevin D
0 siblings, 2 replies; 4+ messages in thread
From: Sundararajan R @ 2015-03-18 8:35 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Matthieu Moy, 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [v3 PATCH 2/2] reset: add tests for git reset -
2015-03-18 8:35 [v3 PATCH 2/2] reset: add tests for git reset - Sundararajan R
@ 2015-03-18 16:54 ` Matthieu Moy
2015-03-19 22:16 ` Kevin D
1 sibling, 0 replies; 4+ messages in thread
From: Matthieu Moy @ 2015-03-18 16:54 UTC (permalink / raw)
To: Sundararajan R; +Cc: git, Junio C Hamano, Eric Sunshine
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/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [v3 PATCH 2/2] reset: add tests for git reset -
2015-03-18 8:35 [v3 PATCH 2/2] reset: add tests for git reset - Sundararajan R
2015-03-18 16:54 ` Matthieu Moy
@ 2015-03-19 22:16 ` Kevin D
[not found] ` <CAPV5_7+Kfuiy01cmwfjJCGJPo7mO_bzBc7DjPr9JyuV=XSXSrA@mail.gmail.com>
1 sibling, 1 reply; 4+ messages in thread
From: Kevin D @ 2015-03-19 22:16 UTC (permalink / raw)
To: Sundararajan R; +Cc: git, Junio C Hamano, Eric Sunshine, Matthieu Moy
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [v3 PATCH 2/2] reset: add tests for git reset -
[not found] ` <CAPV5_7+Kfuiy01cmwfjJCGJPo7mO_bzBc7DjPr9JyuV=XSXSrA@mail.gmail.com>
@ 2015-03-20 20:32 ` Kevin D
0 siblings, 0 replies; 4+ messages in thread
From: Kevin D @ 2015-03-20 20:32 UTC (permalink / raw)
To: Sundararajan R; +Cc: git, Junio C Hamano, Eric Sunshine, Matthieu Moy
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-03-20 20:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-18 8:35 [v3 PATCH 2/2] reset: add tests for git reset - Sundararajan R
2015-03-18 16:54 ` Matthieu Moy
2015-03-19 22:16 ` Kevin D
[not found] ` <CAPV5_7+Kfuiy01cmwfjJCGJPo7mO_bzBc7DjPr9JyuV=XSXSrA@mail.gmail.com>
2015-03-20 20:32 ` Kevin D
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).