Skip to content

Breaking: t.end requires test.async, and t.plan will not auto end tests. Fixes #244. #253

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
Nov 27, 2015
Merged

Breaking: t.end requires test.async, and t.plan will not auto end tests. Fixes #244. #253

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
387c30a
Breaking: t.end requires test be declared async
jamestalmage Nov 22, 2015
f816815
Breaking: t.plan() will not auto-end tests.
jamestalmage Nov 24, 2015
982a451
update documentation to reflect new behaviors
jamestalmage Nov 25, 2015
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
6 changes: 4 additions & 2 deletions lib/runner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ function Runner(opts) {
type: 'test',
serial: false,
exclusive: false,
skipped: false
skipped: false,
async: false
}, this._addFn.bind(this));
}

Expand All @@ -47,7 +48,8 @@ var chainableFunctions = {
skip: {skipped: true},
only: {exclusive: true},
beforeEach: {type: 'beforeEach'},
afterEach: {type: 'afterEach'}
afterEach: {type: 'afterEach'},
async: {async: true}
};

function makeChain(defaults, parentAdd) {
Expand Down
39 changes: 30 additions & 9 deletions lib/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var fnName = require('fn-name');
var co = require('co-with-promise');
var observableToPromise = require('observable-to-promise');
var isPromise = require('is-promise');
var isObservable = require('is-observable');
var assert = require('./assert');
var globals = require('./globals');

Expand All @@ -28,6 +29,10 @@ function Test(title, fn) {
this.duration = null;
this.assertError = undefined;

// TODO(jamestalmage): make this an optional constructor arg instead of having Runner set it after the fact.
// metadata should just always exist, otherwise it requires a bunch of ugly checks all over the place.
this.metadata = {};

// store the time point before test execution
// to calculate the total time spent in test
this._timeStart = null;
Expand All @@ -46,10 +51,6 @@ module.exports = Test;

Test.prototype._assert = function () {
this.assertCount++;

if (this.assertCount === this.planCount) {
globals.setImmediate(this.exit.bind(this));
}
};

// patch assert methods to increase assert count and store errors
Expand Down Expand Up @@ -132,14 +133,23 @@ Test.prototype.run = function () {
this.exit();
}

ret = observableToPromise(ret);
var asyncType = 'promises';

if (isObservable(ret)) {
asyncType = 'observables';
ret = observableToPromise(ret);
}

if (isPromise(ret)) {
if (this.metadata.async) {
self._setAssertError(new Error('Do not return ' + asyncType + ' from tests declared via `test.async(...)`, if you want to return a promise simply declare the test via `test(...)`'));
this.exit();
return this.promise.promise;
}

ret
.then(function () {
if (!self.planCount || self.planCount === self.assertCount) {
self.exit();
}
self.exit();
})
.catch(function (err) {
self._setAssertError(new assert.AssertionError({
Expand All @@ -150,12 +160,23 @@ Test.prototype.run = function () {

self.exit();
});
} else if (!this.metadata.async) {
this.exit();
}

return this.promise.promise;
};

Test.prototype.end = function (err) {
Object.defineProperty(Test.prototype, 'end', {
get: function () {
if (this.metadata.async) {
return this._end;
}
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)` ');
}
});

Test.prototype._end = function (err) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just this here instead?

if (!this.metadata.async) {
    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)` ');
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean just have t.end throw when executed?

Because we want to fail fast, and if passed as a callback:

   fs.readFile(path, t.end);

We are waiting for the function to actually be called instead of throwing ASAP.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it matter? It shouldn't be used with test() anyways. So I don't see how it makes a difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they don't return a promise - we assume sync. We might mark the test done before the callback returns.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, never mind. Got it now. Sorry about the noise.

if (err) {
this._setAssertError(new assert.AssertionError({
actual: err,
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
"has-flag": "^1.0.0",
"has-generator": "^1.0.0",
"is-generator-fn": "^1.0.0",
"is-observable": "^0.1.0",
"is-promise": "^2.1.0",
"loud-rejection": "^1.2.0",
"max-timeout": "^1.0.0",
Expand Down
Loading