The Wayback Machine - https://web.archive.org/web/20201024040035/https://github.com/facebook/react/issues/19371
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

Setup CI infra to run DevTools tests against multiple React versions #19371

Open
bvaughn opened this issue Jul 15, 2020 · 7 comments · May be fixed by #19912
Open

Setup CI infra to run DevTools tests against multiple React versions #19371

bvaughn opened this issue Jul 15, 2020 · 7 comments · May be fixed by #19912

Comments

@bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Jul 15, 2020

PR #19108 caused some Suspense-related DevTools regressions (more info available on #19368) which we did not catch because of the fact that DevTools tests are only run against the version of React in master.

We should follow the precedent of the regression fixtures tests and have CI run DevTools tests against multiple React versions, including v15, all v16 minors, and the current HEAD of master branch.

Setting this up will involve several things:

  • Infra to checkout older React packages and run tests against them.
  • Some form of gating so that we can account for expected differences in Store snapshots between React versions.
  • Some form of gating so that we can avoid running tests against invalid combinations of features and versions (e.g. don't test for Suspense in a version of React that didn't include that component yet).
@RitikPandey1
Copy link

@RitikPandey1 RitikPandey1 commented Sep 19, 2020

I would like to take this as my first issue , please guide me from where I can start

@bvaughn
Copy link
Contributor Author

@bvaughn bvaughn commented Sep 21, 2020

@RitikPandey1 You are welcome to work on this issue if you would be interested!

Unfortunately, it would have to be self-guided work. I don't have any more of an outline for what needs to be done than what is written in the issue.

@RitikPandey1
Copy link

@RitikPandey1 RitikPandey1 commented Sep 21, 2020

Okay, @bvaughn I will try to figure this out .

@MoSattler
Copy link

@MoSattler MoSattler commented Sep 26, 2020

@bvaughn, not sure if this is easily done with checking out older code.
I tried it here, and I keep running into issues where code is either coupled to newer dependencies, or the newer tests are coupled to the newer versions of the code to be tested. See test results here.

Another issue is, that older checkouts have too restrictive devEngines requirements to be installed with modern node, see this.

@MoSattler
Copy link

@MoSattler MoSattler commented Sep 26, 2020

Also tried it by just installing the old dependencies via npm, with similar results. Maybe some of the tests are testing internals, and hence are coupled to internals of the latest code?

@bvaughn
Copy link
Contributor Author

@bvaughn bvaughn commented Sep 28, 2020

The build-for-devtools command builds an experimental build (RELEASE_CHANNEL=experimental) of React to be bundled along with the DevTools extension. Generally this build always comes from the master branch, so DevTools is built with the latest version of React.

This task is about using the DevTools to connect to applications written with multiple (usually older) versions of React. Replacing the build-for-devtools command doesn't seem like the right approach here, since that would also be changing which version of React the DevTools is actually running with.

The precedent here would be our "legacy" tests:
https://github.com/facebook/react/tree/master/packages/react-devtools-shared/src/__tests__/legacy

These tests using NPM aliasing to run an older version of React with DevTools (which is still built with the latest experimental build):

// Redirect all React/ReactDOM requires to the v15 UMD.
// We use the UMD because Jest doesn't enable us to mock deep imports (e.g. "react/lib/Something").
jest.mock('react', () => jest.requireActual('react-15/dist/react.js'));
jest.mock('react-dom', () =>
jest.requireActual('react-dom-15/dist/react-dom.js'),
);
React = require('react');
ReactDOM = require('react-dom');
@MoSattler
Copy link

@MoSattler MoSattler commented Sep 30, 2020

Thanks for the elaboration, I will check it out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.