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

Fix Detox tests after upgrading to latest CLI #23191

Closed
wants to merge 1 commit into from
Closed

Conversation

grabbou
Copy link
Contributor

@grabbou grabbou commented Jan 28, 2019

Latest changes inside CLI now require that Metro configuration is provided when building RNTester app. This is to let CLI know that instead of looking for "react-native" under "node_modules" (that is obviously not present since we are running from source), it should check the paths provided.

When running "npm start", it finds the configuration at the root. However, when building through Xcode (e.g. "react-native bundle" or "xcodebuild" in Release scheme), it runs "react-native-xcode.sh" that works in different folder and makes Metro not detect the configuration file.

This PR explicitly sets path to the configuration via BUNDLE_CONFIG.

It also removes pwd from being prepended to all BUNDLE_CONFIG values. In my case, pwd was /Users/grabbou and the RNTester files where inside /Users/grabbou/Repositories/react-native. I was unable to point the script to correct location without making it aware of the folder structure - which is not going to work on the CI.

@facebook-github-bot 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. Tests This PR adds or edits a test case. labels Jan 28, 2019
@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Jan 28, 2019
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@react-native-bot
Copy link
Collaborator

@grabbou merged commit 9a93704 into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Jan 29, 2019
@react-native-bot react-native-bot added the Merged This PR has been merged. label Jan 29, 2019
@zpao zpao deleted the fix/detox_tests branch January 31, 2019 01:51
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
matt-oakes pushed a commit to matt-oakes/react-native that referenced this pull request Feb 7, 2019
Summary:
Latest changes inside CLI now require that Metro configuration is provided when building RNTester app. This is to let CLI know that instead of looking for "react-native" under "node_modules" (that is obviously not present since we are running from source), it should check the paths provided.

When running "npm start", it finds the configuration at the root. However, when building through Xcode (e.g. "react-native bundle" or "xcodebuild" in Release scheme), it runs "react-native-xcode.sh" that works in different folder and makes Metro not detect the configuration file.

This PR explicitly sets path to the configuration via `BUNDLE_CONFIG`.

It also removes `pwd` from being prepended to all `BUNDLE_CONFIG` values. In my case, `pwd` was `/Users/grabbou` and the RNTester files where inside `/Users/grabbou/Repositories/react-native`. I was unable to point the script to correct location without making it aware of the folder structure - which is not going to work on the CI.
Pull Request resolved: facebook#23191

Differential Revision: D13851741

Pulled By: hramos

fbshipit-source-id: d920353fd68d39468bd33bd1ad47e03b017a7727
rozele pushed a commit to microsoft/react-native-windows that referenced this pull request Jul 18, 2019
Summary:
Latest changes inside CLI now require that Metro configuration is provided when building RNTester app. This is to let CLI know that instead of looking for "react-native" under "node_modules" (that is obviously not present since we are running from source), it should check the paths provided.

When running "npm start", it finds the configuration at the root. However, when building through Xcode (e.g. "react-native bundle" or "xcodebuild" in Release scheme), it runs "react-native-xcode.sh" that works in different folder and makes Metro not detect the configuration file.

This PR explicitly sets path to the configuration via `BUNDLE_CONFIG`.

It also removes `pwd` from being prepended to all `BUNDLE_CONFIG` values. In my case, `pwd` was `/Users/grabbou` and the RNTester files where inside `/Users/grabbou/Repositories/react-native`. I was unable to point the script to correct location without making it aware of the folder structure - which is not going to work on the CI.
Pull Request resolved: facebook/react-native#23191

Differential Revision: D13851741

Pulled By: hramos

fbshipit-source-id: d920353fd68d39468bd33bd1ad47e03b017a7727
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Tests This PR adds or edits a test case.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants