Skip to content
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

feat: Run integration tests on full Apple CI matrix #1476

Merged
merged 22 commits into from
May 6, 2024

Conversation

jbelkins
Copy link
Contributor

@jbelkins jbelkins commented May 6, 2024

Issue #

#1477

Description of changes

Runs our integration test suite on all of the xcodebuild destinations that CI does: Mac plus iOS, tvOS sims.

  • To get AWS credentials into the simulators, an Xcode scheme is created for CI integration testing, and the scheme is fitted with a .xctestplan file. Since xctestplan is a JSON file, the jq command line utility is used to write the credentials into the file before testing. They are then set as environment variables for the test suite before running.

New/existing dependencies impact assessment, if applicable

No new dependencies were added to this change.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jbelkins jbelkins changed the title Run Apple integration tests on CI matrix using Xcode feat: Run integration tests on full Apple CI matrix May 6, 2024
@jbelkins jbelkins marked this pull request as ready for review May 6, 2024 06:05
# Insert the credentials into the .xctestplan file, write the modified JSON
# to a temp file, then move the temp over the original.
jq ".defaultOptions.environmentVariableEntries += [{\"key\": \"AWS_ACCESS_KEY_ID\", \"value\": $AKID_ESCAPED}, {\"key\": \"AWS_SECRET_ACCESS_KEY\", \"value\": $SECRET_ESCAPED}, {\"key\": \"AWS_DEFAULT_REGION\", \"value\": $REGION_ESCAPED}, {\"key\": \"AWS_SESSION_TOKEN\", \"value\": $TOKEN_ESCAPED}]" XCTestPlans/AWSIntegrationTestsOnCI.xctestplan > testplan.tmp
mv testplan.tmp XCTestPlans/AWSIntegrationTestsOnCI.xctestplan
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Above is the "secret sauce": to get AWS credentials into the Xcode simulators: use the jq command-line utility to insert the credentials into the scheme's .xctestplan file.

(.xctestplan is a JSON file used by Xcode to conbfigure tests, and jq is a CLI utility for reading, writing & updating JSON.)

Copy link
Contributor

@dayaffe dayaffe May 6, 2024

Choose a reason for hiding this comment

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

I don't believe jq comes pre-installed on Macs unless github actions makes it available, does this work when run on a github actions environment?

For a script I wrote to run on Amazon Linux I built-in a bash script method that checks if jq is installed and installs it if not, you may need to do something similar here

Copy link
Contributor

Choose a reason for hiding this comment

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

NVM I see it comes with github actions

@@ -0,0 +1,47 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is basically a scheme with default settings, that delegates all test configuration to the test plan at XCTestPlans/AWSIntegrationTestsOnCI.xctestplan.

@@ -0,0 +1,95 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file defines the test targets and the configuration to be used when running integration tests.

Copy link
Contributor Author

@jbelkins jbelkins May 6, 2024

Choose a reason for hiding this comment

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

This file is actually ignored by Git, and you have to "force" Git to commit changes to it with git add -f.

It must remain ignored, so that Git credentials aren't accidentally committed.

"defaultOptions" : {
"environmentVariableEntries" : [

]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The step defined in the Github workflow above will fill this empty JSON array with environment variable settings for the test run's AWS credentials.


]
},
"testTargets" : [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section of the file contains all the test targets to be run as part of integration tests.

Note that when we add a new integration test target to the repo, we will have to add it here to make it run on CI.

@@ -18,20 +18,36 @@ jobs:
strategy:
fail-fast: false
matrix:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The matrix used here for Apple platforms is copied verbatim out of https://github.com/awslabs/aws-sdk-swift/blob/main/.github/workflows/continuous-integration.yml .

Copy link
Contributor

@dayaffe dayaffe left a comment

Choose a reason for hiding this comment

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

Approved, this should find some good bugs

@jbelkins
Copy link
Contributor Author

jbelkins commented May 6, 2024

Merging this despite the failing integration tests; those are legitimate integration test failures exposed by the config changes in this PR.

Will address the root cause of those failures separately.

@jbelkins jbelkins merged commit d058045 into main May 6, 2024
24 checks passed
@jbelkins jbelkins deleted the jbe/integration_test_on_apple_platforms branch May 6, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants