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

[Bug]: document is null during microtasks at the end of a test #12670

Closed
eps1lon opened this issue Apr 13, 2022 · 21 comments
Closed

[Bug]: document is null during microtasks at the end of a test #12670

eps1lon opened this issue Apr 13, 2022 · 21 comments

Comments

@eps1lon
Copy link
Contributor

eps1lon commented Apr 13, 2022

Version

27.5.1

Steps to reproduce

  1. clone https://github.com/eps1lon/react-testing-library-error-repro/tree/jest-repro
  2. checkout jest-repro branch
  3. yarn install
  4. yarn test

Expected behavior

Test fails due to timeout.

Actual behavior

document is null during a microtask scheduled from a ReactDOMTestUtils.act call.

Additional context

Continued from #9056 (comment) which is a reduced repro of testing-library/react-testing-library#1027

Environment

System:
    OS: Linux 5.13 Ubuntu 20.04.4 LTS (Focal Fossa)
    CPU: (16) x64 Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz
  Binaries:
    Node: 14.18.3 - ~/.nvm/versions/node/v14.18.3/bin/node
    Yarn: 1.22.17 - ~/.nvm/versions/node/v14.18.3/bin/yarn
    npm: 6.14.15 - ~/.nvm/versions/node/v14.18.3/bin/npm
  npmPackages:
    jest: ^27.5.1 => 27.5.1
@thebrianbug
Copy link

This error is difficult for non-react savvy folks to understand. It would be great to fix the issue and add more error messaging to direct folks on what to do about it a bit more clearly.

@github-actions
Copy link

github-actions bot commented Jun 4, 2022

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Jun 4, 2022
@eps1lon
Copy link
Contributor Author

eps1lon commented Jun 4, 2022

comment

@github-actions github-actions bot removed the Stale label Jun 4, 2022
@edishu
Copy link

edishu commented Jun 21, 2022

As discussed in testing-library/react-testing-library#1027. This issue is experienced very often while running async tests on Github Actions CI. As a mitigation I add jest.runAllTimers() at the end of test to exhaust all micro/macro task queues, but it rarely works.

Also, I have tests which run successfully on local machine but fail only in Github Actions CI with:

      var evt = document.createEvent('Event');

@johachi
Copy link

johachi commented Jun 22, 2022

I have been able to consistently reproduce this error on my local machine. But to do that I have to do yarn test --runInBand while also putting my machine under a heavy workload (e.g. playing a few youtube videos on the highest resolution or screen share). Still no luck in finding the culprit. 😭

@Zylphrex
Copy link

I ended up patching react-dom in my node_modules locally and added some debugging code to identify the problematic test. You can see it at https://github.com/Zylphrex/react-testing-library-error-repro. It's a little gross but I hope it helps.

@johachi
Copy link

johachi commented Jun 23, 2022

I ended up patching react-dom in my node_modules locally and added some debugging code to identify the problematic test. You can see it at https://github.com/Zylphrex/react-testing-library-error-repro. It's a little gross but I hope it helps.

This worked excellently well! 🎉 Thank you so much @Zylphrex !

It did point me to the test and place I thought was probably the culprit. I still can't figure out why it is failing though. But I can solve the problem by adding a longer timeout for the test that is failing but it is not a solution I'm satisfied with.

@mjchang
Copy link

mjchang commented Jun 23, 2022

After trying random things to solve this issue, I discovered that increasing test timeout (jest.setTimeout, 30000) or test('title', () => {...}, 30000)) fixed it for me. Disclaimer - I don't fully understand how this helped and whether this is reliable. Would appreciate an explanation if this is the proper workaround 🙏

@Zylphrex
Copy link

@mjchang If you were facing the same problem, that should fix it.

The root cause of this problem when I initially opened up testing-library/react-testing-library#1027 was that the test timed out during one of the getBy* queries. Your choices here are to increase the timeouts or make your test run faster.

I believe what's happening here is because the test had timed out, jest started to do tear downs (I'm not sure exactly what). And at some point after, the getBy* tries to poll for results again and encounters this particular error which is not helpful in determining what the actual problem was.

@drewlustro
Copy link

I experienced this one today. Error is obtuse and stack trace is unhelpful, especially within a large team.

Some days I fight with Jest, some days I let Jest win.

I just gave up and deleted the spec.

Problem solved ;)

@ramonaspence
Copy link

ramonaspence commented Aug 1, 2022

I believe I'm having this same issue, only the test that consistently triggers this error is empty. With or without code in the test case it gives me this error, but as soon as I comment out the entire test case, my other tests pass without an issue.

My Jest version is 24.9.0
Node version is 18.6
I've also tried using node version 16.15.0 which made no difference that I can see.

My test file:

import React from 'react';
import { render, screen, fireEvent } from '@testing-library/react';
import '@testing-library/jest-dom'
import userEvent from '@testing-library/user-event';
import axios from 'axios';

import Login from '../Login.js';


describe('Login component', () => {
    const mockSubmit = jest.fn();
    jest.mock('axios');

    beforeEach(() => {
        mockSubmit.mockClear();
        render(<Login handleLogin={mockSubmit} />);
        const onSubmit = new mockSubmit;
    });

    test('inputing username updates state', () => {

        fireEvent.change(getUsername(), {target: {value: 'Text'}});
        expect(screen.getByDisplayValue('Text')).toBeInTheDocument();
    });

    test('inputing password updates state', () => {

        fireEvent.change(getPassword(), {target: {value: 'password'}});
        expect(screen.getByDisplayValue('password')).toBeInTheDocument();
    });

    test('handleLogin is called when form submits', () => {

        enterFormData();
        fireEvent.submit(screen.getByTitle('login_form'));
        expect(mockSubmit).toHaveBeenCalledTimes(1);
    });

    test('handleLogin is called when submit button clicked', () => {

        enterFormData();
        fireEvent.click(screen.getByRole('button'));
        expect(mockSubmit).toHaveBeenCalledTimes(1);
        expect(mockSubmit).toBeCalled();
        expect(mockSubmit).toReturnWith();
    });

    // test('axios post request', () => {


    // });
});

function getUsername() {
    return screen.getByPlaceholderText(/username/);
}

function getPassword() {
    return screen.getByPlaceholderText(/password/);
}

function enterFormData() {
    fireEvent.change(getUsername(), {target: {value: 'Text'}});
    fireEvent.change(getPassword(), {target: {value: 'password'}});
}

And here is a gist with the stack trace and the component that I'm testing.

When the 5th and last test is commented out (like above) my tests run. But if I do not comment it out, I get the error. The test did have code in it but removing it did not make a difference.

I'm hoping someone could point me in the direction of a work around, as I'm completely stumped.

@Zylphrex
Copy link

Zylphrex commented Aug 2, 2022

@ramonaspence Your error looks slightly different to what I've seen but something you can try is to comment out the test that passed just before the failure (which is probably the second last test in the test suite). If this is the same problem/something similar, the previous test that passed may be leaving some asynchronous work that runs after it passes.

@ramonaspence
Copy link

Okay, thank you! I didn't consider that what I'm testing has a promise in it.

@HansKre
Copy link

HansKre commented Aug 24, 2022

It seems that the reason why this is rarely happening locally but often while running async tests on Github Actions CI is that those failing tests (for whatever various reasons) take up quite some time to complete and ultimately running into a timeout especially on smaller CPU machines / environments.

Of course one would expect those tests to fail with a timeout. What seems to happen instead though is that said tests are getting cleared and after clearing, document-object becomes unavailable. That is the error that we are getting. Totally confusing. That's the reason why increasing timeout for those tests avoids this issue.

@SimenB
Copy link
Member

SimenB commented Aug 25, 2022

Is it possible to get a reproduction without React where this is an issue? I think the underlying problem isn't jest doing something weird (we cannot inspect promise states, so we don't know to wait. And even if we could - how long are we supposed to wait?), but rather React scheduling callbacks in a way that breaks stack traces leading to very confusing errors.

React throws an error, but the trace is useless - if React kept the frames from when the callback was scheduled I assume at least the error would be usable?

https://github.com/facebook/react/blob/d0f396651bd654a9c2058d9c779aab843651ca9a/packages/shared/invokeGuardedCallbackImpl.js#L80-L94


Of course, Jest can add a wait before nulling out document to allow microtasks to run, but that doesn't help for timeouts, only in the very specific case where only a promise is what we're waiting for in the exact moment a test times out

@SimenB
Copy link
Member

SimenB commented Aug 25, 2022

I see I pretty much just repeated what @Zylphrex said in #12670 (comment). IMO React should do that by default (or, not that as it's quite hacky 😅 but something that keeps a usable stack).

Again, I agree the error from Jest is unfortunate in this case, but I'm not sure there's anything we can really do (while React definitely can do something)

@eps1lon
Copy link
Contributor Author

eps1lon commented Aug 25, 2022

The general problem is that Jest is the only environment where document === null. Everywhere else it's either defined or typeof document === 'undefined' (or at least just undefined). We can fix it in React (it already includes Jest specific document checks). But we'll always play catch-up because implementors need to be aware that document can be null.

@SimenB
Copy link
Member

SimenB commented Aug 26, 2022

I guess you can say the underlying issue is something like jsdom/jsdom#955. We call window.close before document is set to null, but while timers gets cancelled it doesn't seem like microtasks does.

I doubt there's anything that we can do in Jest, unless you have any suggestions?

@zxsure
Copy link

zxsure commented Apr 26, 2023

is there any WA for this issue?

@SimenB
Copy link
Member

SimenB commented Aug 13, 2023

As of #13972 (released in 29.5.0), we no longer set the document to null. So this shouldn't be an issue anymore.

If it is, please open up a new issue.

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests