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

Upgrade jest from 26.6.0 to 27.0.0-next.6 #10748

Closed
wants to merge 10 commits into from

Conversation

ankurkaushal
Copy link

@ankurkaushal ankurkaushal commented Mar 25, 2021

Closes #10747. Fixes #9993.

@facebook-github-bot
Copy link

Hi @ankurkaushal!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@ankurkaushal
Copy link
Author

Looking into the CI failures. 👀

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@renatoalencar
Copy link

I guess the tests are failing because of this file https://github.com/facebook/create-react-app/blob/master/packages/react-error-overlay/src/utils/getSourceMap.js#L117 accessing the window object which doesn't exist on the node environment.

I could add the code below to this file https://github.com/facebook/create-react-app/blob/master/packages/react-error-overlay/src/__tests__/get-source-map.js in order to properly setup the correct environment.

/**
 * @jest-environment jsdom
 */

@ankurkaushal
Copy link
Author

@renatoalencar Hmm, now it worked & the test in question is passing 🤔

Looks like I was missing the upgrade in rest of the packages in 3f03a63

@ankurkaushal
Copy link
Author

@SimenB: Seeing a different error now:

TypeError: babelJest.createTransformer is not a function

  24 | })();
  25 |
> 26 | module.exports = babelJest.createTransformer({
     |                            ^
  27 |   presets: [
  28 |     [
  29 |       require.resolve('babel-preset-react-app'),

  at Object.<anonymous> (../../react-scripts/config/jest/babelTransform.js:26:28)
  at ../../../node_modules/@jest/transform/build/ScriptTransformer.js:379:73
      at Array.map (<anonymous>)

@renatoalencar
Copy link

@SimenB: Seeing a different error now:

TypeError: babelJest.createTransformer is not a function

  24 | })();
  25 |
> 26 | module.exports = babelJest.createTransformer({
     |                            ^
  27 |   presets: [
  28 |     [
  29 |       require.resolve('babel-preset-react-app'),

  at Object.<anonymous> (../../react-scripts/config/jest/babelTransform.js:26:28)
  at ../../../node_modules/@jest/transform/build/ScriptTransformer.js:379:73
      at Array.map (<anonymous>)

If we're seen errors we are having some progress. Now it looks like this file https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/config/jest/babelTransform.js is doing a require on an ES module, which export things on a default property.

@renatoalencar
Copy link

This is the import

const babelJest = require('babel-jest');

This is what is being exported

export default transformer;

If you put a default property at the end of the require call it should probably work. Like this

const babelJest = require('babel-jest').default;

@ankurkaushal
Copy link
Author

@renatoalencar That worked! Thank you catching that :)

@gaearon
Copy link
Contributor

gaearon commented Mar 25, 2021

@ankurkaushal Btw thank you for quickly jumping on this, it's really appreciated.

@ankurkaushal
Copy link
Author

@gaearon: My pleasure, always happy to help & I didn't want to miss an opportunity to collaborate with you!

I see a lot of checks are still failing, looks like something to do with fixtures? Would re-running jest with -u be okay?

@gaearon
Copy link
Contributor

gaearon commented Mar 25, 2021

What is the error?

@gaearon
Copy link
Contributor

gaearon commented Mar 25, 2021

Is this what you're seeing too?

$ react-scripts start --smoke-test
/home/vsts/work/_temp/tmp.1yGbnJ8HsL/test-app-pnp/.pnp.js:20934
    throw firstError;
    ^

Error: Package "postcss-browser-comments@3.0.0" (via "/home/vsts/work/1/yarn-cache/v6/npm-postcss-browser-comments-3.0.0-1248d2d935fb72053c8e1f61a84a57292d9f65e9-integrity/node_modules/postcss-browser-comments/index.cjs.js") is trying to require the package "browserslist" (via "browserslist") without it being listed in its dependencies (postcss, postcss-browser-comments)
    at makeError (/home/vsts/work/_temp/tmp.1yGbnJ8HsL/test-app-pnp/.pnp.js:55:17)
    at Object.resolveToUnqualified (/home/vsts/work/_temp/tmp.1yGbnJ8HsL/test-app-pnp/.pnp.js:20672:17)
    at Object.resolveRequest (/home/vsts/work/_temp/tmp.1yGbnJ8HsL/test-app-pnp/.pnp.js:20743:31)
    at Function.Module._resolveFilename (/home/vsts/work/_temp/tmp.1yGbnJ8HsL/test-app-pnp/.pnp.js:20925:30)
    at Function.Module._load (/home/vsts/work/_temp/tmp.1yGbnJ8HsL/test-app-pnp/.pnp.js:20841:31)
    at Module.require (internal/modules/cjs/loader.js:887:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at Object.<anonymous> (/home/vsts/work/1/yarn-cache/v6/npm-postcss-browser-comments-3.0.0-1248d2d935fb72053c8e1f61a84a57292d9f65e9-integrity/node_modules/postcss-browser-comments/index.cjs.js:3:36)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10) {
  code: 'UNDECLARED_DEPENDENCY',
  data: {
    request: 'browserslist',
    issuer: '/home/vsts/work/1/yarn-cache/v6/npm-postcss-browser-comments-3.0.0-1248d2d935fb72053c8e1f61a84a57292d9f65e9-integrity/node_modules/postcss-browser-comments/index.cjs.js',
    issuerLocator: { name: 'postcss-browser-comments', reference: '3.0.0' },
    dependencyName: 'browserslist',
    candidates: [ 'postcss', 'postcss-browser-comments' ]
  }
}

@renatoalencar
Copy link

@gaearon:

No actually, I ran that & it just ran the server & quit.

I was referring to https://dev.azure.com/facebook/create-react-app/_build/results?buildId=2441&view=logs&jobId=8999f565-280a-527f-721e-375d49cc4cd5&j=8999f565-280a-527f-721e-375d49cc4cd5&t=7980f6bf-f3f8-5575-47e3-a23aae330cb3

Since some of those tests are testing the testing environment (this sound funny), I think you should probably change those to match the requirements of this new version of Jest. Since this is a breaking change in Jest, it would make sense to break something like this. What do you think @gaearon?

Can you run e2e tests locally @ankurkaushal?

@ankurkaushal
Copy link
Author

@renatoalencar: Yep I can run it via yarn e2e:docker. Will check there as well.

@renatoalencar
Copy link

@renatoalencar: Yep I can run it via yarn e2e:docker. Will check there as well.

I'll run it here too and see what I find.

@ankurkaushal
Copy link
Author

@renatoalencar: Only error I see is this:

lerna ERR! EPUBLISHCONFLICT Cannot publish confusing-browser-globals@undefined over existing version.
e2e-kitchensink.sh: ERROR! An error was encountered executing line 88.

What about you?

@renatoalencar
Copy link

@renatoalencar: Only error I see is this:

lerna ERR! EPUBLISHCONFLICT Cannot publish confusing-browser-globals@undefined over existing version.
e2e-kitchensink.sh: ERROR! An error was encountered executing line 88.

What about you?

Yeah, same thing here 😞. Let's check for behavior first, then all the cases.

@renatoalencar
Copy link

@SimenB there's a lot of timeouts on the CI logs, do you have any idea why?

@SimenB
Copy link
Contributor

SimenB commented Mar 26, 2021

Nope, I have no idea what the tests are doing

@renatoalencar
Copy link

Nope, I have no idea what the tests are doing

This specific test case is timing out, but there's beforeEach call on this file. Which is imported in the test file, but it looks like it's not working.

I'm trying increasing directly on the file, let's see if that works.

@ankurkaushal
Copy link
Author

Updated to new version of jest. yarn build & yarn start works, most of the tests pass but not the ones testing jsx stuff.

For example, I get this error in packages/cra-template/template/src/App.test.js:

 FAIL  template/src/App.test.js
  ● Test suite failed to run

    Jest encountered an unexpected token

    Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.

    Out of the box Jest supports Babel, which will be used to transform your files into valid JS based on your Babel configuration.

    By default "node_modules" folder is ignored by transformers.

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/ecmascript-modules for how to enable it.
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/configuration
    For information about custom transformations, see:
    https://jestjs.io/docs/code-transformation

    Details:

    SyntaxError: /Users/ankurkaushal/projects/create-react-app/packages/cra-template/template/src/App.test.js: Support for the experimental syntax 'jsx' isn't currently enabled (5:10):

      3 |
      4 | test('renders learn react link', () => {
    > 5 |   render(<App />);
        |          ^
      6 |   const linkElement = screen.getByText(/learn react/i);
      7 |   expect(linkElement).toBeInTheDocument();
      8 | });

    Add @babel/preset-react (https://git.io/JfeDR) to the 'presets' section of your Babel config to enable transformation.
    If you want to leave it as-is, add @babel/plugin-syntax-jsx (https://git.io/vb4yA) to the 'plugins' section to enable parsing.

      at Parser._raise (../../node_modules/@babel/parser/src/parser/error.js:134:45)
      at Parser.raiseWithData (../../node_modules/@babel/parser/src/parser/error.js:129:17)
      at Parser.expectOnePlugin (../../node_modules/@babel/parser/src/parser/util.js:191:18)
      at Parser.parseExprAtom (../../node_modules/@babel/parser/src/parser/expression.js:1212:18)
      at Parser.parseExprSubscripts (../../node_modules/@babel/parser/src/parser/expression.js:616:23)
      at Parser.parseUpdate (../../node_modules/@babel/parser/src/parser/expression.js:596:21)
      at Parser.parseMaybeUnary (../../node_modules/@babel/parser/src/parser/expression.js:563:23)
      at Parser.parseExprOps (../../node_modules/@babel/parser/src/parser/expression.js:367:23)
      at Parser.parseMaybeConditional (../../node_modules/@babel/parser/src/parser/expression.js:332:23)
      at Parser.parseMaybeAssign (../../node_modules/@babel/parser/src/parser/expression.js:287:21)

Am I missing something here with the jest upgrade?

Upgrades from 27.0.1 to 27.0.3
@ankurkaushal
Copy link
Author

ankurkaushal commented Jun 2, 2021

Latest Update:

Updated to new version of jest again. yarn build & yarn start still works but still getting a lot of tests with the following errors:

  • Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.
  • SyntaxError: Identifier 'jest' has already been declared
  • SyntaxError: Cannot use import statement outside a module

Not sure if my machine has something funky going on or is there anything else I need to double check?

Would appreciate another pair of eyes on this.

@mandaputtra
Copy link

@ankurkaushal how do you test this branch? Would love some advice on that, I'll try to use 27.0 jest versions.

@yifanwww
Copy link

@ankurkaushal

For the first error and the third error, add transform: { '^.+\\.js$': 'babel-jest' } into the test/jest.config.js should fix them.

I'm not sure whether jest can find that jest.config.js by itself. You may need to execute yarn jest --config test/jest.config.js to enable the transforms.

BTW, you forgot to upgrade babel-jest.

@iansu
Copy link
Contributor

iansu commented Sep 22, 2021

I'm going to close this in favour of #11338 since the tests are all passing in that PR. Thank you for your contribution.

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

Successfully merging this pull request may close these issues.

Update to Jest 27 Ejecting causes Jest config to contain absolute paths
10 participants