Re: [PATCHv4 02/15] t4017 (diff-retval): replace manual exit code check with test_expect_code

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 1, 2010, 16:20 UTC
Message-ID: <7vd3rtholo.fsf@alter.siamese.dyndns.org>
In-Reply-To: <AANLkTinqVTqxiHL5tEv+-SS6YURGUoWaPxCgpccZgjEq@mail.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 10 quoted lines

> On Fri, Oct 1, 2010 at 10:23, Jonathan Nieder <jrnieder@gmail.com> 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.