Web lists-archives.com

Re: [PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually




On Mon, Jul 2, 2018 at 1:44 PM Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> > diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
> >  test_expect_success 'diff --no-index with binary creation' '
> >         echo Q | q_to_nul >binary &&
> > -       (: hide error code from diff, which just indicates differences
> > -        git diff --binary --no-index /dev/null binary >current ||
> > -        true
> > -       ) &&
> > +       # hide error code from diff, which just indicates differences
> > +       test_might_fail git diff --binary --no-index /dev/null binary >current &&
>
> I am not sure why we need to be non-deterministic here, i.e. I would rather
> test for success or non-success error code and not just *any* error code.
>
> While I think this patch is a strict improvement for the test suite,
> I do wonder if we can tighten the exit code check here (maybe in
> a follow up series?).

I agree that such a change is out of the scope of this patch series.

But, do keep in mind that the exit code of git-diff doesn't indicate
only "success" or "error". Various exit codes represent different
conditions; for instance, 1 means differences were found, 0 means no
differences, and -1 means error. The outcome of this test is based
upon output generated by git-diff, so the test itself will fail
overall if git-diff doesn't produce expected results. Also, there is
an entire test script (t4017-diff-retval.sh) concerned with verifying
git-diff exit code, so I'm not sure we need to worry about it too much
here.

(I'm not arguing against a follow-up patch, though, just thinking
aloud about its potential value.)