Migrate build and test workflow to GitHub Actions #509

timkimadobe · 2024-11-15T21:48:40Z

Questions for reviewers

Is removing the custom destination for test results acceptable? Or should we also make copies of the test results in the same custom destination as before?
Is removing the version rules acceptable? I wasn't sure of the frequency/usefulness of these rules being run locally

Description

This PR updates the Makfile test rules to use configurable device name and OS, and removes some local scripts that are handled by the new reusable workflows.

The configurable device name and OS also enable compatibility with GitHub Actions matrix configurations:
Example run (with matrix using both iOS and tvOS paths): https://github.com/timkimadobe/aepsdk-edge-ios/actions/runs/11695962501

How the PR checks look: trigger change timkimadobe/aepsdk-edge-ios#45

`Makefile`

New computed properties IOS_DESTINATION and TVOS_DESTINATION used in the xcodebuild test commands, which rely on the new input values:

IOS_DEVICE_NAME - defaults to "iPhone 15"
IOS_VERSION - defaults to nothing (same behavior as before)
TVOS_DEVICE_NAME - defaults to "Apple TV"
TVOS_VERSION - defaults to nothing (same behavior as before)

Breaking changes

The custom -derivedDataPath and -resultBundlePath for test rules has been removed so that test coverage is placed in the default location. This is to facilitate compatibility with the Codecov action's Xcode plugin test result coverage search behavior: iOS CircleCI to GitHub Actions reusable workflow conversion aepsdk-commons#67 (comment)
- The test result cleanup pre-step for each test rule has been updated to reflect this new location (whose path is randomly generated)
- Ex default result path: /Users/runner/Library/Developer/Xcode/DerivedData/AEPEdge-byotfosbfuyloqdajycdwzyadpcv/Logs/Test/Test-UnitTests-2024.11.15_13-51-24--0800.xcresult
The version related rules check-version and test-version-update have been removed

Example usage

make unit-test-ios IOS_DEVICE_NAME="iPhone 16"

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

This supports command line args updating the device and OS to test with, and also enables CI matrix support

kevinlind · 2024-11-16T00:56:00Z

The -resultBundlePath option when running the tests is used by the CircleCI config to upload the results to CodeCov. That is currently disabled, but you should still update config.yml with the new path to the test results, or add back the -resultBundlePath option.

Removing the version scripts is great. Seems like that was missed when adding the version update GH Action script.

timkimadobe · 2024-11-19T22:20:07Z

The -resultBundlePath option when running the tests is used by the CircleCI config to upload the results to CodeCov. That is currently disabled, but you should still update config.yml with the new path to the test results, or add back the -resultBundlePath option.

Removing the version scripts is great. Seems like that was missed when adding the version update GH Action script.

Thanks @kevinlind, that's a great point! For some additional context, I was planning on also migrating the Edge iOS repo to use the reusable workflow build and test for PR checks in aepsdk-commons:

iOS CircleCI to GitHub Actions reusable workflow conversion aepsdk-commons#67 (comment)

Basically, the Codecov action's Xcode plugin which handles creating test coverage looks in the default directory xcodebuild uses and this change was meant to align with that

kevinlind · 2024-11-20T05:58:37Z

.github/workflows/build-and-test.yml

+      run-test-ios-unit: true
+      run-test-ios-functional: true
+      run-test-ios-integration: true
+      run-test-tvos-unit: true
+      run-test-tvos-functional: true
+      run-build-xcframework-and-app: true


Is it possible to only run some of these tests on specific branches? For example, we run the integration tests and build the xcframework and app just on the staging and main branches to reduce the time it takes to build the dev and pr branches. Depending on how long it takes to run these jobs it may not be necessary to exclude them, however.

@kevinlind thanks for your help in updating the actions rules for the repo - after running the new build and test workflow which runs all of these in parallel, it looks like integration tests (3m 44s) and building the xcframework and app (3m 36s) take less time than the functional tests (9m+)

In that case, I think it should be ok to keep them in the regular checks? What do you think

We will have to make changes to add branch if we cause or run into the issue of unavailability of runners. Even if we don't have the branch based rule today, it will be a good practice to have it ready in case we need it.

Should we wait to implement that if/when we encounter the unavailability? I could split the usage of the workflow into two pieces - the branch dependent and non-dependent jobs, which should be a quick change

ok, then let's see how this plays out.

codecov · 2024-11-20T21:22:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.97%. Comparing base (3d22a85) to head (9b411f2).
Report is 102 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #509      +/-   ##
==========================================
- Coverage   96.73%   95.97%   -0.77%     
==========================================
  Files          27       28       +1     
  Lines        1653     1463     -190     
==========================================
- Hits         1599     1404     -195     
- Misses         54       59       +5

Rename build and test job Revert change to README

…style

addb · 2024-11-22T01:46:57Z

Tests/FunctionalTests/AEPEdgeFunctionalTests.swift

@@ -22,7 +22,7 @@ import XCTest

 /// End-to-end testing for the AEPEdge public APIs
 class AEPEdgeFunctionalTests: TestBase, AnyCodableAsserts {
-    private let TIMEOUT_SEC: TimeInterval = 2
+    private let TIMEOUT_SEC: TimeInterval = 10
    private let LONGER_TIMEOUT_SEC: TimeInterval = 10


We have LONGER_TIMEOUT_SEC which is 10 seconds. We can move this into common constants file (FuntionalTestConstants.swift) if this applies to all the integration tests, and let's use only one time and update it to 10 sec if that makes our tests more stable.

…to new default

…lows (#512) * Make EdgeEventHandles and completionHandlers interactions thread safe (#481) * Make EdgeEventHandles and completionHandlers interactions thread safe * Minor refactor * Update queue name * Update AEPTestUtils to 5.2.2 * Update GitHub Action workflows to use aepsdk-commons (#507) * Migrate release and update versions workflows to use reusable workflows (testing configuration) * Revert naming update * Update input types * Update input order * Revert testing config * Add code docs on how to update the workflow tag * Update release workflow to use env var and ternary * Temp use testing config * Test github actions style ternary * Update input descriptions and name + usages Update workflow call input variable names * Revert testing config * Add permissions to workflows * Testing new version validation process in release * Update to testing config * Test version script updates * Fix pattern type override missing * Revert back to production configs * Remove default from mandatory fields to trigger error on empty submit * Apply testing config * Update tags * Apply testing config * Remove passing workflow_tag * Remove outdated comment * Update with production tags * Update to full production config * Revert "Make EdgeEventHandles and completionHandlers interactions thread safe (#481)" (#510) This reverts commit 607f927. * Migrate build and test workflow to GitHub Actions (#509) * Remove local version related rules and scripts * Test removing resultBundlePath flag * TEST: versions-update branch compatibility with original versions usage * Apply prod config * Update Makefile test rules for configurable device and OS This supports command line args updating the device and OS to test with, and also enables CI matrix support * Replace CircleCI with GitHub Actions reusable workflow * Add license headers to workflow files * Apply prod config * Trigger change * Remove @ from Makefile rule for integration test Rename build and test job Revert change to README * Increase timeout from 2 to 5 sec * Fix double expectation for first event * Add MobileCore resetSDK to teardown * Add reset test expectations to tear down and increase timeout * Increase timeout duration to 10 sec default * Add timeout to all network request assertions * Update order of files * Rename upstream integration test job to align with naming convention style * Create integration test rule name that aligns with reusable workflow * Use codecov flag based on new default * Create new functional test constants and route functional test usage to new default * Add default timeout to network request assertions * Functional test stability (#513) * Pod lock with 1.16.2 * Apply default timeout to all network request gets --------- Co-authored-by: Arjun Bhadra <[email protected]>

timkimadobe added 5 commits November 8, 2024 15:19

Remove local version related rules and scripts

8daf5dd

Test removing resultBundlePath flag

4f1405b

TEST: versions-update branch compatibility with original versions usage

daa7b9e

Apply prod config

695ac28

Update Makefile test rules for configurable device and OS

3e6328d

This supports command line args updating the device and OS to test with, and also enables CI matrix support

timkimadobe requested review from addb and kevinlind November 15, 2024 22:01

addb approved these changes Nov 15, 2024

View reviewed changes

Replace CircleCI with GitHub Actions reusable workflow

8342c7d

timkimadobe added 3 commits November 19, 2024 16:50

Add license headers to workflow files

c091376

Apply prod config

a38f17e

Merge branch 'dev' into workflow-updates

ce35fb6

kevinlind reviewed Nov 20, 2024

View reviewed changes

Trigger change

716936b

timkimadobe added 4 commits November 20, 2024 13:23

Remove @ from Makefile rule for integration test

9d4d5be

Rename build and test job Revert change to README

Increase timeout from 2 to 5 sec

88a48d6

Fix double expectation for first event

b78103b

Add MobileCore resetSDK to teardown

0d7e430

timkimadobe changed the title ~~Update Makefile test rules for configurable device and OS~~ Migrate build and test workflow to GitHub Actions Nov 20, 2024

timkimadobe added 7 commits November 20, 2024 14:28

Add reset test expectations to tear down and increase timeout

1443f22

Increase timeout duration to 10 sec default

b4a9df3

Add timeout to all network request assertions

d5e5fb3

Update order of files

84cbcc1

Rename upstream integration test job to align with naming convention …

a17b0ab

…style

Create integration test rule name that aligns with reusable workflow

ecff2d0

Use codecov flag based on new default

6244c4a

addb reviewed Nov 22, 2024

View reviewed changes

Create new functional test constants and route functional test usage …

6cf2f82

…to new default

addb approved these changes Nov 22, 2024

View reviewed changes

kevinlind approved these changes Nov 22, 2024

View reviewed changes

Add default timeout to network request assertions

9b411f2

timkimadobe merged commit 3538584 into adobe:dev Nov 25, 2024
17 of 18 checks passed

timkimadobe mentioned this pull request Nov 25, 2024

Dev -> Staging [NON-RELEASE] Migrate to GitHub Actions reusable workflows #512

Merged

10 tasks

timkimadobe mentioned this pull request Nov 26, 2024

staging -> main [NON-RELEASE] Migrate to GitHub Actions reusable workflows #514

Merged

10 tasks

timkimadobe deleted the workflow-updates branch December 5, 2024 02:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate build and test workflow to GitHub Actions #509

Migrate build and test workflow to GitHub Actions #509

timkimadobe commented Nov 15, 2024 •

edited

Loading

kevinlind commented Nov 16, 2024

timkimadobe commented Nov 19, 2024

kevinlind Nov 20, 2024

timkimadobe Nov 20, 2024

addb Nov 22, 2024

timkimadobe Nov 22, 2024

addb Nov 22, 2024

codecov bot commented Nov 20, 2024 •

edited

Loading

addb Nov 22, 2024 •

edited

Loading

Migrate build and test workflow to GitHub Actions #509

Migrate build and test workflow to GitHub Actions #509

Conversation

timkimadobe commented Nov 15, 2024 • edited Loading

Questions for reviewers

Description

Makefile

Example usage

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

kevinlind commented Nov 16, 2024

timkimadobe commented Nov 19, 2024

kevinlind Nov 20, 2024

Choose a reason for hiding this comment

timkimadobe Nov 20, 2024

Choose a reason for hiding this comment

addb Nov 22, 2024

Choose a reason for hiding this comment

timkimadobe Nov 22, 2024

Choose a reason for hiding this comment

addb Nov 22, 2024

Choose a reason for hiding this comment

codecov bot commented Nov 20, 2024 • edited Loading

Codecov Report

addb Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

timkimadobe commented Nov 15, 2024 •

edited

Loading

`Makefile`

codecov bot commented Nov 20, 2024 •

edited

Loading

addb Nov 22, 2024 •

edited

Loading