Skip to content

Before/after hooks #36

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

Closed
wants to merge 8 commits into from

Conversation

vadimdemedes
Copy link
Contributor

Fixes #4.

Hooks are completely the same as regular tests and provide the same functionality/API.
The only thing different is when they are executed.

Changes

  • Add test.before() to execute a test before all others
  • Add test.after() to execute a test after all others
  • Refactor Runner.prototype.concurrent and Runner.prototype.serial to accept array of tests, instead of getting them directly from this.tests.concurrent and this.tests.serial properties respectively
  • Add tests
  • Add information about before/after hooks to the readme

Behavior

After this change, here is how Ava runs tests:

  1. before hooks
  2. serial tests
  3. concurrent tests
  4. after hooks

Update: If at least one before hook fails, all the following tests won't be executed.

Usage

var test = require('ava');

// shortcuts for convenience
var before = test.before;
var after = test.after;

before('before hook', function (t) {
  // this will run before everything else

  t.end();
});

after('after hook', function (t) {
  // this will run after everything else is completed

  t.end();
});

test('regular test', function (t) {
  t.end();
});

Output:

screen shot 2015-09-05 at 1 50 53 pm

### Tests

Tests were added to cover new test.before() and test.after() functions.

Let me know what you think!

vdemedes added 6 commits September 5, 2015 13:28
Hooks execute serially, before and after all tests.
…ents

Before this change, code would be duplicated a lot. For example,
before/after hooks require the same test run functionality as implemented
in Runner#serial. But because previously Runner#serial was hard-coded to
get tests from `this.tests.serial` property, it was not possible to use it
to run before/after hooks.

With this change, one can `this.serial([testA, testB])` to run tests.
Before this change, before hooks were treated just like
regular tests. This caused unexpected behavior, when tests
continued to execute, even if before hooks failed.
var before = this.tests.before;
var after = this.tests.after;

// TODO: refactor this bullshit
Copy link
Contributor

Choose a reason for hiding this comment

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

;) Might want to do that haha.

Copy link
Member

Choose a reason for hiding this comment

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

Relevant: #38

// @vdemedes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought about converting this.serial and this.concurrent to return promises, but decided postpone that. Wanted to hear feedback on a PR and thought that you might not like too much changes at once, haha

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, promisification should be a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could do that after this PR gets merged (to avoid merge conflicts in commit history), no probs

@Qix-
Copy link
Contributor

Qix- commented Sep 5, 2015

Other than the two things I saw (I'm sure @sindresorhus will see something else), it looks good to me. Seems like a clean implementation.

var before = test.before;
var after = test.after;

before (function (t) {
Copy link

Choose a reason for hiding this comment

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

before(function (t) { (spacing)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, will change ;)

@vadimdemedes
Copy link
Contributor Author

Yeah, looks good to me too, except Runner.prototype.run.

@sindresorhus
Copy link
Member

Looks superb to me. Can you fix the merge conflict? :D

@vadimdemedes
Copy link
Contributor Author

@sindresorhus Yeah, sure, already on it ;)

@sindresorhus
Copy link
Member

Excellent! Thanks again @vdemedes for this awesome contribution :)

687474703a2f2f74636c686f73742e636f6d2f36763641355a392e676966

@vadimdemedes
Copy link
Contributor Author

@sindresorhus You're welcome! I am really enjoying contributing to ava and got, incredible stuff. Switched almost all my shit to them from request and mocha.

P.S. That gif is amazing :D

@Qix-
Copy link
Contributor

Qix- commented Sep 6, 2015

Keep them coming @vdemedes :)

sindresorhus pushed a commit that referenced this pull request Sep 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants