From a4edf8ef37f8df5075dc898dd213754adb230d5e Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Fri, 27 Oct 2023 07:25:02 -0700 Subject: [PATCH 1/4] Fix RNTestProject testing on Android (#41172) Summary: While releasing RN 0.73.0-RC3, we relaized that the e2e test script was bugged for Android when used to test RNTestProject with the `-c` option. There were 2 problems: - The downloaded maven-local was not actually used because it doesn't work with a zip. (We were always downloading a version from Maven) - The versions of React Native between maven-local and the locally packaged React Native were different. This change fixes the script by: - Downloading maven-local - Unzipping maven-local and passing the new folder to the Android app - Downloading the React Native version that has been packaged in CI By unzipping maven-local and using the unzipped folder, we make sure that Android is actually using the local repository. By downloading both the packaged react native and the maven-local from the same CI workflow, we ensure that the versions are aligned. This also speeds-up further the Android testing. While running this change, we also moved the `pod install` step inside the `if (iOS)` branch, so we do not install Cocoapods if we need to test Android. [Internal] - Fix Android E2E test script when downloading artefacts from CI Pull Request resolved: https://github.com/facebook/react-native/pull/41172 Test Plan: Tested locally on both main and 0.73-stable, on both Android and iOS Reviewed By: cortinico Differential Revision: D50651448 Pulled By: cipolleschi fbshipit-source-id: 70a9ed19072119d19c5388e8a4309d7333a08e13 --- scripts/circle-ci-artifacts-utils.js | 19 +++++++- scripts/test-e2e-local.js | 68 ++++++++++++---------------- scripts/testing-utils.js | 9 +++- 3 files changed, 52 insertions(+), 44 deletions(-) diff --git a/scripts/circle-ci-artifacts-utils.js b/scripts/circle-ci-artifacts-utils.js index 3a9d9d2a0c83f1..48d02327b088ba 100644 --- a/scripts/circle-ci-artifacts-utils.js +++ b/scripts/circle-ci-artifacts-utils.js @@ -140,8 +140,11 @@ async function _findUrlForJob(jobName, artifactPath) { _throwIfJobIsUnsuccessful(job); const artifacts = await _getJobsArtifacts(job.job_number); - return artifacts.find(artifact => artifact.path.indexOf(artifactPath) > -1) - .url; + let artifact = artifacts.find(a => a.path.indexOf(artifactPath) > -1); + if (!artifact) { + throw new Error(`I could not find the artifact with path ${artifactPath}`); + } + return artifact.url; } function _throwIfJobIsNull(job) { @@ -168,6 +171,17 @@ async function artifactURLForMavenLocal() { return _findUrlForJob('build_and_publish_npm_package-2', 'maven-local.zip'); } +async function artifactURLForReactNative() { + let shortCommit = exec('git rev-parse HEAD', {silent: true}) + .toString() + .trim() + .slice(0, 9); + return _findUrlForJob( + 'build_npm_package', + `react-native-1000.0.0-${shortCommit}.tgz`, + ); +} + async function artifactURLForHermesRNTesterAPK(emulatorArch) { return _findUrlForJob( 'test_android', @@ -194,5 +208,6 @@ module.exports = { artifactURLForHermesRNTesterAPK, artifactURLForMavenLocal, artifactURLHermesDebug, + artifactURLForReactNative, baseTmpPath, }; diff --git a/scripts/test-e2e-local.js b/scripts/test-e2e-local.js index 0be3c7bb6a4950..c2e4f212a2cf69 100644 --- a/scripts/test-e2e-local.js +++ b/scripts/test-e2e-local.js @@ -20,7 +20,6 @@ const {exec, pushd, popd, pwd, cd} = require('shelljs'); const updateTemplatePackage = require('./update-template-package'); const yargs = require('yargs'); const path = require('path'); -const fs = require('fs'); const { checkPackagerRunning, @@ -189,15 +188,12 @@ async function testRNTestProject(circleCIArtifacts) { // in local testing, 1000.0.0 mean we are on main, every other case means we are // working on a release version const buildType = baseVersion !== '1000.0.0' ? 'release' : 'dry-run'; + const shortCommit = exec('git rev-parse HEAD', {silent: true}) + .toString() + .trim() + .slice(0, 9); - // we need to add the unique timestamp to avoid npm/yarn to use some local caches - const dateIdentifier = new Date() - .toISOString() - .slice(0, -8) - .replace(/[-:]/g, '') - .replace(/[T]/g, '-'); - - const releaseVersion = `${baseVersion}-${dateIdentifier}`; + const releaseVersion = `1000.0.0-${shortCommit}`; // Prepare some variables for later use const repoRoot = pwd(); @@ -206,7 +202,7 @@ async function testRNTestProject(circleCIArtifacts) { const mavenLocalPath = circleCIArtifacts != null - ? path.join(circleCIArtifacts.baseTmpPath(), 'maven-local.zip') + ? path.join(circleCIArtifacts.baseTmpPath(), 'maven-local') : '/private/tmp/maven-local'; const hermesPath = await prepareArtifacts( circleCIArtifacts, @@ -218,30 +214,13 @@ async function testRNTestProject(circleCIArtifacts) { ); updateTemplatePackage({ - 'react-native': `file:${localNodeTGZPath}`, + 'react-native': `file://${localNodeTGZPath}`, }); - // create locally the node module - exec('npm pack --pack-destination ', {cwd: reactNativePackagePath}); - - // node pack does not creates a version of React Native with the right name on main. - // Let's add some defensive programming checks: - if (!fs.existsSync(localNodeTGZPath)) { - const tarfile = fs - .readdirSync(reactNativePackagePath) - .find(name => name.startsWith('react-native-') && name.endsWith('.tgz')); - if (!tarfile) { - throw new Error("Couldn't find a zipped version of react-native"); - } - exec( - `cp ${path.join(reactNativePackagePath, tarfile)} ${localNodeTGZPath}`, - ); - } - pushd('/tmp/'); // need to avoid the pod install step - we'll do it later exec( - `node ${reactNativePackagePath}/cli.js init RNTestProject --template ${localNodeTGZPath} --skip-install`, + `node ${reactNativePackagePath}/cli.js init RNTestProject --template ${reactNativePackagePath} --skip-install`, ); cd('RNTestProject'); @@ -249,21 +228,30 @@ async function testRNTestProject(circleCIArtifacts) { // need to do this here so that Android will be properly setup either way exec( - `echo "REACT_NATIVE_MAVEN_LOCAL_REPO=${mavenLocalPath}" >> android/gradle.properties`, - ); - - // doing the pod install here so that it's easier to play around RNTestProject - cd('ios'); - exec('bundle install'); - exec( - `HERMES_ENGINE_TARBALL_PATH=${hermesPath} USE_HERMES=${ - argv.hermes ? 1 : 0 - } bundle exec pod install --ansi`, + `echo "react.internal.mavenLocalRepo=${mavenLocalPath}/tmp/maven-local" >> android/gradle.properties`, ); - cd('..'); + // Update gradle properties to set Hermes as false + if (!argv.hermes) { + sed( + '-i', + 'hermesEnabled=true', + 'hermesEnabled=false', + 'android/gradle.properties', + ); + } if (argv.platform === 'iOS') { + // doing the pod install here so that it's easier to play around RNTestProject + cd('ios'); + exec('bundle install'); + exec( + `HERMES_ENGINE_TARBALL_PATH=${hermesPath} USE_HERMES=${ + argv.hermes ? 1 : 0 + } bundle exec pod install --ansi`, + ); + + cd('..'); exec('yarn ios'); } else { // android diff --git a/scripts/testing-utils.js b/scripts/testing-utils.js index b48d1a85d25d29..b10a322bc9dc73 100644 --- a/scripts/testing-utils.js +++ b/scripts/testing-utils.js @@ -170,16 +170,21 @@ async function downloadArtifactsFromCircleCI( ) { const mavenLocalURL = await circleCIArtifacts.artifactURLForMavenLocal(); const hermesURL = await circleCIArtifacts.artifactURLHermesDebug(); + const reactNativeURL = await circleCIArtifacts.artifactURLForReactNative(); const hermesPath = path.join( circleCIArtifacts.baseTmpPath(), 'hermes-ios-debug.tar.gz', ); - console.info('[Download] Maven Local Artifacts'); - circleCIArtifacts.downloadArtifact(mavenLocalURL, mavenLocalPath); + console.info(`[Download] Maven Local Artifacts from ${mavenLocalURL}`); + const mavenLocalZipPath = `${mavenLocalPath}.zip`; + circleCIArtifacts.downloadArtifact(mavenLocalURL, mavenLocalZipPath); + exec(`unzip -oq ${mavenLocalZipPath} -d ${mavenLocalPath}`); console.info('[Download] Hermes'); circleCIArtifacts.downloadArtifact(hermesURL, hermesPath); + console.info(`[Download] React Native from ${reactNativeURL}`); + circleCIArtifacts.downloadArtifact(reactNativeURL, localNodeTGZPath); return hermesPath; } From eb84092e1b114f4241724f582b6ca12ef7e662e2 Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Mon, 6 Nov 2023 06:05:07 -0800 Subject: [PATCH 2/4] Backport e2e script changes to main (#41332) Summary: Last week, I modified the e2e script to make sure it was working properly with 0.73. This change backport those changes in main ## Changelog: [Internal] - Backport e2e script changes Pull Request resolved: https://github.com/facebook/react-native/pull/41332 Test Plan: Tested locally Reviewed By: dmytrorykun Differential Revision: D51025796 Pulled By: cipolleschi fbshipit-source-id: 89ecd3701eaac4ba4bdde2c640df45a158329158 --- scripts/test-e2e-local.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/scripts/test-e2e-local.js b/scripts/test-e2e-local.js index c2e4f212a2cf69..6c795b15dd8307 100644 --- a/scripts/test-e2e-local.js +++ b/scripts/test-e2e-local.js @@ -183,17 +183,16 @@ async function testRNTestProject(circleCIArtifacts) { // create the local npm package to feed the CLI // base setup required (specular to publish-npm.js) - const baseVersion = require('../packages/react-native/package.json').version; // in local testing, 1000.0.0 mean we are on main, every other case means we are // working on a release version - const buildType = baseVersion !== '1000.0.0' ? 'release' : 'dry-run'; const shortCommit = exec('git rev-parse HEAD', {silent: true}) .toString() .trim() .slice(0, 9); const releaseVersion = `1000.0.0-${shortCommit}`; + const buildType = 'dry-run'; // Prepare some variables for later use const repoRoot = pwd(); @@ -204,6 +203,7 @@ async function testRNTestProject(circleCIArtifacts) { circleCIArtifacts != null ? path.join(circleCIArtifacts.baseTmpPath(), 'maven-local') : '/private/tmp/maven-local'; + const hermesPath = await prepareArtifacts( circleCIArtifacts, mavenLocalPath, @@ -226,9 +226,16 @@ async function testRNTestProject(circleCIArtifacts) { cd('RNTestProject'); exec('yarn install'); + // When using CircleCI artifacts, the CI will zip maven local into a + // /tmp/maven-local subfolder struct. + // When we generate the project manually, there is no such structure. + const expandedMavenLocal = + circleCIArtifacts == null + ? mavenLocalPath + : `${mavenLocalPath}/tmp/maven-local`; // need to do this here so that Android will be properly setup either way exec( - `echo "react.internal.mavenLocalRepo=${mavenLocalPath}/tmp/maven-local" >> android/gradle.properties`, + `echo "react.internal.mavenLocalRepo=${expandedMavenLocal}" >> android/gradle.properties`, ); // Update gradle properties to set Hermes as false From 95fe3fc49965fb449d4035bf9ac2ccbba6f5cd57 Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Wed, 8 Nov 2023 14:55:03 +0000 Subject: [PATCH 3/4] Use old REACT_NATIVE_MAVEN_LOCAL_REPO env var --- scripts/test-e2e-local.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/test-e2e-local.js b/scripts/test-e2e-local.js index 6c795b15dd8307..13d2bda126e89e 100644 --- a/scripts/test-e2e-local.js +++ b/scripts/test-e2e-local.js @@ -235,7 +235,7 @@ async function testRNTestProject(circleCIArtifacts) { : `${mavenLocalPath}/tmp/maven-local`; // need to do this here so that Android will be properly setup either way exec( - `echo "react.internal.mavenLocalRepo=${expandedMavenLocal}" >> android/gradle.properties`, +`echo "REACT_NATIVE_MAVEN_LOCAL_REPO=${expandedMavenLocal}" >> android/gradle.properties`, ); // Update gradle properties to set Hermes as false From 2bb5da1dc3150f062983993cf69d9048e0d3839b Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Wed, 8 Nov 2023 16:19:20 +0000 Subject: [PATCH 4/4] prettier --- scripts/test-e2e-local.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/test-e2e-local.js b/scripts/test-e2e-local.js index 13d2bda126e89e..3d23322931ca18 100644 --- a/scripts/test-e2e-local.js +++ b/scripts/test-e2e-local.js @@ -235,7 +235,7 @@ async function testRNTestProject(circleCIArtifacts) { : `${mavenLocalPath}/tmp/maven-local`; // need to do this here so that Android will be properly setup either way exec( -`echo "REACT_NATIVE_MAVEN_LOCAL_REPO=${expandedMavenLocal}" >> android/gradle.properties`, + `echo "REACT_NATIVE_MAVEN_LOCAL_REPO=${expandedMavenLocal}" >> android/gradle.properties`, ); // Update gradle properties to set Hermes as false