-
Notifications
You must be signed in to change notification settings - Fork 11
Move spying inside proxy #28
base: master
Are you sure you want to change the base?
Conversation
Thanks for the fix. I'm gonna try to figure out exactly why it was failing before merging in order to lessen the risk of unintended side effects. I also want to add jobs in CI that test against jest 25 & 26. Should hopefully have some time to do so today. |
I've added jest@26 to the build pipeline and the test spass on master. Are you able to set up a reduced test case illustrating the situation when things are breaking for you. Pasting in a comment here is fine |
Here is the gist of the test case that is failing: it('should handle address input changing', () => {
const handleAddressChange = jest.fn();
const { getByLabelText } = render(
<AddressAutocomplete
name={name}
label={label}
address={''}
handleAddressChange={handleAddressChange}
{...i18nProps}
/>
);
const input = getByLabelText(label, { selector: 'input' });
const value = '123';
const url = `${TEST_URL}/actions/addressPredictions?filter[address]=${value}`;
const addressMock = fetchMock.mock(
url,
{ status: 200, body: [] },
{
method: 'GET',
}
);
fireEvent.focus(input);
fireEvent.change(input, { target: { value } });
jest.runAllTimers();
expect(addressMock).toHaveBeenCalled();
expect(handleAddressChange).toHaveBeenCalledTimes(1);
expect(handleAddressChange).toHaveBeenCalledWith({ address: value });
}); The actual component implementation just debounces the search value and then executes a redux dispatch which in turn triggers the fetch. Not really much special going on. Unless it's something weird with using the timers and delaying the fetch call. |
@wheresrhys - could it potentially be a node version issue? I'm running v14. |
Is the test running in the browser? Or in a browser-like environment (e.g. JSDom) and mocking the I'll probably need to see the failing test + all the test runner set up in order to properly debug |
These tests are running as part of a I only mentioned the sandbox because I thought it was relevant to some failing tests I had locally, but I now see that all of the test cases use the There are a couple other cases in our codebase that do not do any debouncing or timer related business, and those also have the same issue, so I don't think it's anything there like I suggested earlier. These are simpler functions that just call the browser Edit: I'm not using |
I don't really have at the moment to set up a full environment to replicate. If you could create a minimal test case repo I could clone (or are able to share an existing repo with me) that'd be a great help |
Sorry for the delay in getting back with you, I'm working through a separate upgrade issue with another library since I found a workaround for this. I should hopefully have that wrapped up today / tomorrow, then I plan on putting together a codesandbox to try and replicate the issue. |
@wheresrhys @green-arrow don't forget about this one! Based on your conversation, it sounds like y'all are close! 🤞 |
There are 3 failing tests, and it seems to be around using the sandbox mode. I'm a bit out of my depth when it gets to what's happening with the sandbox, but for some reason it seems the
fetchHandler
isn't even being called when I debug a specific test (like "exposescalls
property" inside thejest-built-ins
spec).fixes #26