Re: [PATCH v2 1/3] Add tests for describe with --work-tree
- Date: Wed, 30 Jan 2019 08:47:26 -0800
- From: Junio C Hamano <gitster@xxxxxxxxx>
- Subject: Re: [PATCH v2 1/3] Add tests for describe with --work-tree
Duy Nguyen <pclouds@xxxxxxxxx> writes:
>> My function is a modified version of check_describe().
> Whoa. That function is 12 years old! I think our style has evolved a
> bit since then.
> I mean chaining within a test. This is to make sure any failure
> triggers the test failure (as it should, if some command is expected
> to fail, we have other ways to catch it).
> I would start with something simple, not using shell function at
> all. Something like this as an example (I added run_describe() because
> that "git" command becomes too long). Have a look at the "do's and
> don'ts" in t/README too.
Thanks for guiding new contributors with an easy to understand example.
> BTW, careful about _success or _failure. You need to make sure bisect
> is not broken. If you add a test to confirm a broken case then it
> should be test_expect_failure (and the test suite will pass). Then
> when you fix it you can flip it to test_expect_success.
And if the fix is simple enough (i.e. a good rule of thumb is if the
fixes themselves without tests need to be multi-patch series, it is
not simple enough), have a single patch that has both fix and test
that expects success, instead of splitting them into two to make a
two patch series whose first step expects a failure and whose second
step fixes and flips failure to success.