[WIP] A way to submit tests that should fail.

[cgcgbcbc]

see #673

[mention-bot]

By analyzing the blame information on this pull request, we identified @ariporad, @sotojuan and @BarryThePenguin to be potential reviewers

[jfmengels]

There needs to be a test that makes the tests fail when a test passes I'd say.
Thanks for taking this on btw @cgcgbcbc :)

[jamestalmage]

Agreed, needs quite a few more tests.

• failing sync test fails - result is passing
• failing test.cb test fails - result is passing
• failing test with t.throws(nonThrowingPromise) - result is passing (test async assertions)
• failing test returns a rejected promise - result is passing
• failing sync test passes - result is a failure with helpful error message
• failing test.cb test passes - result is a failure
• failing test with t.throws(throws) - result is failure (test async assertions)
• failing test returns a resolved promise - result is failure

[cgcgbcbc]

Just for tracking

• failing sync test fails - result is passing
• failing test.cb test fails - result is passing
• failing test with t.throws(nonThrowingPromise) - result is passing (test async assertions)
• failing test returns a rejected promise - result is passing
• failing sync test passes - result is a failure with helpful error message
• failing test.cb test passes - result is a failure
• failing test with t.throws(throws) - result is failure (test async assertions)
• failing test returns a resolved promise - result is failure

[jamestalmage]

Just for tracking

Any reason you haven't tackled the last two?

// this should pass
test.failing(t => {
  t.throws(Promise.resolve('foo'));
});
// this should fail
test.failing(t => {
  t.notThrows(Promise.resolve('foo'));
});

[cgcgbcbc]

Any reason you haven't tackled the last two?

Cause they failed...

[jamestalmage]

This should fix your failing test:

diff --git a/test/test.js b/test/test.js
index d2f3f1b..9fd23a9 100644
--- a/test/test.js
+++ b/test/test.js
@@ -676,10 +676,10 @@ test('failing test with t.throws(nonThrowingPromise) is passing', function (t) {

 test('failing test with t.throws(throws) is failure', function (t) {
    ava.failing(function (a) {
-       a.throws(Promise.resolve('foo'));
+       a.notThrows(Promise.resolve('foo'));
    }).run().then(function (result) {
        t.is(result.passed, false);
-       t.is(result.message, failingTestHint);
+       t.is(result.reason.message, failingTestHint);
        t.end();
    });
 });

You should update the test title with t.notThrows

[cgcgbcbc]

😅Aha, not the first time making such embarrassing mistake

[jamestalmage]

Awesome!

LGTM!

[jamestalmage]

@cgcgbcbc - Can you add documentation for this to the Readme?

[cgcgbcbc]

@jamestalmage , do you think it's necessary to check that the failing modifier can only be used with test and throws error when used with hooks?

[jamestalmage]

do you think it's necessary to check that the failing modifier can only be used with test and throws error when used with hooks?

No, it's conceivable you could write an afterEach test that failed.

test.failing.afterEach('always leaves X in pristine state', t => {
  t.true(isPristine(t.context.state));
});

That's enough of a corner case that it should probably be flagged by the linter though.

Can you open an issue in eslint-plugin-ava about that?

[cgcgbcbc]

Can you open an issue in eslint-plugin-ava about that?

Do you mean the no-unknown-modifiers rule as I mentioned in avajs/eslint-plugin-ava#107 ?

[jamestalmage]

I just did it: avajs/eslint-plugin-ava#108

[jamestalmage]

I would like to expand on this a bit:

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.

[cgcgbcbc]

@jamestalmage Thank you for your advice! I've fixed the documents.

[jamestalmage]

This looks great to me!

Just need a second reviewer to merge.

// @sindresorhus @novemberborn @vdemedes @sotojuan

[novemberborn]

Looks good @cgcgbcbc!

I have some nits but will follow up with a PR rather than putting you through more back-and-forth 😄

[sotojuan]

Thanks @cgcgbcbc!