Re: [PATCH 5/9] t3510 (cherry-pick-sequencer): use exit status
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Dec 9, 2011, 20:21 UTC
- Message-ID
- <20111209202149.GH20913@elie.hsd1.il.comcast.net>
- In-Reply-To
- <1323445326-24637-6-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
Show 6 quoted lines
> Since cf3e2486 (revert: Propagate errors upwards from do_pick_commit, > 2011-08-04), 'git cherry-pick' has three different ways of failing: > > 1. die() with the exit status 128. > 3. error() out with exit status -1. > 2. exit with positive exit status to indicate a conflict.
I think your list-item counter is showing a little jitter. :)
error() does not produce exit status -1, and any situation other than propagating exit status from a user-defined script in which git exits with status 255 is a bug (yes, I know there are a couple, though none I know of in cherry-pick code paths).
Hasn't cherry-pick had two ways to exit with failing status like "git merge" does (conflicts versus error that didn't even allow us to start) since the very beginning?
[...]
> So, replace all instances of 'test_must_fail' with > 'test_expect_code' to check the exit status explicitly.
Sounds like a sensible idea. Probably this one sentence, plus a quick note on the user-visible exit status convention, would suffice for explaining it.
> --- a/t/t3510-cherry-pick-sequence.sh > +++ b/t/t3510-cherry-pick-sequence.sh
[...]
Show 7 quoted lines
> @@ -53,7 +53,7 @@ test_expect_success 'cherry-pick persists data on failure' ' > > test_expect_success 'cherry-pick persists opts correctly' ' > pristine_detach initial && > - test_must_fail git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours base..anotherpick && > + test_expect_code 128 git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours base..anotherpick && > test_path_is_dir .git/sequencer &&
Encountered conflicts, preserving options, but the exit is with status 128? Smells like a bug.
[...]
Show 6 quoted lines
> @@ -88,12 +88,12 @@ test_expect_success '--quit does not complain when no cherry-pick is in progress > > test_expect_success '--abort requires cherry-pick in progress' ' > pristine_detach initial && > - test_must_fail git cherry-pick --abort > + test_expect_code 128 git cherry-pick --abort
I don't think the exit status is important for this one. (I.e., I can imagine some future version of cherry-pick using different small positive integers to refer to different reasons for --abort to fail, and I don't think that would be a problem or break anything.)
[...]
Show 9 quoted lines
> @@ -179,9 +179,9 @@ test_expect_success '--abort keeps unrelated change, easy case' ' > test_expect_success '--abort refuses to clobber unrelated change, harder case' ' > pristine_detach initial && > echo changed >expect && > - test_must_fail git cherry-pick base..anotherpick && > + test_expect_code 1 git cherry-pick base..anotherpick && > echo changed >unrelated && > - test_must_fail git cherry-pick --abort && > + test_expect_code 128 git cherry-pick --abort &&
Likewise here.
Except as noted above, Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>