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

2015-03-20 Thread 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-19 Thread 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-18 Thread 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 gits...@pobox.com
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Sundararajan R dyou...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-18 Thread Matthieu Moy
Sundararajan R dyou...@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 dyou...@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/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html