From: Junio C Hamano Date: Fri, 01 Oct 2010 16:20:03 GMT Subject: Re: [PATCHv4 02/15] t4017 (diff-retval): replace manual exit code check with test_expect_code Message-ID: <7vd3rtholo.fsf@alter.siamese.dyndns.org> In-Reply-To: Ævar Arnfjörð Bjarmason writes: > On Fri, Oct 1, 2010 at 10:23, Jonathan Nieder wrote: > >> test_expect_success 'git diff --quiet HEAD^^ HEAD^' ' >> - test_must_fail git diff --quiet HEAD^^ HEAD^ >> + check_exit_status 1 git diff --quiet HEAD^^ HEAD^ >> ' > > In most uses of check_exit_status you're using it is the very last > command within a test_expect_success. Isn't it redundant to using just > "test_expect_code $code ..." there? Doesn't test_expect_code 1 'phoney' ' foo && bar && (exit 1) ' incorrectly succeed if "foo" or "bar" happened to fail with exit status 1? > So IMO the best thing to do would be to re-appropriate > "test_expect_code" so that it runs inside a test (i.e. does what your > check_exit_status does), and not at the top-level. I like it. It should have been done the way you suggest from the beginning. It is unfortunate that we didn't think of wrappers like test_must_fail and friends when we originally did test_expect_code, but it is never too late to correct that mistake.