Re: [PATCH 5/7] t5520: test --rebase with multiple branches
- From
Paul Tan <pyokagan@gmail.com>
- Date
- May 5, 2015, 16:00 UTC
- Message-ID
- <CACRoPnSeJT8MK5esRManC7a7v4D2jA4d8YOSvW90+rnoDxuhTQ@mail.gmail.com>
- In-Reply-To
- <CAGZ79kbSMbhW2FLK05CaQjJjSTqEurB8iyeEBgyA1wbWJ4g2vw@mail.gmail.com>
Hi,
On Tue, May 5, 2015 at 1:09 AM, Stefan Beller <sbeller@google.com> wrote:
Show 25 quoted lines
> On Sat, May 2, 2015 at 8:37 AM, Paul Tan <pyokagan@gmail.com> wrote: >> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh >> index 25d519d..17c63ff 100755 >> --- a/t/t5520-pull.sh >> +++ b/t/t5520-pull.sh >> @@ -223,6 +223,14 @@ test_expect_success '--rebase' ' >> test $(git rev-parse HEAD^) = $(git rev-parse copy) && >> test new = $(git show HEAD:file2) >> ' >> + >> +test_expect_success '--rebase fails with multiple branches' ' >> + git reset --hard before-rebase && >> + test_must_fail git pull --rebase . copy master 2>out && >> + test_when_finished "rm -f out" && > > I think it would make sense to switch the previous 2 lines, because the > test_when_finished should also be run if the test actually fails. > The actual tested part may segfault, which would interrupt the && chain > from reaching the test_when_finished line. And here, the line to test > is meant to be the "git pull --rebase" line, so we'd assume it may be > stopping there due to failures in the tested program. > > If you grep through the code base, you'll find lots of test_when_finished > commands as the very first command (or the earliest spot where it makes > sense).
Hmm, thinking about it again, I actually don't really think there's that much of a need to "rm -f out" in this case ;-)
But yes, I should go review the placements of the test_when_finished calls :-).
Thanks.