Hello Jest
I'd like to reflect on an example of jest test that you've recommended in official React blogpost:
import React from 'react';
import ReactDOM from 'react-dom';
import { act } from 'react-dom/test-utils';
import Counter from './Counter';
let container;
beforeEach(() => {
container = document.createElement('div');
document.body.appendChild(container);
});
afterEach(() => {
document.body.removeChild(container);
container = null;
});
it('can render and update a counter', () => {
// Test first render and effect
act(() => {
ReactDOM.render(<Counter />, container);
});
const button = container.querySelector('button');
const label = container.querySelector('p');
expect(label.textContent).toBe('You clicked 0 times');
expect(document.title).toBe('You clicked 0 times');
// Test second render and effect
act(() => {
button.dispatchEvent(new MouseEvent('click', {bubbles: true}));
});
expect(label.textContent).toBe('You clicked 1 times');
expect(document.title).toBe('You clicked 1 times');
});
An interesting thing is happening here:
container is declared outside of tests and shared between themThis might work when tests are synchronous. This might even work if tests are asynchronous because Jest is somehow smart enough to prevent out of order beforeEach and afterEach calls.
But let's face it, it's ugly.
Wouldn't it be better if Jest passed to before hooks and currently tested example the same object that is instantiated from scratch for each individual test? Then your example would look as so:
import React from 'react';
import ReactDOM from 'react-dom';
import { act } from 'react-dom/test-utils';
import Counter from './Counter';
beforeEach(context => {
context.container = document.createElement('div');
document.body.appendChild(context.container);
});
afterEach(({ container }) => {
document.body.removeChild(container);
});
it('can render and update a counter', ({ container }) => {
// Test first render and effect
act(() => {
ReactDOM.render(<Counter />, container);
});
const button = container.querySelector('button');
const label = container.querySelector('p');
expect(label.textContent).toBe('You clicked 0 times');
expect(document.title).toBe('You clicked 0 times');
// Test second render and effect
act(() => {
button.dispatchEvent(new MouseEvent('click', {bubbles: true}));
});
expect(label.textContent).toBe('You clicked 1 times');
expect(document.title).toBe('You clicked 1 times');
});
See? We don't need to declare global container variable and because context is created from scratch for each test we also don't need container = null; in afterEach.
This can be totally compatible with done, because Jest can detect that beforeEach or afterEach accept an object and enable this mode only then.
If someone still needed callback-style done-like behavior, but with context, then one could use expect.assertions expect.pass or expect.fail instead (or return new Promise((resolve, reject) => { ... })).
Thank you
Another possibility is to enable this mode when beforeEach returns a context:
beforeEach(() => {
const container = document.createElement('div');
document.body.appendChild(container);
return { container }
});
Thanks for the detailed description!
Wouldn't it be better if Jest passed to before hooks and currently tested example the same object that is instantiated from scratch for each individual test?
Yeah, we've talked about this some privately - some way of mimicking e.g. Ava's API (with a context passed to every test, and also ditching globals). This missing feature is also one of the blockers for more work on test.concurrent. We don't currently have any concrete plans - only step we've taken is adding an ESLint rule that will transform functions that takes a done callback to return a promise instead. The intention is to move people off of done callbacks so that we can free up the callback argument for a future version of Jest to stick a context there.
This can be totally compatible with
done, because Jest can detect that beforeEach or afterEach accept an object and enable this mode only then.
This is interesting - how would we detect it? We currently do testFn.length to check how many arguments are accepted by the test function. If we can detect a non-function, that'll unblock us.
This might work when tests are synchronous. This might even work if tests are asynchronous because Jest is somehow smart enough to prevent out of order beforeEach and afterEach calls.
All tests complete in the declaration order unless you use test.concurrent. So it'll work fine sync or async.
Tl;dr: We do want to provide some sort of context object, but we have no concrete plans or roadmap on how to get there.
/cc @rubennorte who might have more thoughts on this
This is interesting - how would we detect it? We currently do
testFn.lengthto check how many arguments are accepted by the test function. If we can detect a non-function, that'll unblock us.
Normally there are no arguments passed to beforeEach and afterEach, so you can check whether beforeEach.length > 0, and in such case enable passing context to functions.
It's not possible to perform such check on test because it already can have more than one argument and it's possible, but computationally hard, to detect whether passed argument is used as context or as done callback (you need parse and analyse syntax, and check for done() call).
I think the best way is to deprecate done and introduce context, where enabling context can happen by either accepting context to beforeEach (without beforeEach context doesn't make any sense), or when a context is returned from beforeEach.
Normally there are no arguments passed to beforeEach and afterEach, so you can check whether
beforeEach.length > 0, and in such case enable passing context to functions.
Lifecycle hooks also supports a done callback, same as test.
Then I guess we're left with enabling this feature when beforeEach returns context
I like this more anyway because it avoids mutating context in beforeEach, instead you return it in more "functional" way
I think it'd make more sense to have people specifying some config option rather than dynamically figuring it out at runtime. Or maybe setting up some codemod (like the eslint plugin), and saying that in version x, done is removed so we don't have to support both ways of doing it
I agree with config option + codemod that rewrites done() calls to something better
btw. I'd be in favor of rewriting:
test('something', done => {
setTimeout(() => {
Math.random() < 0.5 ? done() : done("Error")
), 1000);
})
to something like:
test('something', () => {
const done = expect.assertions(1)
setTimeout(() => {
Math.random() < 0.5 ? done() : done('Error')
), 1000);
})
Because it means minimal changes are necessary (just remove done from argument, and add one line at the top of the test)
IMO, that should ideally be transformed to:
test('something', () =>
new Promise((resolve, reject) => {
setTimeout(() => {
Math.random() < 0.5 ? resolve() : reject('Error')
}, 1000)
}))
All asynchronicity should be done via promises - Jest shouldn't provide its own callbacks for handling async (passed into the test function, or created via some other mechanism).
The ESLint plugin doesn't handle the failing case actually (totally slipped my mind when implementing it), so I opened up an issue for it: https://github.com/jest-community/eslint-plugin-jest/issues/223
Just one last comment from me: maybe it would be good idea to add eslint-plugin-jest rule disallow mutation of global variables in hooks if test.concurrent or it.concurrent is used in given file, so for example:
let container = null
beforeEach(() => {
container = document.createElement('div');
document.body.appendChild(container);
});
afterEach(() => {
document.body.removeChild(container);
container = null;
});
it.concurrent('can render and update a counter', () => {
// ...
})
would complain at first line of beforeEach and last line of afterEach (but only if context passing is enabled).
Yeah, that makes sense to me - not sure how to statically discover what tests are safe to do and what they're not, though
I've received feedback from many people that want to keep done, so we should provide an equivalent solution if we use the context object.
We could do something like this:
beforeEach(t => {
t.context.user = createUser(); // could also do t.ctx for short
});
it('should return the user name', t => {
expect(getUserName(t.context.user)).toBe(t.context.user.name);
});
it('should return the user name async', async t => {
await expect(getUserName(t.context.user)).resolves.toBe(t.context.user.name);
});
it('should return the user name async 2', t => {
getUserName(t.context.user, username => {
expect(username).toBe(t.context.user.name);
t.done();
});
return t.awaitDone();
});
As a point of reference, ava requires you to opt in to callbacks by supplying a modifier to the test definition: https://github.com/avajs/ava/blob/65133a85ecbc881f9610df78feffd498474bcc8c/docs/01-writing-tests.md#callback-support
test.cb('data.txt can be read', t => {
// `t.end` automatically checks for error as first argument
fs.readFile('data.txt', t.end);
});
I also don't like this example:
it('should return the user name async 2', t => {
getUserName(t.context.user, username => {
expect(username).toBe(t.context.user.name);
t.done();
});
return t.awaitDone();
});
It will time out since expect throws, swallowing the error. I guess if we do t.expect(username)... that'll work
I've received feedback from many people that want to keep
done, so we should provide an equivalent solution if we use the context object.
I think const done = expect.assertions(1) is better than providing any methods on context. I think context ought to be just plain and empty object.
Also doing something like t.context.user, prevents you to use short destructuring syntax I've shown at the beginning:
afterEach(({ container }) => {
document.body.removeChild(container);
});
Instead you'd need to write
afterEach(({ context: { container } }) => {
document.body.removeChild(container);
});
it.cb could be better but it sill requires adding some default fields to context.. like t.end. I don't like introducing such "reserved words" on context.
So my vote is to recommend for done-likers:
test('something', () => {
const done = expect.assertions(1)
setTimeout(() => {
Math.random() < 0.5 ? done() : done('Error')
), 1000);
})
@SimenB my bad, I wrote that too fast 馃槄
@sheerun I didn't mean to include methods in the context but having the context as a separate field in the object. The context would indeed be an empty object. I like you proposal better :)
Maybe I'm missing something, but why not just pass context as a second parameter to it and test? It would default to {} and if you return a different object from beforeEach then that would be used instead, and it would also be passed as the first (only?) parameter to afterEach. Then everyone still gets to keep done as the first parameter to it and test and can opt-in to the context param by just declaring it in the parameter list after done. Users who don't need done can just put _ there to silence linters.
To clarify by adjusting your examples:
beforeEach(() => {
const container = document.createElement('div');
document.body.appendChild(container);
return { container }
});
afterEach(({ container }) => {
document.body.removeChild(container);
});
it('can render and update a counter', (done /* or _ */, { container }) => {
// Test first render and effect
act(() => {
ReactDOM.render(<Counter />, container);
});
const button = container.querySelector('button');
const label = container.querySelector('p');
expect(label.textContent).toBe('You clicked 0 times');
expect(document.title).toBe('You clicked 0 times');
// Test second render and effect
act(() => {
button.dispatchEvent(new MouseEvent('click', {bubbles: true}));
});
expect(label.textContent).toBe('You clicked 1 times');
expect(document.title).toBe('You clicked 1 times');
});
@sdegutis done in its current form is supposed to go away anyway, after that happens we could implement something like this properly right away.
Also we use the function arity to determine if we should wait for the user to call done to finish the test. If we pass it unconditionally there's no way to tell (unless we use a different method)
Ok thanks for the explanation never mind.
Thanks a lot for the link between related issues.
I think, this very important feature to write cleaner tests.
Just a quick thought: Mocha passes context as this. i.e.:
beforeEach(function() {
const container = document.createElement('div');
document.body.appendChild(container);
this.container = container;
});
afterEach(function() {
document.body.removeChild(this.container);
});
it('can render and update a counter', function() {
// Test first render and effect
act(() => {
ReactDOM.render(<Counter />, this.container);
});
/* ... etc ... */
});
This would not clash with done.
Downside is test functions must be full function() {}, not arrow functions, so as to receive this. But obviously if you don't want to use context, you can use arrow functions, so it wouldn't require any changes to existing tests.
Until a permanent solution is found: I found an article's approach to context with Jest that works well, with a slight tweak. Can pass variables between before/tests/after and even across files as well. I put a comment about it here, in case no one finds it since the issue is closed.
https://github.com/facebook/jest/issues/4903#issuecomment-487335531
Also, it would be great if we can access the overall title of the test:
describe('a', () => {
describe('b', () => {
test('c', ({ title }) => {
console.log(title) // 'a > b > c'
}
}
}
I am looking for a way to get a unique id of each test, and getting the title of the test would be a great way to do it.
@unional this is sort of unrelated, but fyi the test title is not suitable as a uid:

@jeysal thanks. Fully aware of that. In my use case that is acceptable. Users are not suppose to do that and I can detect and emit a warning.
Ok while we don't have a solution I did a small wrapper for describe(), it works like that, what any of the lifecycle returns it will passed as the context, usage on the repository, or what you pass on the initial, also works with typescript.
Something like that could be used as a describe.context() also with some hacks I can make test and beforeEach with context available during execution without the need for destructuring.
import { withContext } from '../src';
withContext('before each', ({ test, beforeEach }) => {
beforeEach((state = { foo: 0 }) => {
return { foo: state.foo + 1 };
});
test('foo 1', ({ foo }) => {
expect(foo).toBe(1);
});
test('foo 2', ({ foo }) => {
expect(foo).toBe(2);
});
});
May I suggest a working example from Elixir:
https://hexdocs.pm/ex_unit/ExUnit.Callbacks.html#module-context
Most helpful comment
Just a quick thought: Mocha passes context as
this. i.e.:This would not clash with
done.Downside is test functions must be full
function() {}, not arrow functions, so as to receivethis. But obviously if you don't want to use context, you can use arrow functions, so it wouldn't require any changes to existing tests.