Skip to content

Fail tests that finish with pending assertions #1335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 5, 2017
Merged

Fail tests that finish with pending assertions #1335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cf24bcf
Reword error message for when assertions are used after test has fini…
novemberborn Apr 3, 2017
15e9ae3
Remove unnecessary test to verify throws/notThrows return promises
novemberborn Apr 3, 2017
66c664d
Fail tests that finish with pending assertions
novemberborn Apr 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 20 additions & 22 deletions lib/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class Test {
this.finishDueToAttributedError = null;
this.finishDueToInactivity = null;
this.finishing = false;
this.pendingAssertions = [];
this.pendingAssertionCount = 0;
this.pendingThrowsAssertion = null;
this.planCount = null;
this.startedAt = 0;
Expand Down Expand Up @@ -154,24 +154,27 @@ class Test {

countPassedAssertion() {
if (this.finishing) {
this.saveFirstError(new Error('Assertion passed, but test has already ended'));
this.saveFirstError(new Error('Assertion passed, but test has already finished'));
}

this.assertCount++;
}

addPendingAssertion(promise) {
if (this.finishing) {
this.saveFirstError(new Error('Assertion passed, but test has already ended'));
this.saveFirstError(new Error('Assertion passed, but test has already finished'));
}

this.assertCount++;
this.pendingAssertions.push(promise.catch(err => this.saveFirstError(err)));
this.pendingAssertionCount++;
promise
.catch(err => this.saveFirstError(err))
.then(() => this.pendingAssertionCount--);
}

addFailedAssertion(error) {
if (this.finishing) {
this.saveFirstError(new Error('Assertion failed, but test has already ended'));
this.saveFirstError(new Error('Assertion failed, but test has already finished'));
}

this.assertCount++;
Expand Down Expand Up @@ -208,8 +211,12 @@ class Test {
}

verifyAssertions() {
if (this.failWithoutAssertions && !this.assertError && !this.calledEnd && this.planCount === null && this.assertCount === 0) {
this.saveFirstError(new Error('Test finished without running any assertions'));
if (!this.assertError) {
if (this.failWithoutAssertions && !this.calledEnd && this.planCount === null && this.assertCount === 0) {
this.saveFirstError(new Error('Test finished without running any assertions'));
} else if (this.pendingAssertionCount > 0) {
this.saveFirstError(new Error('Test finished, but an assertion is still pending'));
}
}
}

Expand Down Expand Up @@ -375,21 +382,6 @@ class Test {
this.verifyPlan();
this.verifyAssertions();

if (this.assertError || this.pendingAssertions.length === 0) {
return this.completeFinish();
}

// Finish after potential errors from pending assertions have been consumed.
return Promise.all(this.pendingAssertions).then(() => this.completeFinish());
}

finishPromised() {
return new Promise(resolve => {
resolve(this.finish());
});
}

completeFinish() {
this.duration = globals.now() - this.startedAt;

let reason = this.assertError;
Expand All @@ -413,6 +405,12 @@ class Test {

return passed;
}

finishPromised() {
return new Promise(resolve => {
resolve(this.finish());
});
}
}

module.exports = Test;
16 changes: 16 additions & 0 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -964,10 +964,26 @@ test('rejects', async t => {
});
```

When testing a promise you must wait for the assertion to complete:

```js
test('rejects', async t => {
await t.throws(promise);
});
```

### `.notThrows(function|promise, [message])`

Assert that `function` does not throw an error or that `promise` does not reject with an error.

Like the `.throws()` assertion, when testing a promise you must wait for the assertion to complete:

```js
test('rejects', async t => {
await t.notThrows(promise);
});
```

### `.regex(contents, regex, [message])`

Assert that `contents` matches `regex`.
Expand Down
184 changes: 64 additions & 120 deletions test/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict';
const test = require('tap').test;
const delay = require('delay');
const isPromise = require('is-promise');
const formatValue = require('../lib/format-assert-error').formatValue;
const Test = require('../lib/test');

Expand Down Expand Up @@ -451,10 +450,12 @@ test('throws and notThrows work with promises', t => {
let result;
ava(a => {
a.plan(2);
a.throws(delay.reject(10, new Error('foo')), 'foo');
a.notThrows(delay(20).then(() => {
asyncCalled = true;
}));
return Promise.all([
a.throws(delay.reject(10, new Error('foo')), 'foo'),
a.notThrows(delay(20).then(() => {
asyncCalled = true;
}))
]);
}, null, r => {
result = r;
}).run().then(passed => {
Expand Down Expand Up @@ -496,154 +497,97 @@ test('cb test that throws sync', t => {
t.end();
});

test('waits for t.throws to resolve after t.end is called', t => {
test('multiple resolving and rejecting promises passed to t.throws/t.notThrows', t => {
let result;
ava.cb(a => {
a.plan(1);
a.notThrows(delay(10), 'foo');
a.end();
ava(a => {
a.plan(6);
const promises = [];
for (let i = 0; i < 3; i++) {
promises.push(
a.throws(delay.reject(10, new Error('foo')), 'foo'),
a.notThrows(delay(10), 'foo')
);
}
return Promise.all(promises);
}, null, r => {
result = r;
}).run().then(passed => {
t.is(passed, true);
t.is(result.result.planCount, 1);
t.is(result.result.assertCount, 1);
t.is(result.result.planCount, 6);
t.is(result.result.assertCount, 6);
t.end();
});
});

test('waits for t.throws to reject after t.end is called', t => {
test('fails if test ends while there are pending assertions', t => {
let result;
ava.cb(a => {
a.plan(1);
a.throws(delay.reject(10, new Error('foo')), 'foo');
a.end();
const passed = ava(a => {
a.throws(Promise.reject(new Error()));
}, null, r => {
result = r;
}).run().then(passed => {
t.is(passed, true);
t.is(result.result.planCount, 1);
t.is(result.result.assertCount, 1);
t.end();
});
});
}).run();

test('waits for t.throws to resolve after the promise returned from the test resolves', t => {
let result;
ava(a => {
a.plan(1);
a.notThrows(delay(10), 'foo');
return Promise.resolve();
}, null, r => {
result = r;
}).run().then(passed => {
t.is(passed, true);
t.is(result.result.planCount, 1);
t.is(result.result.assertCount, 1);
t.end();
});
t.is(passed, false);
t.is(result.reason.name, 'Error');
t.match(result.reason.message, /Test finished, but an assertion is still pending/);
t.end();
});

test('waits for t.throws to reject after the promise returned from the test resolves', t => {
test('fails if callback test ends while there are pending assertions', t => {
let result;
ava(a => {
a.plan(1);
a.throws(delay.reject(10, new Error('foo')), 'foo');
return Promise.resolve();
const passed = ava.cb(a => {
a.throws(Promise.reject(new Error()));
a.end();
}, null, r => {
result = r;
}).run().then(passed => {
t.is(passed, true);
t.is(result.result.planCount, 1);
t.is(result.result.assertCount, 1);
t.end();
});
});
}).run();

test('multiple resolving and rejecting promises passed to t.throws/t.notThrows', t => {
let result;
ava(a => {
a.plan(6);
for (let i = 0; i < 3; i++) {
a.throws(delay.reject(10, new Error('foo')), 'foo');
a.notThrows(delay(10), 'foo');
}
}, null, r => {
result = r;
}).run().then(passed => {
t.is(passed, true);
t.is(result.result.planCount, 6);
t.is(result.result.assertCount, 6);
t.end();
});
t.is(passed, false);
t.is(result.reason.name, 'Error');
t.match(result.reason.message, /Test finished, but an assertion is still pending/);
t.end();
});

test('number of assertions matches t.plan when the test exits, but before all pending assertions resolve another is added', t => {
test('fails if async test ends while there are pending assertions', t => {
let result;
ava(a => {
a.plan(2);
a.throws(delay.reject(10, new Error('foo')), 'foo');
a.notThrows(delay(10), 'foo');
setTimeout(() => {
a.pass();
}, 5);
a.throws(Promise.reject(new Error()));
return Promise.resolve();
}, null, r => {
result = r;
}).run().then(passed => {
t.is(passed, false);
t.match(result.reason.message, /Assertion passed, but test has already ended/);
t.is(result.reason.name, 'Error');
t.match(result.reason.message, /Test finished, but an assertion is still pending/);
t.end();
});
});

test('number of assertions matches t.plan when the test exits, but before all pending assertions resolve, a failing assertion is added', t => {
let result;
ava(a => {
a.plan(2);
a.throws(delay.reject(10, new Error('foo')), 'foo');
a.notThrows(delay(10), 'foo');
setTimeout(() => {
a.fail();
}, 5);
}, null, r => {
result = r;
}).run().then(passed => {
t.is(passed, false);
t.match(result.reason.message, /Assertion failed, but test has already ended/);
t.is(result.reason.name, 'Error');
t.end();
});
});
// This behavior is incorrect, but feedback cannot be provided to the user due to
// https://github.com/avajs/ava/issues/1330
test('no crash when adding assertions after the test has ended', t => {
t.plan(3);

test('number of assertions doesn\'t match plan when the test exits, but before all promises resolve another is added', t => {
let result;
const passed = ava(a => {
a.plan(3);
a.throws(delay.reject(10, new Error('foo')), 'foo');
a.notThrows(delay(10), 'foo');
setTimeout(() => {
a.throws(Promise.reject(new Error('foo')), 'foo');
}, 5);
}, null, r => {
result = r;
ava(a => {
a.pass();
setImmediate(() => {
t.doesNotThrow(() => a.pass());
});
}).run();

t.is(passed, false);
t.is(result.reason.assertion, 'plan');
t.is(result.reason.operator, '===');
t.end();
});
ava(a => {
a.pass();
setImmediate(() => {
t.doesNotThrow(() => a.fail());
});
}).run();

test('assertions return promises', t => {
ava(a => {
a.plan(2);
t.ok(isPromise(a.throws(Promise.reject(new Error('foo')))));
t.ok(isPromise(a.notThrows(Promise.resolve(true))));
}).run().then(passed => {
t.is(passed, true);
t.end();
});
a.pass();
setImmediate(() => {
t.doesNotThrow(() => a.notThrows(Promise.resolve()));
});
}).run();
});

test('contextRef', t => {
Expand Down Expand Up @@ -737,8 +681,8 @@ test('failing tests must not return a fulfilled promise', t => {
test('failing tests pass when returning a rejected promise', t => {
ava.failing(a => {
a.plan(1);
a.notThrows(delay(10), 'foo');
return Promise.reject();
return a.notThrows(delay(10), 'foo')
.then(() => Promise.reject());
}).run().then(passed => {
t.is(passed, true);
t.end();
Expand All @@ -747,7 +691,7 @@ test('failing tests pass when returning a rejected promise', t => {

test('failing tests pass with `t.throws(nonThrowingPromise)`', t => {
ava.failing(a => {
a.throws(Promise.resolve(10));
return a.throws(Promise.resolve(10));
}).run().then(passed => {
t.is(passed, true);
t.end();
Expand All @@ -757,7 +701,7 @@ test('failing tests pass with `t.throws(nonThrowingPromise)`', t => {
test('failing tests fail with `t.notThrows(throws)`', t => {
let result;
ava.failing(a => {
a.notThrows(Promise.resolve('foo'));
return a.notThrows(Promise.resolve('foo'));
}, null, r => {
result = r;
}).run().then(passed => {
Expand Down