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

Rule proposal: Forbid afterEach etc callback with serial #166

Closed
novemberborn opened this issue Jan 12, 2017 · 9 comments
Closed

Rule proposal: Forbid afterEach etc callback with serial #166

novemberborn opened this issue Jan 12, 2017 · 9 comments

Comments

@novemberborn
Copy link
Member

This implies the callback only applies to serial tests, which is not the case. If I'm not mistaken regular test.afterEach hooks still run serially when run after a test.serial() test.

See avajs/ava#1178.

Fail

test.serial.before(fn);
test.serial.beforeEach(fn);
test.serial.afterEach(fn);
test.serial.after(fn);
test.serial.afterEach.always(fn);
test.serial.after.always(fn);

Valid

test.before(fn);
test.beforeEach(fn);
test.afterEach(fn);
test.after(fn);
test.afterEach.always(fn);
test.after.always(fn);
@sindresorhus
Copy link
Member

It does have a purpose though:

// This would run first
test.serial.beforeEach(fn);

// Then these would run concurrently
test.beforeEach(fn);
test.beforeEach(fn);

@novemberborn
Copy link
Member Author

No, hooks run in order of registration, and for serial tests they automatically run serially:

import test from 'ava'

test.beforeEach(async () => {
  console.log('1.1', new Date())
  await new Promise(resolve => setTimeout(resolve, 1000))
  console.log('1.2', new Date())
})

test.serial.beforeEach(() => {
  console.log('2', new Date())
})

test.beforeEach(async () => {
  console.log('3.1', new Date())
  await new Promise(resolve => setTimeout(resolve, 1000))
  console.log('3.2', new Date())
})

test.beforeEach(async () => {
  console.log('4.1', new Date())
  await new Promise(resolve => setTimeout(resolve, 1000))
  console.log('4.2', new Date())
})

test.serial('5', t => console.log('5', new Date()))
test('6', t => console.log('6', new Date()))
test('7', t => console.log('7', new Date()))

Gives (note the timestamps):

$ "$(npm bin)"/ava --verbose test.js

1.1 2017-01-12T16:25:58.964Z
1.2 2017-01-12T16:25:59.970Z
2 2017-01-12T16:25:59.975Z
3.1 2017-01-12T16:25:59.976Z
3.2 2017-01-12T16:26:00.981Z
4.1 2017-01-12T16:26:00.982Z
4.2 2017-01-12T16:26:01.986Z
5 2017-01-12T16:26:01.986Z
  ✔ 5
1.1 2017-01-12T16:26:01.988Z
1.1 2017-01-12T16:26:01.989Z
1.2 2017-01-12T16:26:02.994Z
1.2 2017-01-12T16:26:02.994Z
2 2017-01-12T16:26:02.995Z
2 2017-01-12T16:26:02.995Z
3.1 2017-01-12T16:26:02.995Z
3.1 2017-01-12T16:26:02.995Z
3.2 2017-01-12T16:26:04.000Z
3.2 2017-01-12T16:26:04.000Z
4.1 2017-01-12T16:26:04.001Z
4.1 2017-01-12T16:26:04.001Z
4.2 2017-01-12T16:26:05.003Z
4.2 2017-01-12T16:26:05.003Z
6 2017-01-12T16:26:05.003Z
  ✔ 6
  ✔ 7
7 2017-01-12T16:26:05.004Z

  3 tests passed [16:26:05]

@sindresorhus
Copy link
Member

Oh, right. Then I think we should disallow that combination in AVA too, at runtime, with a user-friendly message.

@mightyiam
Copy link

No need for a rule for something that gets an error thrown, right?

@sindresorhus
Copy link
Member

@mightyiam There is. It's useful with an early error that can be caught in the editor even before running the tests.

@novemberborn
Copy link
Member Author

novemberborn commented Jan 13, 2017

Oh, right. Then I think we should disallow that combination in AVA too, at runtime, with a user-friendly message.

avajs/ava#1182.

@sindresorhus
Copy link
Member

@novemberborn I just tried test.serial.before(() => {}); in the latest AVA 1.0.0 beta and it doesn't throw.

@sindresorhus
Copy link
Member

@novemberborn Should we close this issue in favor of #186?

@novemberborn
Copy link
Member Author

Yup. test.serial.before() is now a serially-running before hook, and works as expected.

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

3 participants