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

tests: Upgrade jest to fix current node 11 build #7413

Merged
merged 2 commits into from
Mar 7, 2019
Merged

Conversation

mattzeunert
Copy link
Contributor

@mattzeunert mattzeunert commented Mar 7, 2019

Requiring jest-mock breaks latest Node 11. Upgrade jest to fix.

Also jest-mock should be an explicit dependency if we require it directly.

@mattzeunert mattzeunert changed the title Upgrade jest to fix current node 11 build tests: Upgrade jest to fix current node 11 build Mar 7, 2019
@brendankenny
Copy link
Member

Requiring jest-mock breaks latest Node 11. Upgrade jest to fix.

ah, nice find!

Also jest-mock should be an explicit dependency if we require it directly.

do we require it directly? We're running jest as a command, I'm assuming it handles injecting mocking into the environment so wouldn't it be an indirect dependency?

@brendankenny
Copy link
Member

+1,204 −694

dear lord, jest, what are you doing

@mattzeunert
Copy link
Contributor Author

do we require it directly? We're running jest as a command, I'm assuming it handles injecting mocking into the environment so wouldn't it be an indirect dependency?

We use it inside mocha for yarn test-clients. Is mocha just being phased out or do we use it for a reason?

@brendankenny
Copy link
Member

Is mocha just being phased out or do we use it for a reason?

phased out. It should be trivial, but I haven't wanted to sit down and figure out our code coverage situation in package.json :)

@brendankenny
Copy link
Member

We use it inside mocha for yarn test-clients

including jest-mock for this sounds 👍 , all the more motivation for removing mocha :)

The command "yarn unit:silentcoverage" exited with 1.

speaking of having to figure out our code coverage situation :(

@mattzeunert
Copy link
Contributor Author

The command "yarn unit:silentcoverage" exited with 1.

Does that mean something? Can't repro it on local.

@mattzeunert
Copy link
Contributor Author

My laptop is out of battery and my charger is at the office, so I'll leave this for today. Feel free to clean up my mess 😭

@paulirish
Copy link
Member

during nyc report --reporter text-lcov > unit-coverage.lcov it hits an error:

/home/travis/build/GoogleChrome/lighthouse/node_modules/nyc/node_modules/yargs/yargs.js:1133
      else throw err
           ^
TypeError: Cannot read property 'decl' of undefined
    at /home/travis/build/GoogleChrome/lighthouse/node_modules/nyc/node_modules/istanbul-reports/lib/lcovonly/index.js:32:38
    at Array.forEach (<anonymous>)
    at TextLcov.LcovOnlyReport.onDetail (/home/travis/build/GoogleChrome/lighthouse/node_modules/nyc/node_modules/istanbul-reports/lib/lcovonly/index.js:30:28)
    at Visitor.(anonymous function) [as onDetail] (/home/travis/build/GoogleChrome/lighthouse/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:34:30)
    at ReportNode.Node.visit (/home/travis/build/GoogleChrome/lighthouse/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:123:17)
    at /home/travis/build/GoogleChrome/lighthouse/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:116:23
    at Array.forEach (<anonymous>)
    at visitChildren (/home/travis/build/GoogleChrome/lighthouse/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:115:32)
    at ReportNode.Node.visit (/home/travis/build/GoogleChrome/lighthouse/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:126:5)
    at /home/travis/build/GoogleChrome/lighthouse/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:116:23
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

And the offending line in lcovonly is this one:
https://github.com/istanbuljs/istanbuljs/blob/76069111918d07f35fe831fd6d0ca24982a20303/packages/istanbul-reports/lib/lcovonly/index.js#L31

and yeah thats 3/3 of us that can't repro this locally. :/

@paulirish
Copy link
Member

travis is green. i guess the travis cache clearing fixed it.

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@paulirish paulirish merged commit b1d2b83 into master Mar 7, 2019
@paulirish paulirish deleted the upgrade-jest branch March 7, 2019 21:27
@paulirish paulirish mentioned this pull request Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants