Skip to content

Conversation

@anonymous-akorn
Copy link
Contributor

This contains the following high level changes:

  1. Adding the setup-msbuild step, as used by the workflow to build the SDK.
  2. Working around an issue with the gCloud SDK on Windows, by setting an environment variable (CLOUDSDK_PYTHON)
  3. Fixing some issues with using gradle on Windows: build_testapps.py patches a path into gradle.settings, but gradle does not handle Windows path separators. As part of this, all paths provided through flags are passed through a function to normalize them and convert relative paths to full paths. Additionally, we now use gradlew.bat on Windows, instead of gradlew.
  4. Fixed an issue with gcloud and gsutil not being found on Windows. subprocess.run does not check the system path, so e.g. subprocess.run(["gcloud"]) was failing with a FileNotFoundError, despite gcloud being in the environment's path. This was fixed by using full paths to the Google Cloud command line tools.
This modifies the integration test workflow to use the test_lab.py script to send testapps to Firebase Test Lab, and validates the results. This requires a GCP service account. To authorize its use, we need a key file, which is added to the encrypted files in gha-encrypted. Note that this is not mirrored in google3, since policy prohibits this kind of data from being placed in google3. Instead, the unencrypted file is found in valentine, its valentine id mentioned in the test_lab.py file. To install the Cloud SDK, we use GCP's setup-gcloud action. There's no particularly convenient way to automate the installation of the Cloud SDK so we use this as a (possibly temporary) convenience. It may be replaced by custom logic so that we can simplify local setup.
Currently, build_testapps.py will terminate if any subprocess call times out. Now, timeouts during the building or running of an integration test will be caught and processed as failures, to avoid blocking other integration tests. This is preferred behaviour since e.g. analytics failing should not block remote config from building or running. Details: Previously, we were catching subprocess.CalledProcessError, but missing subprocess.TimeoutExpired. These are both instances of the base class subprocess.SubprocessError, so we catch that instead.
By default, a step will be skipped if any previous step fails. We override this behaviour for the Android test steps by running on success or failure (but not cancellation). This ensures that in the case where we have 1+ failed builds and 1+ successfuly builds, we still run the tests for the successfully built integration tests. In the case where all fail to build, or the failure comes from a prior step (so the build step is skipped), the test step will still fail fast. The test_lab.py script is changed to look for testapps before authorizing the GCS service account to fail faster and with less noise.
Instead of explicitly checking for success or failure, just check that we're not cancelled. More readable, and will allow it to run under new statuses that Github may add in the future.
This adds msbuild, needed to build the Android SDK on Windows.
gradlew is for MacOS/Linux, gradlew.bat is for Windows.
This doesn't work in Windows.
To ensure paths are correct on windows, this normalizes all paths supplied through flags. It also turns relative paths to absolute paths, as previously only full paths would work as flags.
Gradle will treat the backslashes as escapes, causing the path to get mangled in settings.gradle. On Windows, replace those backslashes with forward slashes.
Previousy commit tried to replace slashes, but forgot to reassign the modified string back to the variable it was supposed to modify.
When running on Windows, an error occured due to a file in an intermediate directory still being open in another process when trying to delete the directory. The details are not clear as to why: my best guess is that the process opened by gradle still had one of the files it generated open, suggesting a (garbage collection?) race of some sort. Since this occurs inconsistently, we can handle the error and raise a warning.
The Google Cloud SDK's command line tools have an issue on Windows. google-github-actions/setup-gcloud#100 This causes steps using the command line tools to fail. This adds a workaround, which involves setting an environment variable. This was implemented in a way that will make it easy to revert once the issue is fixed.
On GHA the the tool is complaining of a file not found, though it's not clear which one, only that it's happening during the gcs authorization check. This puts paths through abspath and expanduser to ensure there aren't any issues with how they're formatted. It also adds check to ensure the Google Cloud command line tools are present, and the key file exists. This will make the cause of issues more obvious.
Python's subprocess.run does not check the PATH, which was leading to gcloud and gsutil not being found. This fixes it, by passing the full path as found by shutil.which.
This step was getting skipped if the build step failed, which caused the test step to fail on the integration tests that were built.
DellaBitta
DellaBitta previously approved these changes Oct 7, 2020
Copy link
Contributor

@DellaBitta DellaBitta left a comment

Choose a reason for hiding this comment

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

Approved with a nit.

NDK_ROOT: '/tmp/android-ndk-r16b'
run: |
python scripts/gha/build_testapps.py --t ${{ github.event.inputs.apis }} --p ${{ matrix.target_platform }} --output_directory ${{ github.workspace }} --use_vcpkg --execute_desktop_testapp --noadd_timestamp
# Workaround for https://github.com/GoogleCloudPlatform/github-actions/issues/100
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: add a new line above

@anonymous-akorn anonymous-akorn merged commit 0a6f42e into dev Oct 7, 2020
@anonymous-akorn anonymous-akorn deleted the feature/ak-integration-tests branch October 7, 2020 19:51
@firebase firebase locked and limited conversation to collaborators Nov 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

2 participants