-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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.
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
Add programmatic API #279
Add programmatic API #279
Conversation
I think this should go in the base directory: require('ava/api');
// is better than
require('ava/lib/api'); |
this.tests = []; | ||
|
||
Object.keys(Api.prototype).forEach(function (key) { | ||
this[key] = this[key].bind(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a typeof check?
if (typeof this[key] === 'function') {
this[key] = this[key].bind(this);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from lib/test.js
.
@vdemedes - see my change. The issue you were having was |
I propose we change this up just a bit. Rather than Use the array: on('unhandledRejection', function(e) {
self.errors.push({type: 'unhandledRejection', error: e});
});
on('uncaughtException', function(e) {
self.errors.push({type: 'uncaughtException', error: e});
});
// ...
self.errors.push({
type: 'nonZeroExit',
error: new Error('process exited with code: ' + code)
});
// ...
self.errors.push({
type: 'assertionError',
// ...
}); Then extend API with methods like: api.hasErrors(); // true / false
api.getErrors(); // returns all {type: typeName, error: errorObj}
api.getErrors(typeName); // returns all errors of matching type. I think the promise returned by the API should almost always resolve, and almost never reject. Users will then use We should continue emitting events so people can implement their own realtime logging, but it should be possible to create basically the same log output after the fact using just the array returned from Thoughts? |
I've shared a number of thoughts, but overall, very much 👍 |
I'm not sure of this one. I like that We might be able to move the majority of them to |
testCount += stats.testCount; | ||
if (cli.flags.init) { | ||
require('ava-init')(); | ||
process.exit(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't process.exit()
here as ava-init
won't have a time to finish. Just return
and XO will pass when https://github.com/sindresorhus/ava/pull/277/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R132 lands.
@jamestalmage Yup, that's the plan. We still want to explicitly test the CLI, but many of the current CLI tests really just test "API" behavior. |
👍 Overall looks really good! |
@jamestalmage I appreciate your suggestions, but I'd like to ask you not to modify WIP-kind of PRs. I already had a fix in development, but I had to cancel all my changes to merge yours in. Or at least communicate that with PR author. Thanks. |
Sure no - problem. Sorry for messing you up. |
@jamestalmage Good suggestion about errors! |
ea15d7c
to
98e1af2
Compare
78b4376
to
9d38c8f
Compare
9d38c8f
to
63395f0
Compare
I'd really like to merge this now and add TAP support for the next release. I could add/convert tests in a separate PR. @sindresorhus @jamestalmage what do you think? |
I would wait for the release. We already have one major refactor in this release. I think that we let major changes sit on master for at least a few days before release. |
I agree. Not really any user-facing changes here anyways. |
Landed! Exactly what I had in mind when I opened the issue about it. Really nice work @vdemedes :) |
I need to step up my GIF game. |
}) | ||
.then(flatten) | ||
.filter(function (file) { | ||
return path.extname(file) === '.js' && path.basename(file)[0] !== '_'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do .jsx test files run?
@vdemedes
Debugged to find this, but unfortunately/luckily this file has been refactored by @jamestalmage
Hoping that its fixed in next release or minor release to patch this is released soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok the code has been moved to https://github.com/avajs/ava/blob/e01ee00/lib/ava-files.js#L248
Will see if I can account extension from files or source and raise a PR
This PR adds a long-awaited programmatic API (#83).
To-do:
child_process.exec
It also opens a road for #27 (TAP support).