Closed RyanAtViceSoftware closed 6 years ago
SimenB
commented
6 years ago Could you set up a reproduction showing the error? The sample is well and good, but a running example is more likely to get attention quicker 🙂 I highly doubt this is an issue in Jest, but should be able to confirm it one way or the other with a full reproduction
RyanAtViceSoftware
commented
6 years ago @SimenB I doubt I will have time to create a reproduction as you have to have a pretty complex setup to create this but if I have time I might take a swing at it.
In the meantime hopefully reporting what seems like a bug to me (definitely unexpected behavior) can be helpful in someway, if nothing else it can help other folks with the work around that I found.
SimenB
commented
6 years ago I'm not sure why you get an error, but in general you need to wait for your promises in the test. My guess is that since you don't wait, the single tick difference is what bites you.
All .then and .catch happens in the next tick, so it's always async.
RyanAtViceSoftware
commented
6 years ago That's a good guess. We uses Promise's and setTimeout in our tests to move the execution to the next tick but maybe I needed to add an extra layer for the additional nesting. I can try it real quick. I should probably write some wait utility like the one in kent dodd's framework.
RyanAtViceSoftware
commented
6 years ago That said in this case though the number of ticks would be the same for both block though right? All I did was move the nested .then to a changed .then. Maybe I'm lacking some understanding on some nuances here.
RyanAtViceSoftware
commented
6 years ago @SimenB wow how embarrassing... you were right I was needing one more tick. I've shared my code so that maybe someone else will benefit (look for comments with <=====).
Note that you can use the this library to make waiting for ticks easier: https://github.com/TheBrainFamily/wait-for-expect
Broke
import { mountApp } from '../../../test/behaviors';
import fetch from 'cross-fetch';
import {
expectNode,
expectNoErrors,
resetRedux
} from '../../../test/common.utils';
import mockFetch from '../../../test/mockFetch';
import { resetHttpRequestCache } from '../../../modules/http/httpCache';
import { navigateToRetailers } from './retailers.behaviors';
import { getInactiveDummyRetailersPlusInactives, getInactivesDummyRetailers } from "./inactiveRetailers.data";
jest.mock('cross-fetch');
describe('Given we are on the items page ', () => {
afterEach(() => {
fetch.mockReset();
resetRedux();
resetHttpRequestCache();
});
describe('and we navigate to the retailers page ', () => {
describe('and we click the add button and we fill out add retailers form ', () => {
describe('Then client side validations work properly and once the form is valid we POST correct body to /retailers ', () => {
it('', done => {
expectNoErrors(done);
return mountApp()
.then(context =>
navigateToRetailers({
...context,
dummyRetailers: getInactivesDummyRetailers
})
)
.then(selectARetailerRowAndClickEditDeactivate)
.then(({wrapper, store, calls}) => {
expect(calls.length).toBe(1);
calls.expect('/deactivateRetailersRequest', 'post');
})
.then(() => done())
.catch(done.fail);
function selectARetailerRowAndClickEditDeactivate({
wrapper,
store
}) {
const calls = [];
fetch.mockImplementation(
mockFetch(
[
{
url: '/deactivateRetailersRequest',
method: 'post',
response: () => ({ result: { retailer: [{}] } })
},
{
url: '/plusinactives',
response: getInactiveDummyRetailersPlusInactives
}
,
{
url: '/retailers',
response: () => {
const retailers = getInactivesDummyRetailers();
return {
result: {
companies: retailers.result.companies.filter(r => r.id !== '-2')
}
}
}
}
],
{ calls }
)
);
const gridContainer = wrapper.find('GridContainer');
const distributorRow = [{ id: '-2' }];
gridContainer.props().selectionChanged(distributorRow);
wrapper.update();
const expectDeactivateButtonEnabledBecauseADistributorRowSelected = () =>
expect(wrapper.find('#deactivateButton').props().disabled).toBe(
false
);
expectDeactivateButtonEnabledBecauseADistributorRowSelected();
expectNode(wrapper.find('#deactivateButton')).simulate('click');
return Promise.resolve({ wrapper, store, calls }); // <=== we need an extra tick here
}
});
});
});
});
});import { mountApp } from '../../../test/behaviors';
import fetch from 'cross-fetch';
import {
expectNode,
expectNoErrors,
resetRedux
} from '../../../test/common.utils';
import mockFetch from '../../../test/mockFetch';
import { resetHttpRequestCache } from '../../../modules/http/httpCache';
import { navigateToRetailers } from './retailers.behaviors';
import { getInactiveDummyRetailersPlusInactives, getInactivesDummyRetailers } from "./inactiveRetailers.data";
jest.mock('cross-fetch');
describe('Given we are on the items page ', () => {
afterEach(() => {
fetch.mockReset();
resetRedux();
resetHttpRequestCache();
});
describe('and we navigate to the retailers page ', () => {
describe('and we click the add button and we fill out add retailers form ', () => {
describe('Then client side validations work properly and once the form is valid we POST correct body to /retailers ', () => {
it('', done => {
expectNoErrors(done);
return mountApp()
.then(context =>
navigateToRetailers({
...context,
dummyRetailers: getInactivesDummyRetailers
})
)
.then(selectARetailerRowAndClickEditDeactivate)
.then(({wrapper, store, calls}) => {
expect(calls.length).toBe(1);
calls.expect('/deactivateRetailersRequest', 'post');
})
.then(() => done())
.catch(done.fail);
function selectARetailerRowAndClickEditDeactivate({
wrapper,
store
}) {
const calls = [];
fetch.mockImplementation(
mockFetch(
[
{
url: '/deactivateRetailersRequest',
method: 'post',
response: () => ({ result: { retailer: [{}] } })
},
{
url: '/plusinactives',
response: getInactiveDummyRetailersPlusInactives
}
,
{
url: '/retailers',
response: () => {
const retailers = getInactivesDummyRetailers();
return {
result: {
companies: retailers.result.companies.filter(r => r.id !== '-2')
}
}
}
}
],
{ calls }
)
);
const gridContainer = wrapper.find('GridContainer');
const distributorRow = [{ id: '-2' }];
gridContainer.props().selectionChanged(distributorRow);
wrapper.update();
const expectDeactivateButtonEnabledBecauseADistributorRowSelected = () =>
expect(wrapper.find('#deactivateButton').props().disabled).toBe(
false
);
expectDeactivateButtonEnabledBecauseADistributorRowSelected();
expectNode(wrapper.find('#deactivateButton')).simulate('click');
return new Promise(
resolve => setTimeout(() => resolve({ wrapper, store, calls })) // <==== Adding this moves code to next tick and fixes the issue
);
}
});
});
});
});
});If you replace Promise with e.g. https://www.npmjs.com/package/promise#usage and setImmediate you can use jest fake timers and jest.runAllImmediates to power promises synchronously.
What I would recommend is to somehow make your promise chain available from the outside or otherwise observable. Waiting arbitrary ticks instead of waiting for something concrete is pretty brittle (as you noticed here 🙂)
RyanAtViceSoftware
commented
6 years ago Those are some great tips thanks. I'm not crazy about the idea of having to modify the code under test to create seams if I don't need to and would probably prefer counting ticks.
Note that wait-for-next will loop and retry the code until it hits a timeout. That would remove the brittle factor. Up to now we've been OK with the idea that our tests are coupled to the tick count of the code under test. I think I probably was just tired yesterday...
Thanks so much for your time and tips!
github-actions[bot]
commented
3 years ago This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.
🐛 Bug Report
Nesting Promise.then() blocks causes module mock to reset
This code was causing the
cross-fetchmock we use to reset which was causing test to fail withCannot read property 'then' of undefinederror. This is because we mockfetch()(defaultcross-fetchexport) in our test to return aPromise.resolve(someData)to fake network request.Broke
However, if we flatten out the promises like below the mock doesn't reset and nothing blows up.
Works
How do I know that the mock is resetting? To validate this I added some logging code
console.log('===> about to fetch', fetch, fetch.mock && fetch.mock.calls);and the output shows the
fetch.mock.callsgetting reset to be empty after a few calls as shown below.To Reproduce
Steps to reproduce the behavior:
I put our code that causes this above. Because it's using nested promises it seems hard to create a small sample to pre
Expected behavior
Expect the mock to continue to work and not reset during test run.
Link to repl or repo (highly encouraged)
Please provide either a repl.it demo or a minimal repository on GitHub.
Issues without a reproduction link are likely to stall.
Run
npx envinfo --preset jestPaste the results here: