Re: [PATCH v1 2/6] Status and checkout unit tests for --porcelain[=<n>]

From: Jeff King <peff@peff.net>
Date: Jul 20, 2016, 16:03 UTC
Message-ID: <20160720160357.GA25076@sigill.intra.peff.net>
In-Reply-To: <20160720160007.GB24902@sigill.intra.peff.net>

On Wed, Jul 20, 2016 at 10:00:07AM -0600, Jeff King wrote:

Show 9 quoted lines

> On Tue, Jul 19, 2016 at 06:10:54PM -0400, Jeff Hostetler wrote:
> 
> > +test_expect_failure '--porcelain=bogus with stuff to commit returns ok' '
> > +	echo bongo bongo bongo >>file &&
> > +	git commit -m next -a --porcelain=bogus
> > +'
> 
> Hrm. That seems unexpected to me. Shouldn't it complain about
> --porcelain=bogus?

Pondering more, did you mean:

  test_expect_success '--porcelain=bogus complains about format' '
	echo bongo bongo bongo >>file &&
	test_must_fail git commit -m next -a --porcelain=bogus
  '

expect_failure is for tests which we _want_ to succeed, but do not yet (so they get annotated in test results appropriately). expect_success is for an outcome we expect to happen, but which may involve specific steps returning failure.

The names are kind of confusing in that regard.

I wonder if just "test_expect" would be a better name for test_expect_success, and an argument or environment variable to trigger "we know this is currently broken" rather than having a separate test_expect_failure function. That's clearly outside the scope of your series, of course.

-Peff