react-native
react-native copied to clipboard
Extend the local e2e test case to provide the REACT_NATIVE_MAVEN_LOCAL_REPO property
Summary
This PR is a follow up of https://github.com/facebook/react-native/pull/35075/files to ensure that even in the local e2e testing scenario the new maven approach is followed - without this, RNTestProject on Android won't work, like so:

Changelog
[Internal] [Fixed] - Extend the local e2e test case to provide the REACT_NATIVE_MAVEN_LOCAL_REPO property
Test Plan
Run yarn test-e2e-local -t RNTestProject -p Android successfully. (which is not what's happening now, reason why PR is still in draft)
| Platform | Engine | Arch | Size (bytes) | Diff |
|---|---|---|---|---|
| ios | - | universal | n/a | -- |
Base commit: 64c3906280bbb643f758fa86f92a6d179007e36b Branch: main
| Platform | Engine | Arch | Size (bytes) | Diff |
|---|---|---|---|---|
| android | hermes | arm64-v8a | 6,992,247 | +0 |
| android | hermes | armeabi-v7a | 6,368,778 | +0 |
| android | hermes | x86 | 7,404,941 | +0 |
| android | hermes | x86_64 | 7,268,948 | +0 |
| android | jsc | arm64-v8a | 8,855,839 | +0 |
| android | jsc | armeabi-v7a | 7,594,355 | +0 |
| android | jsc | x86 | 8,913,589 | +0 |
| android | jsc | x86_64 | 9,396,951 | +0 |
Base commit: 64c3906280bbb643f758fa86f92a6d179007e36b Branch: main
@kelset I am planning to rename the tarball methods in hermes-utils.js in order to make a clear distinction between the different tarballs that we deal with:
- Hermes source code tarball, generated by GitHub and downloaded from github.com/facebook/hermes
- Hermes prebuilt artifact tarball, generated by Circle CI and downloaded from Maven
- Hermes debug symbols tarball, generated by Circle CI and downloaded from Maven (WIP)
Here's the PR: https://github.com/facebook/react-native/pull/35156 - I have updated all the internal references to these methods.
I realize this would conflict with your PR. What do you think? Would renaming these have an adverse impact on other scripts from the community? Would you be okay rebasing your work on top of https://github.com/facebook/react-native/pull/35156 if it is merged?
@hramos go ahead and merge yours, yeah I'll have conflicts but I'm still in the testing phase for this one so it's likely that your PR will be quicker to land
Question/idea for a later PR: do we want to setup a circleci job that runs this script? In that way, if someone commits something that could potentially break it, the check will ask him to also fix the script.
WDYT?
update: now the PR should be ready, I've run a few more tests and done more tweaks. Every flow seems to be working as expected.
re: @cipolleschi
Question/idea for a later PR: do we want to setup a circleci job that runs this script? In that way, if someone commits something that could potentially break it, the check will ask him to also fix the script.
No, I don't think there's value in running this script specifically - on CI it'd basically just build RNTester and RNTestProject, take a lot of time and not verify much that is not already verified by other CI jobs. The main advantage of this script is in the local scenario, where you want to have 1 handy command to get easily to have a local build working so that you can play around with it in the emulator/simulator.
This topic overlaps with the E2E conversation that is been happening in parallel, so I'm sure that we'll be able to do some refactoring so that both the E2E flows and this script will have less custom code and we can generalize certain sections
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
quick headsup, I've updated this PR to account for some of the commits that landed in the last 12/24 hrs, such as:
- https://github.com/facebook/react-native/commit/d71d0db51dec825ed9151ea02533279411381bb2
- https://github.com/facebook/react-native/commit/b5bb227be88fc89538279cfd84fbec810e2b8aa2
And especially https://github.com/facebook/react-native/commit/6b8e13f53c5fc9c5e794bb7491648e4b25ad5811 which for some reason did add some new scripts but not as executable, so took care of it.
Before:

After:

final update (I hope 🤣):
everything working as expected:

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
This pull request was successfully merged by @kelset in c540ff7bd156a3b1636161a42128dfc2b575e031.
When will my fix make it into a release? | Upcoming Releases