# [PATCH] stash: fix false positive in the invalid ref test. 4 messages from 2011-04-05 to 2011-04-06. Participants: Jon Seymour, Jeff King. Thread: https://gitlist.dev/t/27003 ## Jon Seymour, 2011-04-05 23:21 Subject: [PATCH] stash: fix false positive in the invalid ref test. Message-ID: <1302045673-59982-1-git-send-email-jon.seymour@gmail.com> URL: https://gitlist.dev/e/1302045673-59982-1-git-send-email-jon.seymour%40gmail.com ``` Jeff King reported a problem with git stash apply incorrectly applying an invalid stash reference. There is an existing test that should have caught this, but the test itself was broken, resulting in a false positive. Signed-off-by: Jon Seymour --- t/t3903-stash.sh | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 11077f0..5263de7 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -543,11 +543,11 @@ test_expect_success 'invalid ref of the form stash@{n}, n >= N' ' echo bar6 > file2 && git add file2 && git stash && - test_must_fail git drop stash@{1} && - test_must_fail git pop stash@{1} && - test_must_fail git apply stash@{1} && - test_must_fail git show stash@{1} && - test_must_fail git branch tmp stash@{1} && + test_must_fail git stash drop stash@{1} && + test_must_fail git stash pop stash@{1} && + test_must_fail git stash apply stash@{1} && + test_must_fail git stash show stash@{1} && + test_must_fail git stash branch tmp stash@{1} && git stash drop ' -- 1.7.5.rc0.132.g4c19c ``` ## Jeff King, 2011-04-06 18:27 Subject: Re: [PATCH] stash: fix false positive in the invalid ref test. Message-ID: <20110406182726.GC8205@sigill.intra.peff.net> URL: https://gitlist.dev/e/20110406182726.GC8205%40sigill.intra.peff.net In-Reply-To: <1302045673-59982-1-git-send-email-jon.seymour@gmail.com> ``` On Wed, Apr 06, 2011 at 09:21:13AM +1000, Jon Seymour wrote: > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh > index 11077f0..5263de7 100755 > --- a/t/t3903-stash.sh > +++ b/t/t3903-stash.sh > @@ -543,11 +543,11 @@ test_expect_success 'invalid ref of the form stash@{n}, n >= N' ' > echo bar6 > file2 && > git add file2 && > git stash && > - test_must_fail git drop stash@{1} && > - test_must_fail git pop stash@{1} && > - test_must_fail git apply stash@{1} && > - test_must_fail git show stash@{1} && > - test_must_fail git branch tmp stash@{1} && > + test_must_fail git stash drop stash@{1} && > + test_must_fail git stash pop stash@{1} && > + test_must_fail git stash apply stash@{1} && > + test_must_fail git stash show stash@{1} && > + test_must_fail git stash branch tmp stash@{1} && > git stash drop Probably we should just squash your fix in with my first patch, and drop my test. Your fixed version is a superset of what mine tests. -Peff ``` ## Jon Seymour, 2011-04-06 22:47 Subject: Re: [PATCH] stash: fix false positive in the invalid ref test. Message-ID: URL: https://gitlist.dev/e/BANLkTimqG4XSTRGH-%2BEMi6vYL7N8JU1Mtw%40mail.gmail.com In-Reply-To: <20110406182726.GC8205@sigill.intra.peff.net> ``` On Thu, Apr 7, 2011 at 4:27 AM, Jeff King wrote: > On Wed, Apr 06, 2011 at 09:21:13AM +1000, Jon Seymour wrote:     git stash drop > > Probably we should just squash your fix in with my first patch, and drop > my test.  Your fixed version is a superset of what mine tests. > I was wondering if it might be better to break up the original test, so that each sub-function has its own invalid ref test? If there is agreement, I can do this on top of 9355fc which is the tip of the branch containing these fixes that has been merged into pu. jon. ``` ## Jeff King, 2011-04-06 23:04 Subject: Re: [PATCH] stash: fix false positive in the invalid ref test. Message-ID: <20110406230459.GC20904@sigill.intra.peff.net> URL: https://gitlist.dev/e/20110406230459.GC20904%40sigill.intra.peff.net In-Reply-To: ``` On Thu, Apr 07, 2011 at 08:47:07AM +1000, Jon Seymour wrote: > On Thu, Apr 7, 2011 at 4:27 AM, Jeff King wrote: > > On Wed, Apr 06, 2011 at 09:21:13AM +1000, Jon Seymour wrote: >     git stash drop > > > > Probably we should just squash your fix in with my first patch, and drop > > my test.  Your fixed version is a superset of what mine tests. > > > > I was wondering if it might be better to break up the original test, > so that each sub-function has its own invalid ref test? If there is > agreement, I can do this on top of 9355fc which is the tip of the > branch containing these fixes that has been merged into pu. I don't think it's that big a deal. You could do a more thorough test for each, I guess (to make sure they not just failed but also show that they didn't have any effects), but that would not be checking an actual regression we have seen. IOW, I'm sure you have something more productive to do with your time. :) -Peff ```