facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.08k stars 46.86k forks source link

Flaky tests on node version 10 #13907

Closed alexandraleah closed 6 years ago

alexandraleah commented 6 years ago

When running yarn test on node v10.5.0, the following 3 tests fail:

On node v8.12.0 and node v9.11.2 all tests passed

Below is the output for these 3 tests:

Summary of all failing tests
 FAIL  packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js
  ● ReactDebugFiberPerf › supports Suspense and lazy

    An update was suspended, but no placeholder UI was provided.

      312 |     } while (workInProgress !== null);
      313 |     // No boundary was found. Fallthrough to error mode.
    > 314 |     value = new Error(
          |             ^
      315 |       'An update was suspended, but no placeholder UI was provided.',
      316 |     );
      317 |   }

      at throwException (packages/react-reconciler/src/ReactFiberUnwindWork.js:314:13)
      at renderRoot (packages/react-reconciler/src/ReactFiberScheduler.js:1263:52)
      at performWorkOnRoot (packages/react-reconciler/src/ReactFiberScheduler.js:2262:7)
      at performWork (packages/react-reconciler/src/ReactFiberScheduler.js:2139:7)
      at performAsyncWork (packages/react-reconciler/src/ReactFiberScheduler.js:2108:3)
      at flushUnitsOfWork (packages/react-noop-renderer/src/createReactNoop.js:523:7)
          at flushUnitsOfWork.next (<anonymous>)
      at Object.flushUnitsOfWork (packages/react-noop-renderer/src/createReactNoop.js:739:252)
      at Object.flush (packages/react-noop-renderer/src/createReactNoop.js:726:24)
      at Object.<anonymous> (packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js:612:15)

 FAIL  packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
  ● ReactLazy › supports class and forwardRef components

    expect(received).toEqual(expected)

    Expected value to equal:
      ["Foo", "forwardRef", "Bar"]
    Received:
      ["Foo", "Loading..."]

    Difference:

    - Expected
    + Received

      Array [
        "Foo",
    -   "forwardRef",
    -   "Bar",
    +   "Loading...",
      ]

      351 |     await Promise.resolve();
      352 |
    > 353 |     expect(root).toFlushAndYield(['Foo', 'forwardRef', 'Bar']);
          |                  ^
      354 |     expect(root).toMatchRenderedOutput('FooBar');
      355 |     expect(ref.current).not.toBe(null);
      356 |   });

      at Object.<anonymous> (packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js:353:18)
      at step (packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js:1:1609)
      at packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js:1:1769

 FAIL  packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js
  ● ReactDOMServerHydration › should be able to use lazy components after hydrating

    expect(received).toBe(expected) // Object.is equality

    Expected: "Hello world"
    Received: "Hello loading"

      493 |     jest.runAllTimers();
      494 |     await Lazy;
    > 495 |     expect(element.textContent).toBe('Hello world');
          |                                 ^
      496 |   });
      497 | });
      498 |

      at Object.<anonymous> (packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js:495:33)
      at step (packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js:10:714)
      at packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js:10:874

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React? React version 16.6.0-alpha.8af6728 Node v10.5.0 Mac OS 10.13.6 Also reproduced on Ubuntu 18.04 (bionic)

Note: I had an error when attempting to run yarn build based on an unrelated issue.

gaearon commented 6 years ago

Yeah. Can you try to figure out why? It started happening recently.

gaearon commented 6 years ago

My guess is that something's going on with await calls in those tests. I know they don't await a Promise so maybe that's the reason? cc @acdlite

nomadtechie commented 6 years ago

hi @gaearon & @acdlite hello from Boston! We hosting and mentoring a HactoberFest in Boston and we just filed this bug. A special message from us -- thanks for hopping on this so quickly- we will investigate:

img_20181020_132709

acdlite commented 6 years ago

This should fix it (I tested in Node 10 and 8): https://github.com/facebook/react/commit/b753f76a74644dd19e7a29f8aa4e8c759190f9ef

As a follow-up we should rewrite those tests to be less flaky, like using timers instead of a microtask.

acdlite commented 6 years ago

I'm guessing that there was some change to async/await and microtasks in Node 10.

acdlite commented 6 years ago

Might be related to this: https://github.com/nodejs/node/issues/22257