Felipe Contreras [off-list ref] writes:
On Tue, Sep 3, 2013 at 5:59 PM, Junio C Hamano [off-list ref] wrote:
quoted
Felipe Contreras [off-list ref] writes:
quoted
On Tue, Sep 3, 2013 at 2:32 PM, Junio C Hamano [off-list ref] wrote:
quoted
Felipe Contreras [off-list ref] writes:
quoted
quoted
test_expect_success 'refuse --edit-description on unborn branch for now' '
+ test_when_finished "git checkout -" &&
I am not sure if this is a good change. Depending on the outcome of
the "git checkout" in the test (it may succeed and set @{-1} to the
branch we were on when we entered the test, or it may fail and leave
@{-1} to the branch before we were on when we entered the test),
this will take us to a different place, no?
It is better than leaving Git in an unborn branch.
True, but it falls short of "After every test the environment should
be as close as to how it was before as possible.", doesn't it?
No it doesn't. Closer is closer.
Closer is not "as close as ... possible".
I think the fix is just to use the name of a concrete branch we want
the next test to start on, instead of "-", which could be far
quickly done than making excuses like that.
On Wed, Sep 4, 2013 at 12:19 PM, Junio C Hamano [off-list ref] wrote:
Felipe Contreras [off-list ref] writes:
quoted
On Tue, Sep 3, 2013 at 5:59 PM, Junio C Hamano [off-list ref] wrote:
quoted
Felipe Contreras [off-list ref] writes:
quoted
On Tue, Sep 3, 2013 at 2:32 PM, Junio C Hamano [off-list ref] wrote:
quoted
Felipe Contreras [off-list ref] writes:
quoted
quoted
test_expect_success 'refuse --edit-description on unborn branch for now' '
+ test_when_finished "git checkout -" &&
I am not sure if this is a good change. Depending on the outcome of
the "git checkout" in the test (it may succeed and set @{-1} to the
branch we were on when we entered the test, or it may fail and leave
@{-1} to the branch before we were on when we entered the test),
this will take us to a different place, no?
It is better than leaving Git in an unborn branch.
True, but it falls short of "After every test the environment should
be as close as to how it was before as possible.", doesn't it?
No it doesn't. Closer is closer.
Closer is not "as close as ... possible".
Yes it is, because there are no better alternatives within our
ability, capacity, or realization.
http://www.merriam-webster.com/dictionary/possible
I think the fix is just to use the name of a concrete branch we want
the next test to start on, instead of "-", which could be far
quickly done than making excuses like that.
No, what if that test is skipped for some reason? Ideally each test
case should do 'git checkout $branch_i_need', not rely on the previous
tests to checkout the branch they need, but that requires too much
work to fix.
'git checkout -' works perfectly fine, and it's the closest we have to
my suggested test_checkout() which I think would be ideal, but I'm not
going to work on.
--
Felipe Contreras
On Sat, Sep 07, 2013 at 10:02:59PM -0500, Felipe Contreras wrote:
'git checkout -' works perfectly fine, and it's the closest we have to
my suggested test_checkout() which I think would be ideal, but I'm not
going to work on.
I do not think it works perfectly fine if the checkout itself fails. For
example, after your patch, one of the tests reads:
test_when_finished "git checkout -" &&
git checkout HEAD^{} &&
test_must_fail git branch --set-upstream-to master
The cleanup is undoing what happened on the second line. So if we fail
on the third line, it does what we want. If we fail on the second line,
then what branch are switching to?
I think you would at least want to reverse the order as:
git checkout HEAD^{} &&
test_when_finished "git checkout -" &&
test_must_fail git branch --set-upstream-to master
so that you know you are actually going to the pre-test state.
-Peff
On Sat, Sep 7, 2013 at 11:56 PM, Jeff King [off-list ref] wrote:
On Sat, Sep 07, 2013 at 10:02:59PM -0500, Felipe Contreras wrote:
quoted
'git checkout -' works perfectly fine, and it's the closest we have to
my suggested test_checkout() which I think would be ideal, but I'm not
going to work on.
I do not think it works perfectly fine if the checkout itself fails. For
example, after your patch, one of the tests reads:
test_when_finished "git checkout -" &&
git checkout HEAD^{} &&
test_must_fail git branch --set-upstream-to master
The cleanup is undoing what happened on the second line. So if we fail
on the third line, it does what we want. If we fail on the second line,
then what branch are switching to?
I think you would at least want to reverse the order as:
git checkout HEAD^{} &&
test_when_finished "git checkout -" &&
test_must_fail git branch --set-upstream-to master
so that you know you are actually going to the pre-test state.
Yeah, that would be better and easily achievable.
--
Felipe Contreras