Skip to content
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

afterEach and beforeEach not running? #560

Closed
kentcdodds opened this issue Feb 17, 2016 · 18 comments
Closed

afterEach and beforeEach not running? #560

kentcdodds opened this issue Feb 17, 2016 · 18 comments

Comments

@kentcdodds
Copy link
Contributor

I've reproduced this bug in this repo. From the README there:

The test directory has a bug.js and a workaround.js.

If you run: npm install and then npm t you'll get:

~/Developer/ava-beforeEach-afterEach-bug (master)
⛄  $ npm run test -s

  ✔ workaround › first log
  ✔ workaround › second log
  ✔ bug › first log
  ✖ bug › second log   
  t.true(console.log.calledOnce)
                     |          
                     false      


  1 test failed

  1. second log
  AssertionError: false === true
    Test.fn (test/bug.js:16:3)


For some reason, it appears that perhaps the cleanup function is not run between the two test runs.

It's using the latest version of AVA. Specifically, I spotted this here. Let me know if you need more info!

@SamVerschueren
Copy link
Contributor

What's the callCount of console.log?

@SamVerschueren
Copy link
Contributor

Think I found the issue. npm test -s does not run your tests in serial, which is why it doesn't work. Update your npm script to "test": "ava -s" and try again.

You can also make use of the context object to store your original log.

import test from 'ava';
import sinon from 'sinon';

test.beforeEach(t => {
    t.context.log = console.log;

    console.log = sinon.spy();
});

test.afterEach(t => {
    console.log = t.context.log;
});

test('first', t => {
    console.log('first');
    t.true(console.log.calledOnce);
});

test('second', t => {
    console.log('second');
    t.true(console.log.calledOnce);
});

@sindresorhus
Copy link
Member

Yes, from the AVA readme:

When using npm test, you can pass positional arguments directly npm test test2.js, but flags needs to be passed like npm test -- --verbose.

So it has to be npm test -- -s (Really wish npm test would just pass them through... Anyone wanna open a npm ticket about that?)

@SamVerschueren
Copy link
Contributor

@sindresorhus Cool, didn't know it was possible. Thanks :).

@kentcdodds
Copy link
Contributor Author

Actually I'm not trying to pass the -s flag to AVA. I'm using it to suppress npm run output. I want aware that using beforeEach and afterEach require that tests run serially. I guess that makes sense, but I can't think of a use case where including those would be useful when running in parallel... Is there a better way to do what I'm trying to do?

@SamVerschueren
Copy link
Contributor

@kentcdodds They don't require that tests run serially. It's because console.log is global, so shared between the two tests. This means that console.log is referring to the same spy. So in this situation, it's necessary to run them serially. If you are able (which is not the case here) to use the context object, you don't have to run them serially because it's a totally new object.

import test from 'ava';
import sinon from 'sinon';

test.beforeEach(t => {
    t.context.log = sinon.spy();
});

test.afterEach(t => {
    t.context.log.restore();
});

test('first', t => {
    t.context.log('first');
    t.true(t.context.log.calledOnce);
});

test('second', t => {
    t.context.log('second');
    t.true(t.context.log.calledOnce);
});

@sindresorhus
Copy link
Member

@SamVerschueren On a side note, the above would be great as a recipe. Hint hint ;)

@kentcdodds
Copy link
Contributor Author

That won't work for my actual use-case. And it doesn't make sense to me that the tests are using the same spy because the spy should be reset before each test.

@sindresorhus
Copy link
Member

@kentcdodds Can you elaborate on your use-case? They're not using the same spy. The hook is run before each test and with a unique t.context for each test.

@SamVerschueren
Copy link
Contributor

And it doesn't make sense to me that the tests are using the same spy because the spy should be reset before each test.

They are, but AVA is just too fast :).

Look at the execution flow

beforeEach
beforeEach
first
second
afterEach

Note: Not sure though why the second afterEach isn't called. Bug? @sindresorhus

As you see, the console.log will be the same for first and second, being the one created in the second call to beforeEach. So yes, they are shared :).

@SamVerschueren
Copy link
Contributor

@sindresorhus He was pointing to the use case without the context object

On a side note, the above would be great as a recipe. Hint hint ;)

Will do my best to not forget. Should create a repository to keep track of my todos :).

@SamVerschueren
Copy link
Contributor

Shared variable

import test from 'ava';

let i = 0;

test.beforeEach(t => {
    i++;
});

test.afterEach(t => {
    i--;
});

test('first', t => {
    console.log(i);
});

test('second', t => {
    console.log(i);
});

Output:

2
2

Context variable

import test from 'ava';

test.beforeEach(t => {
    t.context.i = (t.context.i || 0) + 1;
});

test.afterEach(t => {
    t.context.i--;
});

test('first', t => {
    console.log(t.context.i);
});

test('second', t => {
    console.log(t.context.i);
});

Output:

1
1

@kentcdodds
Copy link
Contributor Author

They are, but AVA is just too fast :).

Ah, I see. That appears to be a bug to me.

He was pointing to the use case without the context object

Correct. In my actual use case, my tests are asserting that something else is calling console.log

@SamVerschueren
Copy link
Contributor

Ah, I see. That appears to be a bug to me.

It's not a bug, running tests concurrently comes with a "cost", this is one of them. Take a look at the number example (2 posts above this one) again. It shows the behaviour you experience.

This is not something you can have in mocha (or others) because they are running tests serially and where the execution flow is easier to follow.

@SamVerschueren
Copy link
Contributor

In my actual use case, my tests are asserting that something else is calling console.log

What I do if I have a similar use case is only running that particular test serially.

import test from 'ava';
import fn from './';

// Do spy stuff here

test.serial('test log', t => {
     fn.logger('hello world');
     t.true(console.log.calledOnce);
});

test('test something else', t => {
     t.true(fn.somethingElse());
});

@kentcdodds
Copy link
Contributor Author

Ah, I think I understand now. This is part of how AVA helps (read: forces) you isolate your tests which is good. I'll submit a PR which mentions this.

@SamVerschueren
Copy link
Contributor

Exactly. From the moment you have something shared (which might not seem to be shared in the first place), you will have to execute them serially.

@karlhorky
Copy link

@kentcdodds I believe this is the same problem that I was having with the shared ngMocks state in Angular tests.

The workaround I came up with was to use after() instead of afterEach(). I've opened kentcdodds/ava-beforeEach-afterEach-bug#1 as an issue in your repo.

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

No branches or pull requests

4 participants