Follow-up to #831 (#835)

* change failing test hint

* mark .failing as code in readme

* group and reword failing tests

The tests are better when grouped together. I tried to give them more consistent titles too.

* remove spurious trailing space in t.end error message

Author: novemberborn

lib/test.js

@@ -191,7 +191,7 @@ Test.prototype._result = function () {
 	if (this.metadata.failing) {
 		passed = !passed;
 		if (!passed) {
-			reason = new Error('Test was expected to fail, but succeeded, you should unmark the test as failing');
+			reason = new Error('Test was expected to fail, but succeeded, you should stop marking the test as failing');
 		}
 	}
 	return {passed: passed, result: this, reason: reason};
@@ -203,7 +203,7 @@ Object.defineProperty(Test.prototype, 'end', {
 			return this._end.bind(this);
 		}
 
-		throw new Error('t.end is not supported in this context. To use t.end as a callback, you must use "callback mode" via `test.cb(testName, fn)` ');
+		throw new Error('t.end is not supported in this context. To use t.end as a callback, you must use "callback mode" via `test.cb(testName, fn)`');
 	}
 });
 

readme.md

@@ -417,7 +417,7 @@ test.todo('will think about writing this later');
 
 ### Failing tests
 
-You can use the `.failing` modifier to document issues with your code that need to be fixed. Failing tests are run just like normal ones, but they are expected to fail, and will not break your build when they do. If a test marked as failing actually passes, it will be reported as an error and fail the build with a helpful message instructing you to remove the .failing modifier.
+You can use the `.failing` modifier to document issues with your code that need to be fixed. Failing tests are run just like normal ones, but they are expected to fail, and will not break your build when they do. If a test marked as failing actually passes, it will be reported as an error and fail the build with a helpful message instructing you to remove the `.failing` modifier.
 
 This allows you to merge `.failing` tests before a fix is implemented without breaking CI. This is a great way to recognize good bug report PR's with a commit credit, even if the reporter is unable to actually fix the problem.
 

test/test.js

@@ -5,7 +5,7 @@ var delay = require('delay');
 var isPromise = require('is-promise');
 var Test = require('../lib/test');
 
-var failingTestHint = 'Test was expected to fail, but succeeded, you should unmark the test as failing';
+var failingTestHint = 'Test was expected to fail, but succeeded, you should stop marking the test as failing';
 
 function ava(title, fn, contextRef, report) {
 	var t = new Test(title, fn, contextRef, report);
@@ -48,25 +48,6 @@ test('run test', function (t) {
 	t.end();
 });
 
-test('expected failing test', function (t) {
-	var result = ava.failing('foo', function (a) {
-		a.fail();
-	}).run();
-
-	t.is(result.passed, true);
-	t.end();
-});
-
-test('fail a failing test if it pass', function (t) {
-	var result = ava.failing('foo', function (a) {
-		a.pass();
-	}).run();
-
-	t.is(result.passed, false);
-	t.is(result.reason.message, failingTestHint);
-	t.end();
-});
-
 test('title is optional', function (t) {
 	var result = ava(function (a) {
 		a.pass();
@@ -171,27 +152,6 @@ test('end can be used as callback with error', function (t) {
 	});
 });
 
-test('fail a failing callback test if it passed', function (t) {
-	ava.cb.failing(function (a) {
-		a.end();
-	}).run().then(function (result) {
-		t.is(result.passed, false);
-		t.is(result.reason.message, failingTestHint);
-		t.end();
-	});
-});
-
-test('failing can work with callback', function (t) {
-	var err = new Error('failed');
-	ava.cb.failing(function (a) {
-		a.end(err);
-	}).run().then(function (result) {
-		t.is(result.passed, true);
-		t.is(result.reason, err);
-		t.end();
-	});
-});
-
 test('end can be used as callback with a non-error as its error argument', function (t) {
 	var nonError = {foo: 'bar'};
 	ava.cb(function (a) {
@@ -642,10 +602,48 @@ test('it is an error to set context in a hook', function (t) {
 	t.end();
 });
 
-test('failing test returns a resolved promise is failure', function (t) {
-	ava.failing(function (a) {
-		a.plan(1);
-		a.notThrows(delay(10), 'foo');
+test('failing tests should fail', function (t) {
+	var result = ava.failing('foo', function (a) {
+		a.fail();
+	}).run();
+
+	t.is(result.passed, true);
+	t.end();
+});
+
+test('failing callback tests should end with an error', function (t) {
+	var err = new Error('failed');
+	ava.cb.failing(function (a) {
+		a.end(err);
+	}).run().then(function (result) {
+		t.is(result.passed, true);
+		t.is(result.reason, err);
+		t.end();
+	});
+});
+
+test('failing tests must not pass', function (t) {
+	var result = ava.failing('foo', function (a) {
+		a.pass();
+	}).run();
+
+	t.is(result.passed, false);
+	t.is(result.reason.message, failingTestHint);
+	t.end();
+});
+
+test('failing callback tests must not pass', function (t) {
+	ava.cb.failing(function (a) {
+		a.end();
+	}).run().then(function (result) {
+		t.is(result.passed, false);
+		t.is(result.reason.message, failingTestHint);
+		t.end();
+	});
+});
+
+test('failing tests must not return a fulfilled promise', function (t) {
+	ava.failing(function () {
 		return Promise.resolve();
 	}).run().then(function (result) {
 		t.is(result.passed, false);
@@ -654,7 +652,7 @@ test('failing test returns a resolved promise is failure', function (t) {
 	});
 });
 
-test('failing test returns a rejected promise is passing', function (t) {
+test('failing tests pass when returning a rejected promise', function (t) {
 	ava.failing(function (a) {
 		a.plan(1);
 		a.notThrows(delay(10), 'foo');
@@ -665,7 +663,7 @@ test('failing test returns a rejected promise is passing', function (t) {
 	});
 });
 
-test('failing test with t.throws(nonThrowingPromise) is passing', function (t) {
+test('failing tests pass with `t.throws(nonThrowingPromise)`', function (t) {
 	ava.failing(function (a) {
 		a.throws(Promise.resolve(10));
 	}).run().then(function (result) {
@@ -674,7 +672,7 @@ test('failing test with t.throws(nonThrowingPromise) is passing', function (t) {
 	});
 });
 
-test('failing test with t.notThrows(throws) is failure', function (t) {
+test('failing tests fail with `t.notThrows(throws)`', function (t) {
 	ava.failing(function (a) {
 		a.notThrows(Promise.resolve('foo'));
 	}).run().then(function (result) {