fix: correct derivation of fully specified android launch activities #2388
+35
−5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mikehardy
added
platform: android
javascript
We really need tests for those functions :( |
|
@cortinico what's a way to run just a single jest test? if the test/debug loop was quick I'd add a new AndroidManifest (or a run with a --main-activity specified) that matches my case and fails-without-patch / works-with-patch There are tests actually - for most of the cases I think. Just not this one which allowed the regression to slip in |
You can use |
If you have a main activity that is not in package, and is fully specified, it used to work, but it regressed after adding auto-activity detection If your launcher / `MAIN` activity either specified by --main-activity or in AndroidManifest.xml is fully specified for example like "com.zoontek.rnbootsplash.RNBootSplashActivity" then it needs to launch for example "com.kullki.kscore.dev/com.zoontek.rnbootsplash.RNBootSplashActivity" not "com.kullki.kscore.dev/com.kullki.kscore.dev.com.zoontek.rnbootsplash.RNBootSplashActivity"
mikehardy
force-pushed
the
fix-fully-specified-non-package-android-main-activity
branch
from
b5e0f56
to
dc3ce6a
Compare
|
@cortinico Got it! 
Passes with my patch. No other tests fail with or without my patch so hopefully I haven't regressed something else... mike@isabela:~/work/react-random/cli (fix-fully-specified-non-package-android-main-activity *) % yarn jest --testMatch "**/cli-platform-android/**/__tests__/*tryLaunchAppOnDevice*.test.ts"
yarn run v1.22.22
$ /Users/mike/work/react-random/cli/node_modules/.bin/jest --testMatch '**/cli-platform-android/**/__tests__/*tryLaunchAppOnDevice*.test.ts'
PASS unit packages/cli-platform-android/src/commands/runAndroid/__tests__/tryLaunchAppOnDevice.test.ts
PASS e2e packages/cli-platform-android/src/commands/runAndroid/__tests__/tryLaunchAppOnDevice.test.ts
● Console
console.log
info Starting the app on "emulator-5554"...
at Object.info (packages/cli-tools/src/logger.ts:20:13)
console.log
info Starting the app on "emulator-5554"...
at Object.info (packages/cli-tools/src/logger.ts:20:13)
console.log
info Starting the app on "emulator-5554"...
at Object.info (packages/cli-tools/src/logger.ts:20:13)
console.log
info Starting the app...
at Object.info (packages/cli-tools/src/logger.ts:20:13)
console.log
info Starting the app on "emulator-5554"...
at Object.info (packages/cli-tools/src/logger.ts:20:13)
console.log
info Starting the app...
at Object.info (packages/cli-tools/src/logger.ts:20:13)
console.log
info Starting the app...
at Object.info (packages/cli-tools/src/logger.ts:20:13)
Test Suites: 2 passed, 2 total
Tests: 14 passed, 14 total
Snapshots: 0 total
Time: 0.747 s, estimated 1 s
Ran all test suites in 2 projects.
✨ Done in 1.79s.
mike@isabela:~/work/react-random/cli (fix-fully-specified-non-package-android-main-activity *) % |
|
Persistent windows-2019 node20 CI test failure appears unrelated |
|
@cortinico just a gentle ping on this, as the change is small (so maybe a satisfying review!) and I've backed it by a test now :-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
If you have a main activity that is not in package, and is fully specified, it used to work, but it regressed after adding auto-activity detection
If your launcher /
MAINactivity either specified by --main-activity or in AndroidManifest.xml is fully specified for example like "com.zoontek.rnbootsplash.RNBootSplashActivity" then it needs to launch for example "com.kullki.kscore.dev/com.zoontek.rnbootsplash.RNBootSplashActivity" not "com.kullki.kscore.dev/com.kullki.kscore.dev.com.zoontek.rnbootsplash.RNBootSplashActivity"So basically I added an "or" in the first conditional chunk that just uses the
mainActivityas-is, that will allow that if(!mainActivity.startsWith('.') && mainActivity.includes('.'))which I think captures the spirit of the previous code in this spot that was trying to build the component nameTest Plan:
I added a test case for the failure - when I run
yarn testbefore it fails only that test, and after, they all passI ran my app with the built
tryLaunchAppOnDevice.jsfile before and after the fix, it failed before the fix, it worked after the fixChecklist