Implement a bash script needed to configure simulators for App development #29061

shubham1206agra · 2023-10-08T14:09:30Z

Details

Fixed Issues

$ #28910
PROPOSAL: #28910 (comment)

Tests

Note -

Please close all the emulators of both android and ios cause this script will close all the emulators at the finish.
Please test this flow with Add https to webpack dev server #28422
Please run this flow on physical devices with extreme caution if you want to run (not recommended)

Pre-requisite for Android flow

Open any emulator using Android Studio
Use adb push "$(mkcert -CAROOT)/rootCA.pem" /storage/emulated/0/Download/ to push certificate to install in Download folder.
Install the certificate as CA certificate from the settings.
Close the emulator.

Note - If you want to run app on https://127.0.0.1:8082, then just install the certificate and use adb reverse tcp:8082 tcp:8082 on every startup.

Android Flow

Run npm run setupNewDotWebForEmulators android
Select the emulator you want to run if prompted. (If single emulator is available, then it will open automatically)
Let the script execute till the message 🎉 Done!.
Check the emulator is closed.

Note - If you want to run app on https://new.expensify.com.dev:8082, then just do the Android flow and use npm run startAndroidEmulator to start the Android Emulator every time (It will configure the emulator).

Possible Scenario:

It may run the second flow on a new device, then to check first flow just run the flow again and select the same device.
It may fail to root with error adbd cannot run as root in production builds, then it will point to https://stackoverflow.com/a/45668555 in the console.

iOS Flow

Run npm run setupNewDotWebForEmulators ios
Select the emulator you want to run if prompted. (If single emulator is available, then it will open automatically)
Let the script execute till the message 🎉 Done!.
Check the emulator is closed.

All Flow

Run npm run setupNewDotWebForEmulators all or npm run setupNewDotWebForEmulators
Check if the iOS flow runs first and then Android flow runs.
Let the script execute till the message 🎉 Done!.
Check the emulator is closed.

Verify that script works as expected

QA Steps

NO QA

PR Author Checklist

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

melvin-bot · 2023-10-08T14:09:35Z

@mananjadhav Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

mananjadhav · 2023-10-09T12:57:40Z

Please run this flow on physical devices with extreme caution if you want to run (not recommended)

@shubham1206agra Can you clarify what you mean by this? Let's highlight the impact if any. And is it not required or not recommended?

mananjadhav · 2023-10-09T12:58:20Z

@shubham1206agra I don't see any screencasts in the author list?

scripts/select-device.sh

shubham1206agra · 2023-10-09T12:58:44Z

Idk
But this script is strictly for emulators
That's why this warning

scripts/select-device.sh

mananjadhav · 2023-10-09T13:00:08Z

scripts/setup-newdot-web-emulators.sh

+setup_ios()
+{
+ select_device_ios
+ sleep 5


why 5 seconds?

Just to give time to start the device

shubham1206agra · 2023-10-09T13:00:24Z

@shubham1206agra I don't see any screencasts in the author list?

Since this is a script
What do you want as screencasts?

mananjadhav · 2023-10-09T13:01:13Z

scripts/shellUtils.sh

-declare -r BLUE=$'\e[1;34m'
-declare -r TITLE=$'\e[1;4;34m'
-declare -r RESET=$'\e[0m'
+# Check if GREEN has already been defined


why do we need to change this?

Cause this is causing errors
scripts/shellUtils.sh: line 3: declare: GREEN: readonly variable

scripts/setup-newdot-web-emulators.sh

mananjadhav · 2023-10-09T13:01:57Z

scripts/setup-newdot-web-emulators.sh

+if [ "$platform" = "ios" ] || [ "$platform" = "all" ]; then
+ setup_ios || { 
+ error "Failed to setup iOS simulator"
+ exit 1 


why exit if ios fails? especially if $platform == 'all' ?

So that the user knows immediately that the flow failed. Instead of thinking that the flow passed.

mananjadhav · 2023-10-09T13:02:27Z

scripts/setup-newdot-web-emulators.sh

+ sleep 2
+ adb disable-verity
+ adb reboot
+ sleep 30


how do we come up with these numbers like 2 seconds, 30 seconds? Is there a way to avoid so many sleep commands?

Nope
You can't reduce the number of sleep command unfortunately

mananjadhav · 2023-10-09T13:03:19Z

package.json

@@ -8,6 +8,7 @@
 "private": true,
 "scripts": {
 "configure-mapbox": "scripts/setup-mapbox-sdk-walkthrough.sh",
+ "setupNewDotWebForEmulators": "scripts/setup-newdot-web-emulators.sh",


Are these supposed to be a part of prebuild? or atleast added to the README? @hayata-suenaga ?

I think README.md will be updated here
#28422

@shubham1206agra can you check the current README change in that PR and put a comment in that PR on additional instruction we need to add because of these new scripts if there are any?

mananjadhav · 2023-10-09T13:05:35Z

@shubham1206agra I don't see any screencasts in the author list?

Since this is a script What do you want as screencasts?

Once the setup is done, to verify the setup works as expected. Basically running the apps on the emulators.

shubham1206agra · 2023-10-09T13:09:12Z

Can't test this as this requires another PR to be merged and maybe some setup

@hayata-suenaga please tell me what I should do here?

hayata-suenaga · 2023-10-09T13:34:35Z

@shubham1206agra for the PR that adds the setup for https support, you can bring the changes from that PR temporarily into your PR and check that you can access the app though new.expensify.com.dev on both iOS and Android emulators.

@mananjadhav please do the same

This is the link to the PR

I know this process is very cumbersome, but we have to ensure that this works before merging 🙇

hayata-suenaga · 2023-10-09T13:35:32Z

@shubham1206agra as @mananjadhav suggested, please edit README file to add instructions for running the script before accessing the dev App on browsers on emulators.

shubham1206agra · 2023-10-09T13:41:59Z

@shubham1206agra as @mananjadhav suggested, please edit README file to add instructions for running the script before accessing the dev App on browsers on emulators.

I will send the updated instruction to https://github.com/Expensify/App/pull/28422/files. I think if we should put this there only

shubham1206agra · 2023-10-09T13:48:11Z

@shubham1206agra for the PR that adds the setup for https support, you can bring the changes from that PR temporarily into your PR and check that you can access the app though new.expensify.com.dev on both iOS and Android emulators.

@mananjadhav please do the same

This is the link to the PR

I know this process is very cumbersome, but we have to ensure that this works before merging 🙇

How to do this?
Can you tell me?

shubham1206agra · 2023-10-09T14:17:33Z

After accepting certificate

shubham1206agra · 2023-10-09T14:18:16Z

BTW https://new.expensify.com.dev:8082/ not working
https://127.0.0.1:8082/ is working

shubham1206agra · 2023-10-11T14:29:11Z

Use adb push "$(mkcert -CAROOT)/rootCA.pem" /storage/emulated/0/Download/ to push certificate to install in Download folder.

hayata-suenaga · 2023-10-12T00:32:42Z

@shubham1206agra thank you for the explanation 😄 So did you make necessary changes to the PR so that Android works now?

Is this good for testing?

melvin-bot · 2023-10-12T02:50:39Z

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

shubham1206agra · 2023-10-12T04:44:45Z

@hayata-suenaga @mananjadhav You can start the review now

akinwale · 2023-10-12T07:50:52Z

@shubham1206agra I get the following error when trying to run the iOS flow after selecting the simulator: iPhone 14 Pro Max Simulator Version: 16.2

🎉 Launching iPhone 14 Pro Max Simulator Version: 16.2
Installing certificates on iOS simulator
An error was encountered processing the command (domain=NSCocoaErrorDomain, code=260):
Simulator device returned an error for the requested operation.
The file doesn’t exist.
Underlying error (domain=NSCocoaErrorDomain, code=260):
	The file “rootCA.pem” couldn’t be opened because there is no such file.
	The file doesn’t exist.
🎉 Done!

akinwale · 2023-10-12T08:02:20Z

@shubham1206agra Apologies for the false alarm. Looks like I had to run the steps from the other PR first. I thought only merging the PR was required. I don't have any more errors.

akinwale · 2023-10-12T09:09:31Z

My observations after testing.

Scripts
Scripts run OK for both Android and iOS.

iOS
iOS works ok after running the scripts testing with the following URLs: https://127.0.0.1:8082, https://localhost:8082 and https://new.expensify.com.dev:8082

Android
The following needs to be done to get SSL to work every time the emulator is started after running the scripts and prerequisites.

Run adb reverse tcp:8082 tcp:8082. This will ensure that the URL https://10.0.2.2:8082 works.
Start the emulator with the -writable-system flag to ensure that the URL https://new.expensify.com.dev:8082 works. This is due to the fact that the /system/etc/hosts file always returns to the default for the emulator if this flag is not used.

@shubham1206agra Could you please confirm that this is the expected behaviour and then I'll start working on the reviewer checklist? Thanks.

cc @hayata-suenaga

shubham1206agra · 2023-10-12T09:12:54Z

For Android point 1 please use https://127.0.0.1:8082
Everything else is fine

akinwale · 2023-10-12T09:20:22Z

For Android point 1 please use https://127.0.0.1:8082 Everything else is fine

Ok, that makes sense. Confirming that both https://127.0.0.1:8082 and https://10.0.2.2:8082 work as expected.

akinwale · 2023-10-12T09:23:42Z

@shubham1206agra Could you please add screenshots to the OP? This is a requirement for all contributor PRs. You can simply take screenshots of the login screen for all platforms. Thanks.

shubham1206agra · 2023-10-12T09:26:06Z

Not required for all platforms. Only mweb is required for which screenshot has been updated.

akinwale · 2023-10-12T09:31:56Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android: Native
- Android: mWeb Chrome
- iOS: Native
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

akinwale

LGTM.

Tests well.

hayata-suenaga · 2023-10-12T12:58:15Z

This PR adds a bash script. This PR can be deployed before the https change.

OSBotify · 2023-10-12T13:06:59Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2023-10-13T21:12:52Z

🚀 Deployed to staging by https://github.com/hayata-suenaga in version: 1.3.84-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2023-10-16T21:13:44Z

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.84-10 🚀

platform	result
🤖 android 🤖	skipped 🚫
🖥 desktop 🖥	skipped 🚫
🍎 iOS 🍎	skipped 🚫
🕸 web 🕸	skipped 🚫

OSBotify · 2023-10-16T21:50:04Z

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.84-10 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2023-10-16T21:53:00Z

🚀 Deployed to staging by https://github.com/hayata-suenaga in version: 1.3.85-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2023-10-17T23:17:45Z

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.85-4 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

made setupNewDotWebForEmulators script

b0afc32

shubham1206agra requested a review from a team as a code owner October 8, 2023 14:09

melvin-bot bot requested review from mananjadhav and removed request for a team October 8, 2023 14:09

shubham1206agra added 2 commits October 8, 2023 20:18

fix lint

b02536a

fix flow

7c7c6cf