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 & react-native #2542

Merged
merged 7 commits into from
Dec 23, 2017
Merged

UPGRADE jest & react-native #2542

merged 7 commits into from
Dec 23, 2017

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Dec 23, 2017

Issue: -jest is outdated and this is causing bithound to nag us-

What I did

  • upgraded Jest
  • upgraded react-native
  • fixed tests / issues on core
  • fixed tests / issues on RN
  • fixed issues in RN examples
  • test React-Native examples

@ndelangen ndelangen added dependencies:update maintenance User-facing maintenance tasks labels Dec 23, 2017
@ndelangen ndelangen self-assigned this Dec 23, 2017
@ndelangen ndelangen requested a review from Hypnosphi December 23, 2017 00:15
@@ -1,6 +1,14 @@
import initStoryshots, { snapshotWithOptions } from '@storybook/addon-storyshots';
import path from 'path';

jest.mock('global', () => ({
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this should actually go in the storyshots addon code

jest.config.js Outdated
@@ -1,5 +1,5 @@
module.exports = {
cacheDirectory: '.cache/jest',
// cacheDirectory: '.cache/jest',
Copy link
Member Author

Choose a reason for hiding this comment

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

Might have to revert this

@ndelangen ndelangen changed the base branch from master to release/3.3 December 23, 2017 17:50
navigator.userAgent !== 'storyshots' &&
!(navigator.userAgent.indexOf('Node.js') > -1);
!(navigator.userAgent.indexOf('Node.js') > -1) &&
!(navigator.userAgent.indexOf('jsdom') > -1);
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the main difference with Jest22, it depends on a higher version of jsdom, and is has a new userAgent

@@ -17,7 +16,7 @@ const onClose = action('onClose');

// For some reason react-modal causes an segfault (infinite loop maybe?)
// when rendered by storyshots/react-test-renderer
if (!navigator.userAgent.match(/Node\.js/)) {
if (!navigator.userAgent.match(/Node\.js/) && !navigator.userAgent.match(/jsdom/)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I kept both for compatibility

@@ -62,14 +62,18 @@
"glob": "^7.1.2",
"husky": "^0.14.3",
"inquirer": "^4.0.1",
"jest": "^21.2.0",
"jest-cli": "^21.2.1",
"jest": "^22.0.4",
Copy link
Member Author

Choose a reason for hiding this comment

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

I added these dependencies to make sure the right versions were hoisted to the top level.. It seems CRA has some old version of something

# Conflicts:
#	examples/cra-kitchen-sink/package.json
#	package.json
#	yarn.lock
@storybookjs storybookjs deleted a comment from codecov bot Dec 23, 2017
@codecov
Copy link

codecov bot commented Dec 23, 2017

Codecov Report

Merging #2542 into release/3.3 will increase coverage by 12.5%.
The diff coverage is 55.55%.

Impacted file tree graph

@@               Coverage Diff               @@
##           release/3.3    #2542      +/-   ##
===============================================
+ Coverage        20.18%   32.69%   +12.5%     
===============================================
  Files              398      398              
  Lines             8838     8837       -1     
  Branches           969      947      -22     
===============================================
+ Hits              1784     2889    +1105     
+ Misses            6286     5310     -976     
+ Partials           768      638     -130
Impacted Files Coverage Δ
app/angular/src/client/preview/index.js 0% <ø> (ø) ⬆️
app/react/src/client/preview/index.js 0% <0%> (ø) ⬆️
app/vue/src/client/preview/index.js 0% <0%> (ø) ⬆️
...amples/react-native-vanilla/storybook/storybook.js 100% <100%> (ø) ⬆️
addons/info/src/components/markdown/index.js 96.66% <100%> (+96.66%) ⬆️
...ui/src/modules/ui/components/search_box.stories.js 57.14% <100%> (-5.36%) ⬇️
lib/ui/src/modules/ui/configs/handle_routing.js 24.21% <50%> (ø) ⬆️
addons/knobs/src/components/PropField.js 10.63% <0%> (ø) ⬆️
lib/ui/src/modules/ui/components/layout/index.js 29.28% <0%> (ø) ⬆️
addons/jest/src/components/Result.js 0% <0%> (ø) ⬆️
... and 104 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac4a547...defc1db. Read the comment docs.

@ndelangen
Copy link
Member Author

This should actually fix the bithound status, I'm confused why it's still red

@ndelangen
Copy link
Member Author

I've tested RN on iOS, it seems to work just fine

@ndelangen
Copy link
Member Author

Can someone else @storybooks/team please verify the RN stuff on their machine? 🙇

@ndelangen ndelangen changed the title UPGRADE jest UPGRADE jest & react-native Dec 23, 2017
@ndelangen ndelangen merged commit f16b9b4 into release/3.3 Dec 23, 2017
@ndelangen ndelangen deleted the upgrade-jest branch December 23, 2017 22:20
@pnkapadia6
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants