Skip to content

Commit 387c30a

Browse files
committed
Breaking: t.end requires test be declared async
Accessing `t.end` now throws an error unless the test is first declared async via `test.async([title], fn)` Reference: #244 (cherry picked from commit 28b1641) Initial implementation of #244 - require explicit test.async
1 parent 7764336 commit 387c30a

18 files changed

+216
-151
lines changed

lib/runner.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ function Runner(opts) {
3333
type: 'test',
3434
serial: false,
3535
exclusive: false,
36-
skipped: false
36+
skipped: false,
37+
async: false
3738
}, this._addFn.bind(this));
3839
}
3940

@@ -47,7 +48,8 @@ var chainableFunctions = {
4748
skip: {skipped: true},
4849
only: {exclusive: true},
4950
beforeEach: {type: 'beforeEach'},
50-
afterEach: {type: 'afterEach'}
51+
afterEach: {type: 'afterEach'},
52+
async: {async: true}
5153
};
5254

5355
function makeChain(defaults, parentAdd) {

lib/test.js

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ function Test(title, fn) {
2828
this.duration = null;
2929
this.assertError = undefined;
3030

31+
// TODO(jamestalmage): make this an optional constructor arg instead of having Runner set it after the fact.
32+
// metadata should just always exist, otherwise it requires a bunch of ugly checks all over the place.
33+
// I will fix this correctly in the upstream PR.
34+
this.metadata = {};
35+
3136
// store the time point before test execution
3237
// to calculate the total time spent in test
3338
this._timeStart = null;
@@ -47,7 +52,9 @@ module.exports = Test;
4752
Test.prototype._assert = function () {
4853
this.assertCount++;
4954

50-
if (this.assertCount === this.planCount) {
55+
// TODO(jamestalmage): no only auto-ends in "declared" async mode `test.async(...)`. Should we drop auto-end behavior all together?
56+
// https://github.com/sindresorhus/ava/issues/244#issuecomment-158705839
57+
if (this.metadata.async && this.assertCount === this.planCount) {
5158
globals.setImmediate(this.exit.bind(this));
5259
}
5360
};
@@ -135,27 +142,38 @@ Test.prototype.run = function () {
135142
ret = observableToPromise(ret);
136143

137144
if (isPromise(ret)) {
138-
ret
139-
.then(function () {
140-
if (!self.planCount || self.planCount === self.assertCount) {
141-
self.exit();
142-
}
143-
})
144-
.catch(function (err) {
145-
self._setAssertError(new assert.AssertionError({
146-
actual: err,
147-
message: 'Promise rejected → ' + err,
148-
operator: 'promise'
149-
}));
150-
145+
if (!this.metadata.async) {
146+
ret = ret.then(function () {
151147
self.exit();
152148
});
149+
}
150+
151+
ret.catch(function (err) {
152+
self._setAssertError(new assert.AssertionError({
153+
actual: err,
154+
message: 'Promise rejected → ' + err,
155+
operator: 'promise'
156+
}));
157+
158+
self.exit();
159+
});
160+
} else if (!this.metadata.async) {
161+
this.exit();
153162
}
154163

155164
return this.promise.promise;
156165
};
157166

158-
Test.prototype.end = function (err) {
167+
Object.defineProperty(Test.prototype, 'end', {
168+
get: function () {
169+
if (this.metadata.async) {
170+
return this._end;
171+
}
172+
throw new Error('t.end is not supported in this context. To use t.end as a callback, you must explicitly declare the test asynchronous via `test.async(testName, fn)` ');
173+
}
174+
});
175+
176+
Test.prototype._end = function (err) {
159177
if (err) {
160178
this._setAssertError(new assert.AssertionError({
161179
actual: err,

test/fixture/circular-reference-on-assertion.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,4 @@ test(t => {
44
var circular = ['a', 'b'];
55
circular.push(circular);
66
t.same([circular, 'c'], [circular, 'd']);
7-
t.end();
87
});

test/fixture/es2015.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,4 @@ import test from '../../';
22

33
test(t => {
44
t.pass();
5-
t.end();
65
});

test/fixture/fail-fast.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@ import test from '../../';
22

33
test(t => {
44
t.fail();
5-
t.end();
65
});
76

87
test(t => {
98
t.pass();
10-
t.end();
119
});

test/fixture/hooks-failing.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@ test.beforeEach(fail);
44
test(pass);
55

66
function pass(t) {
7-
t.end();
87
}
98

109
function fail(t) {
1110
t.fail();
12-
t.end();
1311
}

test/fixture/hooks-passing.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,4 @@ test.afterEach(pass);
77
test(pass);
88

99
function pass(t) {
10-
t.end();
1110
}

test/fixture/long-running.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
const test = require('../../');
33
var onExit = require('signal-exit');
44

5-
test('long running', function (t) {
5+
test.async('long running', function (t) {
66
t.plan(1);
77

88
onExit(function () {

test/fixture/loud-rejection.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
const test = require('../../');
22

3-
test('creates an unhandled rejection', t => {
3+
test.async('creates an unhandled rejection', t => {
44
Promise.reject(new Error(`You can't handle this!`));
55

66
setTimeout(function () {

test/fixture/one-pass-one-fail.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@ const test = require('../../');
22

33
test('this is a passing test', t => {
44
t.ok(true);
5-
t.end();
65
});
76

87
test('this is a failing test', t => {
98
t.ok(false);
10-
t.end();
119
});

0 commit comments

Comments
 (0)