closes #13034 Fixes the ScrollViewMock methods #13048

alvaromb · 2017-03-21T11:09:38Z

Motivation (required)

Now the ScrollView mock has all the methods available.

Test Plan (required)

React Native tests pass.

To test this specific part of the code,

$ react-native init Test
$ cd Test/
$ yarn add react-navigation

Then, add a simple project that uses react-navigation:

import React from 'react';
import { Text } from 'react-native';
import { StackNavigator } from 'react-navigation';

class HomeScreen extends React.Component {
  static navigationOptions = {
    title: 'Welcome',
  };
  render() {
    return <Text>Hello, Navigation!</Text>;
  }
}

const SimpleApp = StackNavigator({
  Home: { screen: HomeScreen },
});

export default SimpleApp

Run the default render tests:

$ npm run test

eslint-bot · 2017-03-21T11:18:09Z

Libraries/Components/ScrollView/__mocks__/ScrollViewMock.js

@@ -17,7 +17,9 @@ const requireNativeComponent = require('requireNativeComponent');

 const RCTScrollView = requireNativeComponent('RCTScrollView');

-class ScrollViewMock extends React.Component {
+const ScrollViewComponent = jest.genMockFromModule('ScrollView');


identifier jest Could not resolve name

I would need some guidance to avoid this eslint error @satya164.

Add /* eslint-env jest */ to the top

Added lint rule to avoid unresolved identifier error

eslint-bot · 2017-03-21T14:55:37Z

Libraries/Components/ScrollView/__mocks__/ScrollViewMock.js

@@ -17,7 +18,9 @@ const requireNativeComponent = require('requireNativeComponent');

 const RCTScrollView = requireNativeComponent('RCTScrollView');

-class ScrollViewMock extends React.Component {


identifier jest Could not resolve name

satya164 · 2017-03-21T15:11:55Z

@facebook-github-bot shipit

facebook-github-bot · 2017-03-21T15:12:55Z

@satya164 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

alvaromb · 2017-04-05T16:55:39Z

Hey @satya164!

Can I help you with something else with this PR?

shergin · 2017-05-29T19:12:12Z

@alvaromb Seems this PR breaks tests. Could you please take a look?

alvaromb · 2017-06-06T07:34:26Z

Hi @shergin!

Sorry, I forgot about this PR. Please ping me if you need a hand with this. Thanks!

shergin · 2017-06-06T18:32:44Z

@alvaromb I would love to see passed tests. 😄

facebook-github-bot · 2017-07-20T01:18:09Z

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2017-07-20T14:48:16Z

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

alvaromb · 2017-07-26T09:34:03Z

Hi @shergin!

This PR got merged? If not, what can I help you with to merge it?

facebook-github-bot · 2017-10-03T17:04:07Z

@alvaromb I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

carlost · 2017-10-05T15:40:14Z

@alvaromb I got bitten by this issue an burned some time trying to track it down. a local change to how jest mocks out the ScrollView works but it would have been nice to have that time back :)

looks like your PR fails in CI and it now has a merge conflict with master.

CI

i can't see the details for the failure in CircleCI ... but the build was from a few months ago so it may have been flushed. The details for the travis-ci build failure are still available: https://travis-ci.org/facebook/react-native/jobs/255491208#L3150

The failure is:

node_modules/react-native/Libraries/Components/ScrollView/__mocks__/ScrollViewMock.js:23
23: const ScrollViewComponent = jest.genMockFromModule('ScrollView');
                                ^^^^ identifier `jest`. Could not resolve name

This occurs during the flow check portion of the iOS build. Based on the date of the build i think this is the line number in the build script where the failure occurs:

react-native/scripts/run-ci-e2e-tests.js

Line 203 in 29d9c35

if (exec(`${ROOT}/node_modules/.bin/flow check`).code) {

So the rub is, your source file to create the mock specifies the use of flow, but flow doesn't recognize the jest identifier.

Two options (and I am not sure which is the react-native preference because there are examples of both in the source):

strip off the @flow annotation from the file: example of that
explicitly declare the jest identifier: example of that

Merge Conflict

Looks like master was updated to support a flow v0.53. I don't think that this impacts your PR, you just need to rebase.

Let me know if you want to see this through or if you want to close out this PR and have me open up a replacement.

alvaromb · 2017-10-06T08:03:25Z

Just merged against master, tests fails due to other issues though.

alvaromb · 2017-10-06T08:29:07Z

Honestly, I don't know how I should proceed from here, should I merge against another branch? @shergin @carlost

shergin · 2017-10-06T17:15:02Z

¯_(ツ)_/¯
Previous landing was unsuccessful because fb internal tool detected that this diff got new version on GH.

alvaromb · 2017-10-09T09:57:43Z

Should I open a new PR @shergin @satya164 ?

facebook-github-bot · 2017-11-08T18:04:03Z

@alvaromb I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

facebook-github-bot

@shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: Solves facebook#13034 Now the `ScrollView` mock has all the methods available. React Native tests pass. To test this specific part of the code, ```sh $ react-native init Test $ cd Test/ $ yarn add react-navigation ``` Then, add a simple project that uses `react-navigation`: ```js import React from 'react'; import { Text } from 'react-native'; import { StackNavigator } from 'react-navigation'; class HomeScreen extends React.Component { static navigationOptions = { title: 'Welcome', }; render() { return <Text>Hello, Navigation!</Text>; } } const SimpleApp = StackNavigator({ Home: { screen: HomeScreen }, }); export default SimpleApp ``` Run the default render tests: ```js $ npm run test ``` Closes facebook#13048 Differential Revision: D4746028 Pulled By: shergin fbshipit-source-id: cb1791978d15be7f5d14b7b22979388066ad6caa

alvaromb mentioned this pull request Mar 21, 2017

Fix checking of scrollTo() satya164/react-native-tab-view#163

Closed

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. GH Review: review-needed labels Mar 21, 2017

alvaromb mentioned this pull request Mar 21, 2017

Bug running Jest with ScrollView dependant components #13034

Closed

eslint-bot reviewed Mar 21, 2017

View reviewed changes

alvaromb mentioned this pull request Mar 21, 2017

Added Linking library mock to the jest setup #13049

Closed

closes facebook#13034 Fixes the ScrollViewMock methods

fdf7f47

Added lint rule to avoid unresolved identifier error

alvaromb force-pushed the iss_13034 branch from a810553 to fdf7f47 Compare March 21, 2017 13:53

eslint-bot reviewed Mar 21, 2017

View reviewed changes

shergin added the JavaScript label Jun 1, 2017

shergin added 2 commits June 5, 2017 22:49

Update ScrollViewMock.js

a72d923

Merge branch 'master' into iss_13034

22dfa82

Merge branch 'master' into iss_13034

a5d8fc8

Merge branch 'master' into iss_13034

36d834c

facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Jul 20, 2017

facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Jul 20, 2017

Merge remote-tracking branch 'upstream/master' into iss_13034

e290e66

alvaromb added 2 commits October 8, 2017 11:59

Merge branch 'master' into iss_13034

aed1627

Merge branch 'master' into iss_13034

11df2b1

facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Nov 28, 2017

facebook-github-bot approved these changes Nov 28, 2017

View reviewed changes

facebook-github-bot closed this in 0c8a3e4 Nov 28, 2017

hramos added the Merged This PR has been merged. label Mar 8, 2019

react-native-bot removed the Import Started This pull request has been imported. This does not imply the PR has been approved. label Mar 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

closes #13034 Fixes the ScrollViewMock methods #13048

closes #13034 Fixes the ScrollViewMock methods #13048

alvaromb commented Mar 21, 2017

eslint-bot Mar 21, 2017

alvaromb Mar 21, 2017

satya164 Mar 21, 2017

eslint-bot Mar 21, 2017

satya164 commented Mar 21, 2017

facebook-github-bot commented Mar 21, 2017

alvaromb commented Apr 5, 2017

shergin commented May 29, 2017

alvaromb commented Jun 6, 2017

shergin commented Jun 6, 2017

facebook-github-bot commented Jul 20, 2017

facebook-github-bot commented Jul 20, 2017

alvaromb commented Jul 26, 2017

facebook-github-bot commented Oct 3, 2017

carlost commented Oct 5, 2017

alvaromb commented Oct 6, 2017

alvaromb commented Oct 6, 2017

shergin commented Oct 6, 2017

alvaromb commented Oct 9, 2017

facebook-github-bot commented Nov 8, 2017

facebook-github-bot left a comment

		@@ -17,7 +18,9 @@ const requireNativeComponent = require('requireNativeComponent');

		const RCTScrollView = requireNativeComponent('RCTScrollView');

		class ScrollViewMock extends React.Component {

closes #13034 Fixes the ScrollViewMock methods #13048

closes #13034 Fixes the ScrollViewMock methods #13048

Conversation

alvaromb commented Mar 21, 2017

Motivation (required)

Test Plan (required)

eslint-bot Mar 21, 2017

Choose a reason for hiding this comment

alvaromb Mar 21, 2017

Choose a reason for hiding this comment

satya164 Mar 21, 2017

Choose a reason for hiding this comment

eslint-bot Mar 21, 2017

Choose a reason for hiding this comment

satya164 commented Mar 21, 2017

facebook-github-bot commented Mar 21, 2017

alvaromb commented Apr 5, 2017

shergin commented May 29, 2017

alvaromb commented Jun 6, 2017

shergin commented Jun 6, 2017

facebook-github-bot commented Jul 20, 2017

facebook-github-bot commented Jul 20, 2017

alvaromb commented Jul 26, 2017

facebook-github-bot commented Oct 3, 2017

carlost commented Oct 5, 2017

CI

Merge Conflict

alvaromb commented Oct 6, 2017

alvaromb commented Oct 6, 2017

shergin commented Oct 6, 2017

alvaromb commented Oct 9, 2017

facebook-github-bot commented Nov 8, 2017

facebook-github-bot left a comment

Choose a reason for hiding this comment