-   Notifications  
You must be signed in to change notification settings  - Fork 125
 
Add Windows integration tests to workflow #156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
   Merged  
     Merged  
  Conversation
   This file contains hidden or 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   
     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.
*_device -> *_model
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.
…ebase/firebase-cpp-sdk into feature/ak-integration-tests
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.
…ebase/firebase-cpp-sdk into feature/ak-integration-tests
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  previously approved these changes   Oct 7, 2020    
  There was a problem hiding this 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 | 
There was a problem hiding this comment.
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
  DellaBitta  approved these changes   Oct 7, 2020    
   Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in. 
  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.    
 
This contains the following high level changes: