Chrobalt on-device test work for Android #728

isarkis · 2024-12-19T06:01:34Z

b/354062546

.github/actions/on_device_tests/action.yaml

.github/workflows/main.yaml

oxve · 2024-12-19T23:43:18Z

.github/actions/on_device_tests/action.yaml

+          --token ${GITHUB_TOKEN} \
+          ${DIMENSION:+"--dimension" "$DIMENSION"} \
+          ${ON_DEVICE_TEST_ATTEMPTS:+"--test_attempts" "$ON_DEVICE_TEST_ATTEMPTS"} \
+          --archive_path "${GCS_PATH}/artifacts.tar" \


This param is not used.

I was referring specifically to archive_path. This parameter is not used by in the ApkTests code path.

How are we going to pass GCS path to where APKs are stored? I think the intention is to use archive_path. Maybe @Libzu can confirm?

.github/actions/on_device_tests/action.yaml

cobalt/testing/android-arm/base_unittests_filter.json

isarkis · 2024-12-21T04:49:57Z

tools/on_device_tests_gateway.proto

+}
+
+// Working directory and command line arguments to be passed to the gateway.
+message OnDeviceTestsCommand {


Do we want to remove unused parameters from this proto message?

Yes, we're never going to use the client on main to trigger old Cobalt ODTs.

isarkis · 2024-12-21T04:51:38Z

tools/on_device_tests_gateway_client.py

+]
+
+# All test configs On-Device tests support
+_TEST_CONFIGS = [


I wonder how this is used, I don't see this being referenced anywhere.

isarkis · 2024-12-21T04:52:25Z

tools/on_device_tests_gateway_client.py

+    #'on-device-tests-gateway-service.on-device-tests.svc.cluster.local')
+    'localhost')
+#_ON_DEVICE_TESTS_GATEWAY_SERVICE_PORT = '50052'
+_ON_DEVICE_TESTS_GATEWAY_SERVICE_PORT = '12345'


Is this a left over from the local testing, I think the official port is 50052?

Yes, this line should be reverted and the lines above uncommented.

isarkis · 2024-12-21T04:53:36Z

.github/actions/on_device_tests/action.yaml

      run: |
-        echo "Nothing yet"
+        set -uxe
+        python3 -u tools/on_device_tests_gateway_client.py \


@oxve , I think some of the labels we removed (e.g. change number, pr number, etc.) might still be useful if we add them to each sponge invocation, WDYT?

Agreed, we should add those back and handle them appropriately in the odt gateway.

@oxve , I went ahead and pushed d6d7c0a (#728). Can you see if that looks complete?

@Libzu , once argument list is finalized, we'll need to update odt gateway client.

.github/workflows/main.yaml

isarkis · 2024-12-21T04:57:35Z

tools/on_device_tests_gateway_client.py

+  platform_json_file_data = _read_json_config(platform_json_file)
+  gtest_device = platform_json_file_data["gtest_device"]
+  gtest_lab = platform_json_file_data["gtest_lab"]
+  gtest_blaze_target = "//experimental/cobalt/chrobalt_poc:custom_chrobalt_unit_tests_" + gtest_device


We would have to change that, see cl/708442211

will do once cl/708442211 is submitted

isarkis · 2024-12-21T04:58:35Z

tools/on_device_tests_gateway_client.py

+  gtest_device = platform_json_file_data["gtest_device"]
+  gtest_lab = platform_json_file_data["gtest_lab"]
+  gtest_blaze_target = "//experimental/cobalt/chrobalt_poc:custom_chrobalt_unit_tests_" + gtest_device
+  default_gtest_filters = "GURL*"


I think default should be * or we shouldn't set filter at all.

.github/actions/on_device_tests/action.yaml

isarkis · 2024-12-21T05:01:11Z

tools/on_device_tests_gateway_client.py

+      if failing_tests_string:
+        gtest_filters += ":-" + failing_tests_string
+      #print(f"    gtest_filters = {gtest_filters}");
+    apk_test["apk_path"] = "/bigstore/yt-temp/" + gtest_target + "-debug.apk"


apk_path should be set from --archive_path.

How would apk_path look for the following?:
--archive_path gs://cobalt-actions-prod-test-artifacts/android_25.lts.1+/1622/android-arm_devel/artifacts.tar

--archive_path /bigstore/cobalt-actions-prod-test-artifacts/android_25.lts.1+/1622/android-arm_devel/

.github/actions/on_device_tests/action.yaml

      shell: bash
+    - name: Run ${{ env.SHARD_NAME }} Tests on ${{ matrix.platform }} Platform


tools/on_device_tests_gateway_pb2.py

tools/on_device_tests_gateway_pb2_grpc.py

.github/actions/on_device_tests/action.yaml

+          --token ${GITHUB_TOKEN} \
+          ${DIMENSION:+"--dimension" "$DIMENSION"} \
+          ${ON_DEVICE_TEST_ATTEMPTS:+"--test_attempts" "$ON_DEVICE_TEST_ATTEMPTS"} \
+          --archive_path "${GCS_PATH}/artifacts.tar" \


.github/actions/on_device_tests/action.yaml

+          trigger \
+      shell: bash
+    - name: Download ${{ matrix.platform }} Test Results
+      if: always() && env.TEST_TYPE == 'unit_test'


.github/actions/on_device_tests/action.yaml

+          --token ${GITHUB_TOKEN} \
+          ${DIMENSION:+"--dimension" "$DIMENSION"} \
+          ${ON_DEVICE_TEST_ATTEMPTS:+"--test_attempts" "$ON_DEVICE_TEST_ATTEMPTS"} \
+          --archive_path "${GCS_PATH}/artifacts.tar" \


oxve · 2024-12-21T01:45:11Z

tools/on_device_tests_gateway.proto

+}
+
+// Working directory and command line arguments to be passed to the gateway.
+message OnDeviceTestsCommand {


Can you remove the fields we're not going to use for chrobalt. Just keep the numbers on the fields we are using the same.

oxve · 2024-12-21T01:45:57Z

tools/on_device_tests_gateway_client.py

+_WORK_DIR = '/on_device_tests_gateway'
+_ON_DEVICE_TESTS_GATEWAY_SERVICE_HOST = (
+    #'on-device-tests-gateway-service.on-device-tests.svc.cluster.local')
+    'localhost')


tools/on_device_tests_gateway_pb2.py

tools/on_device_tests_gateway_pb2_grpc.py

.github/workflows/main.yaml

oxve · 2025-01-06T19:30:51Z

tools/on_device_tests_gateway.proto

+}
+
+// Working directory and command line arguments to be passed to the gateway.
+message OnDeviceTestsCommand {


Yes, we're never going to use the client on main to trigger old Cobalt ODTs.

oxve · 2025-01-06T19:33:36Z

tools/on_device_tests_gateway_client.py

+      failing_tests_string = ":".join(failing_tests_list)
+      #passing_tests_string = ":".join(passing_tests_list)
+      #if passing_tests_string:
+      #  gtest_filters += ":" + passing_tests_string


Remove commented out code?

oxve · 2025-01-06T19:35:41Z

tools/on_device_tests_gateway_client.py

+      print(f"    gtest_filters = {gtest_filters}");
+
+    #apk_test["apk_path"] = "/bigstore/yt-temp/" + gtest_target + "-debug.apk"
+    apk_test["apk_path"] = "/bigstore/yt-temp/base_unittests-debug.apk"


Add a TODO?

tools/on_device_tests_gateway_client.py

tools/on_device_tests_gateway_pb2_grpc.py

tools/on_device_tests_gateway_pb2.py

isarkis · 2025-01-07T20:03:00Z

.github/actions/on_device_tests/action.yaml

+      shell: bash
+    - name: Generate gRPC files
+      run: |
+        python -m grpc_tools.protoc -Itools/ --python_out=cobalt/tools/ --grpc_python_out=cobalt/tools/ cobalt/tools/on_device_tests_gateway.proto


I am not sure if this relative path is correct, the absolute path is ${GITHUB_WORKSPACE}/src/cobalt/tools

+1 to using absolute path here

oxve · 2025-01-07T20:06:32Z

.github/config/android-arm.json

@@ -21,6 +21,8 @@
        "sql_unittests",
        "url_unittests"
    ],
+    "gtest_device": "sabrina",
+    "gtest_lab": "maneki",


These should be nested under a test_dimensions key.

Added test_dimension key, @Libzu please make a note of it.

.github/workflows/main.yaml

b/354062546

oxve reviewed Dec 19, 2024

View reviewed changes

Libzu requested a review from a team as a code owner December 20, 2024 23:37

Libzu requested review from briantting and removed request for a team December 20, 2024 23:37

isarkis commented Dec 21, 2024

View reviewed changes

isarkis commented Jan 3, 2025

View reviewed changes

oxve reviewed Jan 6, 2025

View reviewed changes

isarkis commented Jan 7, 2025

View reviewed changes

tools/on_device_tests_gateway_pb2_grpc.py Outdated Show resolved Hide resolved

isarkis commented Jan 7, 2025

View reviewed changes

tools/on_device_tests_gateway_pb2.py Outdated Show resolved Hide resolved

isarkis commented Jan 7, 2025

View reviewed changes

oxve reviewed Jan 7, 2025

View reviewed changes

isarkis force-pushed the feature/chrobalt_odt branch from 456d4de to 8824e29 Compare January 10, 2025 00:03

isarkis and others added 14 commits January 9, 2025 16:04

Chrobalt on-device test work for Android

af1ffa2

b/354062546

Update params passed to ODT gateway

0923ffc

ODT Client code

ffd6059

Incremental commit based on review comments

73d51b6

new changes for odt gateway

99cc41d

additional changes to odt gateway code

46e1702

changing --archive_path in action.yaml

cc72216

Additional changes for odt gateway

11759a5

Add additional argument for ODT gateway client

14f963f

Additional improvements for odt gateway code

3ecc7aa

removing autogenerated files from pr

a93365c

moving odt gateway code from tools folder to cobalt/tools

57420eb

Introduce "test_dimension" key

a7a759a

Log and XML file processing

0b8c742

isarkis force-pushed the chrobalt-odt-dev branch from 673451f to 0b8c742 Compare January 10, 2025 00:09

		shell: bash
		- name: Run ${{ env.SHARD_NAME }} Tests on ${{ matrix.platform }} Platform

Chrobalt on-device test work for Android #728

Are you sure you want to change the base?

Chrobalt on-device test work for Android #728

Conversation

isarkis commented Dec 19, 2024

Choose a reason for hiding this comment

This comment was marked as outdated.

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

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

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

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

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

Choose a reason for hiding this comment