Add Tools & Scripts that build testapps #41

sunmou99 · 2021-08-03T20:48:25Z

Tested locally with Unity 2017.4.37f1 and 2019.2.8f1.
firestore testapp uses Unity Test Framework, which requires at least Unity 2019.2.
Except for firestore in 2017.4.37f1 and iOS analytics testapps, the rest testapps build successfully with All platform: iOS,Android,Win64,OSXUniversal,Linux64.
For in-editor test, MacOS' Gatekeeper (which blocks untrusted binaries) will prevent playmode tests on MacOS.

Migrated Tools that automate the test. Tools are MenuScene, automated_testapp and etc., under pathscripts/gha/integration_test
Migrated Scripts that build testapps. Python scripts under path scripts/gha. Modified the scripts and configs for GitHub.
Updated the Unity Project setting.

To test locally:

Prerequisite:
Make sure you have Unity 2017.4.37f1 installed and activated.
For iOS, using XCode 10 by default,
For Android, using SDK tool 25.2.*, JAVA_HOME set to JDK8
(For Unity 2019.2.8f1, we need newer JDK, and Android SDK)
Restore secret: python scripts/gha/restore_secrets.py --passphrase ${passphrase}
Build testapps: python scripts/gha/build_testapps.py --t auth,database --u 2017.4.37f1 --p iOS,Android,Win64,OSXUniversal,Linux64 --force_latest_runtime

a-maurice

This is really big, and thus hard to actually review. I know most of this is being brought over from the internal versions of these files. Can you call out the major changes that had to be done, cause that will make it easier to note what is changing.

a-maurice · 2021-08-05T02:10:16Z

analytics/testapp/ProjectSettings/ProjectSettings.asset

@@ -166,7 +166,7 @@ PlayerSettings:
  buildNumber:
    iOS: 0
  AndroidBundleVersionCode: 1
-  AndroidMinSdkVersion: 16
+  AndroidMinSdkVersion: 26


I'm not a fan of this being updated. We should figure out what the issue here is, because 16 should be fine, and bumping this up just hides the problem.

Update the version for now to unblock the build process.
Shawn created a ticket for this problem, and VJ is working on it. Will revert the change once it problem solved.

To make sure we don't forget to revert it back, do you want to add a TODO in the code and link to the ticket?

We have a ticket. And talked with Shawn and VJ today, we are working on it now.

sunmou99 · 2021-08-05T16:35:34Z

This is really big, and thus hard to actually review. I know most of this is being brought over from the internal versions of these files. Can you call out the major changes that had to be done, cause that will make it easier to note what is changing.

build_testapps.py: copy tools that automate the test to our unity project.
build_testapps.json: updated app info (name & path), and deleted obsolete configs.
xcodebuild.py: build for unsigned iOS apps.

yifanyang

I skimmed through all the python code and left a few comments, but they look largely good to me. I hope that once I'm more familiar with the Games stacks, I'll be able to review more type of code.

yifanyang · 2021-08-05T21:23:47Z

analytics/testapp/ProjectSettings/ProjectSettings.asset

@@ -166,7 +166,7 @@ PlayerSettings:
  buildNumber:
    iOS: 0
  AndroidBundleVersionCode: 1
-  AndroidMinSdkVersion: 16
+  AndroidMinSdkVersion: 26


To make sure we don't forget to revert it back, do you want to add a TODO in the code and link to the ticket?

scripts/gha/integration_testing/automated_testapp/AutomatedTestRunner.cs

scripts/gha/integration_testing/entitlements/auth.entitlements

scripts/gha/integration_testing/entitlements/dynamic_links.entitlements

scripts/gha/integration_testing/entitlements/messaging.entitlements

yifanyang · 2021-08-05T22:02:26Z

scripts/gha/integration_testing/xcodebuild.py

+  for _, directories, _ in os.walk(iphone_build_dir):
+    for directory in directories:
+      if directory.endswith('.app'):


Would it be simpler if we use glob here?

Good to know! We use os.walk in a few other scripts. Maybe worthing have a separate PR solve them at once.

yifanyang · 2021-08-05T22:08:57Z

scripts/gha/integration_testing/xcodebuild.py

+        shutil.move(app_path, payload_path)
+        shutil.make_archive(
+            payload_path, "zip", root_dir=iphone_build_dir, base_dir="Payload")
+        shutil.move("%s.%s"%(payload_path, "zip"), ipa_path)


f-string appears to be the modern way for formatting strings. I think they are more readable than str.format() and % operator. It's up to you.

f-string is a new feature in python 3.6. Our script used to support python 2. But I think that it's worth doing this cleaning for the python scripts.

yifanyang · 2021-08-05T22:26:01Z

This is really big, and thus hard to actually review. I know most of this is being brought over from the internal versions of these files. Can you call out the major changes that had to be done, cause that will make it easier to note what is changing.

build_testapps.py: copy tools that automate the test to our unity project.
build_testapps.json: updated app info (name & path), and deleted obsolete configs.
xcodebuild.py: build for unsigned iOS apps.

I feel it will be clearer if this code change is structured as two separate pull requests:

one PR that consists of pure copy-pasting old code from google3
another PR that contains the real change that needs in-depth review

This would make the PR easier to review and faster to merge (go/small-cls). It's okay to me if it's hard to split this time, but maybe starting from next PR, let's be more conscious of the size of our PR. What do you think?

scripts/gha/build_testapps.py

scripts/gha/integration_testing/MenuScene/SceneLoader.cs

cynthiajoan

A few nit comments, this is really an epic PR, great job!

sunmou99 added 3 commits July 27, 2021 12:10

script to restore secret

e3b7e94

tools & scripts that build testapps locally

a80f347

Merge branch 'main' into feature/testapp-build-script

b96ed13

google-cla bot added the cla: yes label Aug 3, 2021

sunmou99 requested review from yifanyang, a-maurice and cynthiajoan August 3, 2021 20:49

sunmou99 added 2 commits August 3, 2021 15:17

fix firestore build

48d3dc7

update default Unity version

226c33c

a-maurice reviewed Aug 5, 2021

View reviewed changes

yifanyang previously approved these changes Aug 5, 2021

View reviewed changes

add newline at EOF

0c7c3df

sunmou99 dismissed yifanyang’s stale review via 0c7c3df August 5, 2021 23:30

sunmou99 requested review from a-maurice and yifanyang August 9, 2021 19:58

cynthiajoan reviewed Aug 11, 2021

View reviewed changes

scripts/gha/build_testapps.py Outdated Show resolved Hide resolved

cynthiajoan reviewed Aug 11, 2021

View reviewed changes

scripts/gha/integration_testing/MenuScene/SceneLoader.cs Show resolved Hide resolved

cynthiajoan reviewed Aug 11, 2021

View reviewed changes

scripts/gha/integration_testing/MenuScene/SceneLoader.cs Show resolved Hide resolved

cynthiajoan previously approved these changes Aug 11, 2021

View reviewed changes

changes based on comments

6bc1225

sunmou99 dismissed cynthiajoan’s stale review via 6bc1225 August 11, 2021 18:24

cynthiajoan approved these changes Aug 11, 2021

View reviewed changes

sunmou99 merged commit f948c75 into main Aug 11, 2021

sunmou99 deleted the feature/testapp-build-script branch August 11, 2021 18:39

sunmou99 changed the title ~~Tools & Scripts that build testapps~~ Add Tools & Scripts that build testapps Nov 29, 2021

firebase locked and limited conversation to collaborators Jun 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Tools & Scripts that build testapps #41

Add Tools & Scripts that build testapps #41

sunmou99 commented Aug 3, 2021 •

edited

Loading

a-maurice left a comment

a-maurice Aug 5, 2021

sunmou99 Aug 5, 2021

yifanyang Aug 5, 2021

sunmou99 Aug 5, 2021

sunmou99 commented Aug 5, 2021

yifanyang left a comment

yifanyang Aug 5, 2021

yifanyang Aug 5, 2021

sunmou99 Aug 5, 2021

yifanyang Aug 5, 2021

sunmou99 Aug 5, 2021

yifanyang commented Aug 5, 2021

cynthiajoan left a comment

Add Tools & Scripts that build testapps #41

Add Tools & Scripts that build testapps #41

Conversation

sunmou99 commented Aug 3, 2021 • edited Loading

a-maurice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunmou99 commented Aug 5, 2021

yifanyang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yifanyang commented Aug 5, 2021

cynthiajoan left a comment

Choose a reason for hiding this comment

sunmou99 commented Aug 3, 2021 •

edited

Loading