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

Use packaged react-native in test-e2e-local script #36703

Closed

Conversation

fortmarek
Copy link
Contributor

@fortmarek fortmarek commented Mar 29, 2023

Summary

The current test-e2e-local script had two bugs:

  • On this line we were initializing a new RN project with the packed react-native created here
  • We were updating the local RN version after running npm pack here. This meant that the version inside the packaged react-native-xyz.tgz was not updated since we ran pack before updating it. This was fine since the init command was using the local react-native repository instead of the packed version.

Changelog:

[INTERNAL] [FIXED] - Use packaged react-native in test-e2e-local script

Test Plan

  • Run yarn test-e2e-local -t RNTestProject -p Android. The command should succeed.

I am not completely sure how to double check that we are using the packed version. Locally, I have a fsmonitor--daemon.ipc in my react-native/.git that can't be copied. The .git folder would be copied only when cli.js init was called with the whole repository – which is how I found out about the issue in the first place.

@fortmarek fortmarek requested a review from kelset March 29, 2023 14:36
@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. p: Shopify Partner: Shopify Partner labels Mar 29, 2023
Copy link
Contributor

@kelset kelset left a comment

Choose a reason for hiding this comment

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

looks reasonable to me

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,546,664 +0
android hermes armeabi-v7a 7,861,177 +0
android hermes x86 9,027,965 +0
android hermes x86_64 8,883,653 +0
android jsc arm64-v8a 9,167,110 +0
android jsc armeabi-v7a 8,357,650 +0
android jsc x86 9,223,172 +0
android jsc x86_64 9,481,667 +0

Base commit: c1c22eb
Branch: main

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Mar 29, 2023
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in 0328a8d.

kelset pushed a commit that referenced this pull request Mar 30, 2023
Summary:
The current `test-e2e-local` script had two bugs:
- On [this](https://github.com/facebook/react-native/blob/c1c22ebacc4097ce56f19385161ebb23ee1624b3/scripts/test-e2e-local.js#L219) line we were initializing a new RN project with the packed `react-native` created [here](https://github.com/facebook/react-native/blob/c1c22ebacc4097ce56f19385161ebb23ee1624b3/scripts/test-e2e-local.js#L211)
- We were updating the local RN version after running `npm pack` [here](https://github.com/facebook/react-native/blob/c1c22ebacc4097ce56f19385161ebb23ee1624b3/scripts/test-e2e-local.js#L214). This meant that the version inside the packaged `react-native-xyz.tgz` was not updated since we ran `pack` before updating it. This was fine since the `init` command was using the local `react-native` repository instead of the packed version.

## Changelog:

[INTERNAL] [FIXED] - Use packaged react-native in test-e2e-local script
<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[INTERNAL] [FIXED] - Use packaged react-native in test-e2e-local script

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

Pull Request resolved: #36703

Test Plan:
- Run `yarn test-e2e-local -t RNTestProject -p Android`. The command should succeed.

I am not completely sure how to double check that we are using the packed version. Locally, I have a `fsmonitor--daemon.ipc` in my `react-native/.git` that can't be copied. The `.git` folder would be copied only when `cli.js init` was called with the whole repository  – which is how I found out about the issue in the first place.

Reviewed By: hoxyq

Differential Revision: D44504599

Pulled By: cipolleschi

fbshipit-source-id: e57e2858bab46d4f978eed3cbaf3e504138594b8

# Conflicts:
#	scripts/test-e2e-local.js
kelset pushed a commit that referenced this pull request Mar 30, 2023
Summary:
The current `test-e2e-local` script had two bugs:
- On [this](https://github.com/facebook/react-native/blob/c1c22ebacc4097ce56f19385161ebb23ee1624b3/scripts/test-e2e-local.js#L219) line we were initializing a new RN project with the packed `react-native` created [here](https://github.com/facebook/react-native/blob/c1c22ebacc4097ce56f19385161ebb23ee1624b3/scripts/test-e2e-local.js#L211)
- We were updating the local RN version after running `npm pack` [here](https://github.com/facebook/react-native/blob/c1c22ebacc4097ce56f19385161ebb23ee1624b3/scripts/test-e2e-local.js#L214). This meant that the version inside the packaged `react-native-xyz.tgz` was not updated since we ran `pack` before updating it. This was fine since the `init` command was using the local `react-native` repository instead of the packed version.

## Changelog:

[INTERNAL] [FIXED] - Use packaged react-native in test-e2e-local script
<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[INTERNAL] [FIXED] - Use packaged react-native in test-e2e-local script

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

Pull Request resolved: #36703

Test Plan:
- Run `yarn test-e2e-local -t RNTestProject -p Android`. The command should succeed.

I am not completely sure how to double check that we are using the packed version. Locally, I have a `fsmonitor--daemon.ipc` in my `react-native/.git` that can't be copied. The `.git` folder would be copied only when `cli.js init` was called with the whole repository  – which is how I found out about the issue in the first place.

Reviewed By: hoxyq

Differential Revision: D44504599

Pulled By: cipolleschi

fbshipit-source-id: e57e2858bab46d4f978eed3cbaf3e504138594b8
@fortmarek fortmarek mentioned this pull request Apr 25, 2023
jeongshin pushed a commit to jeongshin/react-native that referenced this pull request May 7, 2023
Summary:
The current `test-e2e-local` script had two bugs:
- On [this](https://github.com/facebook/react-native/blob/c1c22ebacc4097ce56f19385161ebb23ee1624b3/scripts/test-e2e-local.js#L219) line we were initializing a new RN project with the packed `react-native` created [here](https://github.com/facebook/react-native/blob/c1c22ebacc4097ce56f19385161ebb23ee1624b3/scripts/test-e2e-local.js#L211)
- We were updating the local RN version after running `npm pack` [here](https://github.com/facebook/react-native/blob/c1c22ebacc4097ce56f19385161ebb23ee1624b3/scripts/test-e2e-local.js#L214). This meant that the version inside the packaged `react-native-xyz.tgz` was not updated since we ran `pack` before updating it. This was fine since the `init` command was using the local `react-native` repository instead of the packed version.

## Changelog:

[INTERNAL] [FIXED] - Use packaged react-native in test-e2e-local script
<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[INTERNAL] [FIXED] - Use packaged react-native in test-e2e-local script

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

Pull Request resolved: facebook#36703

Test Plan:
- Run `yarn test-e2e-local -t RNTestProject -p Android`. The command should succeed.

I am not completely sure how to double check that we are using the packed version. Locally, I have a `fsmonitor--daemon.ipc` in my `react-native/.git` that can't be copied. The `.git` folder would be copied only when `cli.js init` was called with the whole repository  – which is how I found out about the issue in the first place.

Reviewed By: hoxyq

Differential Revision: D44504599

Pulled By: cipolleschi

fbshipit-source-id: e57e2858bab46d4f978eed3cbaf3e504138594b8
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
The current `test-e2e-local` script had two bugs:
- On [this](https://github.com/facebook/react-native/blob/c1c22ebacc4097ce56f19385161ebb23ee1624b3/scripts/test-e2e-local.js#L219) line we were initializing a new RN project with the packed `react-native` created [here](https://github.com/facebook/react-native/blob/c1c22ebacc4097ce56f19385161ebb23ee1624b3/scripts/test-e2e-local.js#L211)
- We were updating the local RN version after running `npm pack` [here](https://github.com/facebook/react-native/blob/c1c22ebacc4097ce56f19385161ebb23ee1624b3/scripts/test-e2e-local.js#L214). This meant that the version inside the packaged `react-native-xyz.tgz` was not updated since we ran `pack` before updating it. This was fine since the `init` command was using the local `react-native` repository instead of the packed version.

## Changelog:

[INTERNAL] [FIXED] - Use packaged react-native in test-e2e-local script
<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[INTERNAL] [FIXED] - Use packaged react-native in test-e2e-local script

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

Pull Request resolved: facebook#36703

Test Plan:
- Run `yarn test-e2e-local -t RNTestProject -p Android`. The command should succeed.

I am not completely sure how to double check that we are using the packed version. Locally, I have a `fsmonitor--daemon.ipc` in my `react-native/.git` that can't be copied. The `.git` folder would be copied only when `cli.js init` was called with the whole repository  – which is how I found out about the issue in the first place.

Reviewed By: hoxyq

Differential Revision: D44504599

Pulled By: cipolleschi

fbshipit-source-id: e57e2858bab46d4f978eed3cbaf3e504138594b8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. p: Shopify Partner: Shopify Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants