# [v3 PATCH 2/2] reset: add tests for git reset - 4 messages from 2015-03-18 to 2015-03-20. Participants: Sundararajan R, Matthieu Moy, Kevin D. Thread: https://gitlist.dev/t/38842 ## Sundararajan R, 2015-03-18 08:35 Subject: [v3 PATCH 2/2] reset: add tests for git reset - Message-ID: <1426667709-26698-1-git-send-email-dyoucme@gmail.com> URL: https://gitlist.dev/e/1426667709-26698-1-git-send-email-dyoucme%40gmail.com ``` 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 Helped-by: Eric Sunshine Helped-by: Matthieu Moy Signed-off-by: Sundararajan R --- 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 ``` ## Matthieu Moy, 2015-03-18 16:54 Subject: Re: [v3 PATCH 2/2] reset: add tests for git reset - Message-ID: URL: https://gitlist.dev/e/vpqh9ticklj.fsf%40anie.imag.fr In-Reply-To: <1426667709-26698-1-git-send-email-dyoucme@gmail.com> ``` Sundararajan R 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 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/ ``` ## Kevin D, 2015-03-19 22:16 Subject: Re: [v3 PATCH 2/2] reset: add tests for git reset - Message-ID: <20150319221652.GE11832@vps892.directvps.nl> URL: https://gitlist.dev/e/20150319221652.GE11832%40vps892.directvps.nl In-Reply-To: <1426667709-26698-1-git-send-email-dyoucme@gmail.com> ``` 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. ``` ## Kevin D, 2015-03-20 20:32 Subject: Re: [v3 PATCH 2/2] reset: add tests for git reset - Message-ID: <20150320203236.GF11832@vps892.directvps.nl> URL: https://gitlist.dev/e/20150320203236.GF11832%40vps892.directvps.nl In-Reply-To: ``` 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. ```