Ramkumar Ramachandra wrote: > 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 [...] > @@ -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. [...] > @@ -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.) [...] > @@ -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@xxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html