add snapshots for mocked and unmocked components part 2 #24593
Conversation
facebook-github-bot
added
the
CLA Signed
| describe('<Picker />', () => { | ||
| it('should render as <View> when mocked', () => { | ||
| const instance = render.create( | ||
| <Picker selectedValue="foo" onValueChange={jest.fn()}> |
There was a problem hiding this comment.
Should we be abstracting this out into a constant at the top? It鈥檚 pretty important that they are consistent across all of the tests, right? Probably should do that for all of the tests.
|
|
||
| const React = require('React'); | ||
| // $FlowFixMe | ||
| const ProgressBarAndroid = require('../ProgressBarAndroid.android'); |
There was a problem hiding this comment.
@cpojer do you know how to fix this?
There was a problem hiding this comment.
This is fine. The iOS flow config doesn't see these modules. I don't think it matters much.
| `; | ||
|
|
||
| exports[`<Modal /> should shallow render as <Component> when mocked 1`] = ` | ||
| <Component |
There was a problem hiding this comment.
This is an example of displayName being broken. We鈥檒l need to fix this. Can you either fix it in this PR or change the name of this test to be consistent with the other component tests so it is clear this isn鈥檛 correct?
There was a problem hiding this comment.
馃 hmm modal is a mocked component. Are you sure this line isn't what's causing this?
https://github.com/facebook/react-native/blob/master/jest/mockComponent.js#L20
There was a problem hiding this comment.
Yeah, that鈥檚 probably it but my understanding was the name line in the render function of that file is the thing that should be setting the component鈥檚 display name. Interestingly, that doesn鈥檛 seem to show up in this snapshot test at all.
There was a problem hiding this comment.
Sorry this took a bit. This goes back to my original PR, and this is related to what prompted all of this 馃槵 I tried explaining it but confused myself and deleted it all. I've been doing a bad job explaining it thus far so I think I need some time to put together a cohesive description.
There was a problem hiding this comment.
@TheSavior is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
I'm gonna land this as is, we can continue to iterate and improve |
|
This pull request was successfully merged by @bcarroll22 in 782dc94. When will my fix make it into a release? | Upcoming Releases |
Summary
Per conversation with @TheSavior, in #24538, this adds snapshot tests for more components. Shallow and deep snapshots are included.
Changelog
[General] [Added] - Snapshots
Test Plan
This PR adds a lot of tests 馃槃