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 Install test for RN >= 0.73 #6189

Merged
merged 31 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
70161c9
First attempt
takameyer Oct 10, 2023
aca6331
Patch Gemfile
takameyer Oct 10, 2023
69585ce
Revert Gemfile fix, as this was fixed by meta
takameyer Oct 10, 2023
c2f2c51
Try to use xcode 15
takameyer Oct 10, 2023
58b85f2
Narrow the workflow and fix dir issue
takameyer Oct 10, 2023
46e039c
Narrow the workflow and fix dir issue
takameyer Oct 10, 2023
f35ae3a
Try and skip simulator step
takameyer Oct 10, 2023
70922a9
Try and use simulator action instead of command
takameyer Oct 10, 2023
788e9aa
Add and update README to help in the future
takameyer Oct 10, 2023
818f425
Renable the other tests
takameyer Oct 10, 2023
b082564
Add process shutdown to test
takameyer Oct 11, 2023
fe6985c
Constrain the matrix
takameyer Oct 11, 2023
151d1c1
Full matrix and M1 runner
takameyer Oct 11, 2023
bc1d6f6
Revert larger runner
takameyer Oct 11, 2023
572855d
Try M1 again and remove ruby step
takameyer Oct 11, 2023
d405c0c
Constrain matrix to Android only
takameyer Oct 11, 2023
8fed0c5
Set runner based on platform
takameyer Oct 11, 2023
157acae
Upgrade to minimum Java version
takameyer Oct 11, 2023
64807ce
Open up the matrix
takameyer Oct 11, 2023
592b7df
Constrain matrix to android and make simulator less featurefull
takameyer Oct 11, 2023
23dc80f
Attempt a different version of macos for Android
takameyer Oct 11, 2023
2066a4a
Add more messaging to test server
takameyer Oct 11, 2023
34d107e
Attempt simplifying server to diagnose android simulator issue
takameyer Oct 11, 2023
83e3f13
Turn off interactive mode
takameyer Oct 11, 2023
89d6353
Ensure that node is causing android issues
takameyer Oct 11, 2023
2634204
Tweak emulator options
takameyer Oct 11, 2023
07fb596
More cleanup for metro
takameyer Oct 12, 2023
39c4450
Remove emulator option change
takameyer Oct 12, 2023
8fca763
Run full matrix
takameyer Oct 12, 2023
aa62757
Cleanup
takameyer Oct 12, 2023
dcce394
Update .github/workflows/install-test-react-native.yml
takameyer Oct 13, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 12 additions & 20 deletions .github/workflows/install-test-react-native.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ defaults:
jobs:
install:
name: Install Test on React Native ${{ matrix.platform == 'ios' && 'iOS' || 'Android' }} realm@${{ matrix.realm-version }}, react-native@${{ matrix.react-native-version }} new architecture ${{ matrix.new-architecture && 'enabled' || 'disabled' }} running ${{ matrix.engine }}
runs-on: macos-12
# macos-13-xlarge is an M1 mac (which has no Android SDK)
runs-on: ${{ matrix.platform == 'ios' && 'macos-13-xlarge' || 'macos-13-large' }}
timeout-minutes: 120
strategy:
fail-fast: false
Expand All @@ -23,9 +24,9 @@ jobs:
- ios
- android
realm-version:
- v11 # Enable if we feel the need
- v11 # For the last major version
elle-j marked this conversation as resolved.
Show resolved Hide resolved
- latest
# - next
#- next # For release candidates
react-native-version:
- latest
- next
Expand All @@ -38,13 +39,10 @@ jobs:
- hermes
# See https://github.com/actions/runner-images/blob/main/images/macos/macos-12-Readme.md#xcode
xcode:
# - 14.0.1
- 13.1
- 15.0
node:
# RN bumped minimum Node version: https://github.com/huntie/react-native/blob/6cb6b81dd1c6b74a0f4bac5b06fa5aef93b48fe4/CHANGELOG.md?plain=1#L15
- 18
env:
DEVELOPER_DIR: /Applications/Xcode_${{ matrix.xcode }}.app
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
Expand All @@ -61,26 +59,18 @@ jobs:
- name: Configure ccache
run: ccache --set-config="compiler_check=content"

# TODO: Remove if this becomes unneeded in the future
- name: Invoke the simulator (making subsequent "open -a Simulator" calls work)
# Invoke a specific iPhone simulator to ensure AppleTV is not chosen as the default simulator
- uses: futureware-tech/simulator-action@v2
if: ${{ matrix.platform == 'ios' }}
run: open -a ${{ env.DEVELOPER_DIR }}/Contents/Developer/Applications/Simulator.app/Contents/MacOS/Simulator
with:
model: "iPhone 14"

- name: Install dependencies of the CLI
run: npm ci

- name: Initialize app
# Using "--skip-bundle-install" to let the setup-ruby action install the bundle
# Using "--skip-pod-install" to ensure it happens after setup-ruby has executed
run: npm run init -- --skip-bundle-install --skip-pod-install --realm-version ${{ matrix.realm-version }} --react-native-version ${{ matrix.react-native-version }} --engine ${{ matrix.engine }} --new-architecture ${{ matrix.new-architecture }}

- uses: ruby/setup-ruby@v1
if: ${{ matrix.platform == 'ios' }}
with:
ruby-version: '3.0'
bundler-cache: true
working-directory: install-tests/react-native/app

- if: ${{ matrix.platform == 'ios' }}
run: pod install
working-directory: install-tests/react-native/app/ios
Expand All @@ -89,7 +79,7 @@ jobs:
if: ${{ matrix.platform == 'android' }}
with:
distribution: 'zulu'
java-version: '11'
java-version: '17'

- name: Run test (iOS)
if: ${{ matrix.platform == 'ios' }}
Expand All @@ -103,6 +93,8 @@ jobs:
api-level: 29
target: google_apis
script: npm test -- --platform android
emulator-options: -no-snapshot-save -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
disable-animations: true
working-directory: install-tests/react-native

slack-workflow-status:
Expand Down
18 changes: 18 additions & 0 deletions install-tests/react-native/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# How To Use

There are two commands for the cli tool:
`init` and `test`

The most common usage is the following:

`npm run init -- --react-native-version next --realm-version latest`

This will create the template for latest release candidate of React Native, install the latest publish `realm` package
and patch the install test code necessary for the next step.

`npm run test -- --platform ios`
`npm run test -- --platform android`

This will run test for `ios` and `android` respectively. The `init` step is required first (without failures).
This will spawn the app and a server which will be waiting for the application to post a message to.
When the message has been sent, the test will shutdown and return a success code.
39 changes: 21 additions & 18 deletions install-tests/react-native/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ const DEFAULT_APP_PATH = path.resolve(__dirname, "app");
const PATCHES_PATH = path.resolve(__dirname, "patches");
const APP_JS_PATH = path.resolve(PATCHES_PATH, "App.js");
const CCACHE_PODFILE_PATCH_PATH = path.resolve(PATCHES_PATH, "ccache-Podfile.patch");
const JSC_PODFILE_PATCH_PATH = path.resolve(PATCHES_PATH, "jsc-Podfile.patch");
const JSC_BUILD_GRADLE_PATCH_PATH = path.resolve(PATCHES_PATH, "jsc-build.gradle.patch");
const CCACHE_PODFILE_PATCH_PATH_PRE_73 = path.resolve(PATCHES_PATH, "ccache-Podfile-pre-73.patch");
const PORT = 3000;
const TIMEOUT = 5 * 60 * 1000; // 5 min should be plenty of time from app has launched until message gets received

Expand Down Expand Up @@ -160,26 +159,17 @@ yargs(hideBin(process.argv))
// We're using force to succeed on peer dependency issues
exec("npm", ["install", `realm@${realmVersion}`, "--force"], { cwd: appPath });

const podfilePath = path.resolve(appPath, "ios", "Podfile");
console.log(`Patching podfile to use ccache (${podfilePath})`);
applyPatch(CCACHE_PODFILE_PATCH_PATH, podfilePath);

const { version: resolvedReactNativeVersion } = readPackageJson(
path.resolve(appPath, "node_modules/react-native"),
);

// TODO: Delete this once 0.71.0 or above is latest
if (semver.satisfies(resolvedReactNativeVersion, "<0.71.0")) {
if (engine === "jsc") {
console.log(`Patching Podfile to use JSC (${podfilePath})`);
applyPatch(JSC_PODFILE_PATCH_PATH, podfilePath);
const podfilePath = path.resolve(appPath, "ios", "Podfile");
console.log(`Patching podfile to use ccache (${podfilePath})`);

const appGradleBuildPath = path.resolve(appPath, "android", "app", "build.gradle");
console.log(`Patching app/build.gradle to use JSC (${appGradleBuildPath})`);
applyPatch(JSC_BUILD_GRADLE_PATCH_PATH, appGradleBuildPath);
}
if (semver.satisfies(resolvedReactNativeVersion, "<0.73.0")) {
applyPatch(CCACHE_PODFILE_PATCH_PATH_PRE_73, podfilePath);
} else {
console.log("Skipping patch of Podfile to use JSC, relying on USE_HERMES env variable instead");
applyPatch(CCACHE_PODFILE_PATCH_PATH, podfilePath);
}

// Store Gradle properties for RN >=0.71.0
Expand Down Expand Up @@ -234,7 +224,8 @@ yargs(hideBin(process.argv))
process.exit(code || 1);
}

const metro = cp.spawn("npx", ["react-native", "start"], { cwd: appPath, stdio: "inherit" });
// --no-interactive was added to turn off the dev menu, which was throwing an EIO error when the process was killed
const metro = cp.spawn("npx", ["react-native", "start", "--no-interactive"], { cwd: appPath, stdio: "inherit" });
metro.addListener("exit", prematureExitCallback);

const server = createServer().listen(PORT);
Expand Down Expand Up @@ -300,8 +291,20 @@ yargs(hideBin(process.argv))
// Kill metro, in silence
metro.removeListener("exit", prematureExitCallback);
metro.kill();

// Ensure metro is really dead
// If this is removed, it's highly likely the android emulator runner will not shut down correctly
const METRO_PORT = 8081;
Copy link
Contributor

Choose a reason for hiding this comment

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

If Metro's port was to change for some reason, can this be loaded dynamically rather than hardcoded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, but I think it's not worth the trouble until it actually happens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've also wondered if my workaround here could be substituted by sending a stricter exit code to the .exit() method above.

cp.exec(`lsof -i :${METRO_PORT} | grep LISTEN | awk '{print $2}' | xargs kill -9`, (error) => {
if (error) {
console.error(`Failed to kill process on port ${PORT}:`, error);
} else {
console.log(`Killed process on port ${PORT}`);
}
});

// Stop listening for the app
server.close();
server.close(() => process.exit());
}
},
)
Expand Down
18 changes: 18 additions & 0 deletions install-tests/react-native/patches/ccache-Podfile-pre-73.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
--- app/ios/Podfile
+++ app/ios/Podfile
@@ -39,5 +39,15 @@
:mac_catalyst_enabled => false
)
__apply_Xcode_12_5_M1_post_install_workaround(installer)
+ # Using ccache when building
+ installer.pods_project.targets.each do |target|
+ target.build_configurations.each do |config|
+ # Using the un-qualified names means you can swap in different implementations, for example ccache
+ config.build_settings["CC"] = "clang"
+ config.build_settings["LD"] = "clang"
+ config.build_settings["CXX"] = "clang++"
+ config.build_settings["LDPLUSPLUS"] = "clang++"
+ end
+ end
end
end
8 changes: 4 additions & 4 deletions install-tests/react-native/patches/ccache-Podfile.patch
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
--- app/ios/Podfile
+++ app/ios/Podfile
@@ -39,5 +39,15 @@
--- Podfile.copy 2023-10-09 15:29:25
+++ Podfile 2023-10-09 15:29:42
@@ -51,5 +51,15 @@
config[:reactNativePath],
:mac_catalyst_enabled => false
)
__apply_Xcode_12_5_M1_post_install_workaround(installer)
+ # Using ccache when building
+ installer.pods_project.targets.each do |target|
+ target.build_configurations.each do |config|
Expand Down
10 changes: 0 additions & 10 deletions install-tests/react-native/patches/jsc-Podfile.patch

This file was deleted.

11 changes: 0 additions & 11 deletions install-tests/react-native/patches/jsc-build.gradle.patch

This file was deleted.

Loading