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.)
