Update with latest `dev` changes by timkimadobe · Pull Request #364 · adobe/aepsdk-edge-ios

timkimadobe · 2023-07-11T21:34:52Z

Description

This PR covers:

Bringing latest dev changes into the integration test feature branch
Updating dev changes to use new testing framework and APIs
~~Updating some tests to have greater timeout tolerances for setup/teardown process~~
Updating the build scripts to separate functional and unit test runs into separate job steps
- This is because in the CI job, occasionally the unit test build process would be interleaved with functional test case runs which was causing errors in test setup, state, and/or teardown. By separating them into distinct commands, the unit test build waits until the functional test run is complete, preventing this class of error.
- Note: item 3 was rolled back after applying the fix from 4
- This should also address most of the test case issues described in Fix flaky tests #365
Adding mock network service reset in the per-test-case teardown for every functional test class that uses the mock network service

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.

* Remove deprecated class XDMFormatters * Update timestamp field in MobileSDKCommerceSchema in tutorial apps

…nsion version Bump to 4.0.0 (adobe#344) * Update Min supported version to iOS 11 and tvOS 11. Bump major extension version. * Clean Makefile and add steps to generate xcframeworks * Update Github release workflow to add XCframework with the release * Add ci steps back in Makefile * Fix Github workflow steps * Updates based on reviews * More review updates * Update README * Add version to the release artifact * Update the getting started doc * Add test command in Makefile to run ios and tvos tests * Update makefile * Add commands to remove exisiting coverage reports before running the tests * Update Makefile * Add usage sample in comment for check-version command * Update all extensions to version 4.x * Remove empty marketing versions * Update podspec version to 4.0.0

* Update Podfile to use Core staging and Edge* dev branches * Update SwiftLint version 0.52.0 * Fix format after SwiftLint update

* Update Podfile to use Core/Services production version * Update podfile for Tutorial apps

Merge to Staging for 4.0.0 release

Staging -> Main v4.0.0 release

Update dependencies to prod and links

downmerge main -> dev

* Code changes for chaining parent event to network response events * Add functional tests for event chaining * Update method comments, rename parameter to 'parentRequestEvent' * Rename variable 'requestParentEvent' to 'parentRequestEvent'

# Conflicts: # Makefile # Podfile.lock # README.md # Tests/FunctionalTests/NetworkResponseHandlerFunctionalTests.swift

codecov · 2023-07-11T23:31:10Z

Codecov Report

Merging #364 (8fe73a8) into feature/upstream-integration-tests (d5e42ab) will increase coverage by 0.19%.
The diff coverage is 100.00%.

@@                          Coverage Diff                           @@
##           feature/upstream-integration-tests     #364      +/-   ##
======================================================================
+ Coverage                               96.54%   96.73%   +0.19%     
======================================================================
  Files                                      28       27       -1     
  Lines                                    1649     1653       +4     
======================================================================
+ Hits                                     1592     1599       +7     
+ Misses                                     57       54       -3

emdobrin · 2023-07-13T18:33:56Z

        // Wait .2 seconds in case there are unexpected events that were in the dispatch process during cleanup
-        usleep(200000)
+//        usleep(200000)
+        sleep(1)


this change will increase the test run time with 1 sec times no of tests, what are those events that are blocking for so long before the cleanup?

I was thinking this would catch the failure case from #365 -> 1. Test cleanup failure, as there appears to be some order dependent test failures

However, I've reverted back to the original time in hopes it will be sufficient

emdobrin · 2023-07-13T18:42:08Z

            waitForRegistration.countDown()
        })
-        XCTAssertEqual(DispatchTimeoutResult.success, waitForRegistration.await(timeout: 2))
+        XCTAssertEqual(DispatchTimeoutResult.success, waitForRegistration.await(timeout: 10))


I would think even 2 sec is a lot for registration, do you know aprox. how long it actually requires to run on CI?

Based on the logs it seems like consistently less than a second

Looking at the logs again for test setup failure cases, it looks like this class of test failure happens when the unit test build process is interleaved with the currently running functional test case (I have other examples, but only showing screenshots for testSendEvent_twoConsecutiveCalls_appendsReceivedClientSideStore to demonstrate)

Successful test setup

Failing test setup

When the test setup happens correctly (ex: this latest build), there is no unit test build interleaved

Should the greatly extended timeout be kept to account for this potential interleaving? Or is there a way to prevent the interleaving from happening?
Since the timeout wait can be satisfied early if all conditions are met, having a large timeout value should not affect runtime of most test cases right?

update: we decided to split the test runs in two steps: unit and functional for each platform, see PR #368

timkimadobe

Thank you for the review @emdobrin! Updated based on feedback and added additional context, with some outstanding questions

timkimadobe · 2023-07-13T22:19:30Z

        // Wait .2 seconds in case there are unexpected events that were in the dispatch process during cleanup
-        usleep(200000)
+//        usleep(200000)
+        sleep(1)


I was thinking this would catch the failure case from #365 -> 1. Test cleanup failure, as there appears to be some order dependent test failures

However, I've reverted back to the original time in hopes it will be sufficient

timkimadobe · 2023-07-13T22:39:36Z

            waitForRegistration.countDown()
        })
-        XCTAssertEqual(DispatchTimeoutResult.success, waitForRegistration.await(timeout: 2))
+        XCTAssertEqual(DispatchTimeoutResult.success, waitForRegistration.await(timeout: 10))


Based on the logs it seems like consistently less than a second

Looking at the logs again for test setup failure cases, it looks like this class of test failure happens when the unit test build process is interleaved with the currently running functional test case (I have other examples, but only showing screenshots for testSendEvent_twoConsecutiveCalls_appendsReceivedClientSideStore to demonstrate)

Successful test setup

Failing test setup

When the test setup happens correctly (ex: this latest build), there is no unit test build interleaved

Should the greatly extended timeout be kept to account for this potential interleaving? Or is there a way to prevent the interleaving from happening?
Since the timeout wait can be satisfied early if all conditions are met, having a large timeout value should not affect runtime of most test cases right?

…d warnings (adobe#363) * Look for eventIndex in report object of error and warning in network response * Do not encode eventIndex in EdgeEventError or EdgeEventWarning objects

timkimadobe · 2023-07-15T00:08:36Z

Note: removed the CI testing workflow changes from this branch in anticipation of #368 being merged into dev. Will merge dev into the fork feature branch to bring the workflow fix once #368 is merged

Updating version to 4.1.0

* Split unit and functional test steps Update CircleCI script accordingly * Reorder unit and functional tests * Reorder unit and functional in cleanup step * Test reordering libraries * Split Makefile rules test-ios and test-tvos to run unit and functional tests separately * Run functional tests even when unit tests fail in CI * Move test reports under 'build/reports' folder --------- Co-authored-by: Kevin Lind <lind@adobe.com>

[CI] Sleep after EventHub.shared.start in API unit tests

…e#370)

# Conflicts: # Tests/FunctionalTests/NetworkResponseHandlerFunctionalTests.swift # Tests/UnitTests/EdgePublicAPITests.swift

emdobrin · 2023-07-19T20:59:07Z

    public override func tearDown() {
        super.tearDown()
-
+//        unregisterInstrumentedExtension()


do we need this change or can we revert?

looks like we don't unregister in dev branch either, likely because down bellow we call EventHub.reset() anyway, so that will reset all extensions before next test run.
So, can we remove this commented line then? And I am also curious where else we use this function unregisterInstrumentedExtension

I think we can revert this change! I saw this API available in the TestBase class and just wanted to try it to see if it would address some of the flaky test issues (which ended up being caused by other things)

The unregisterInstrumentedExtension extension isnt used anywhere else that I could see

Thanks for checking, changes look good to me. We can revisit this in the future and see if we can remove the unused API and the associated code in the InstrumentedExtension.

timkimadobe

Thanks for the review @emdobrin! Addressed feedback with latest update

timkimadobe · 2023-07-19T22:59:22Z

    public override func tearDown() {
        super.tearDown()
-
+//        unregisterInstrumentedExtension()


I think we can revert this change! I saw this API available in the TestBase class and just wanted to try it to see if it would address some of the flaky test issues (which ended up being caused by other things)

The unregisterInstrumentedExtension extension isnt used anywhere else that I could see

kevinlind and others added 14 commits May 31, 2023 15:45

Remove deprecated class XDMFormatters (adobe#350)

7f530ce

* Remove deprecated class XDMFormatters * Update timestamp field in MobileSDKCommerceSchema in tutorial apps

Update Podfile to use Core/Services staging branch (adobe#353)

8bb2d4f

* Update Podfile to use Core staging and Edge* dev branches * Update SwiftLint version 0.52.0 * Fix format after SwiftLint update

Update Podfile to use Core/Services production version (adobe#354)

677f96c

* Update Podfile to use Core/Services production version * Update podfile for Tutorial apps

Merge pull request adobe#351 from adobe/dev

859ec7c

Merge to Staging for 4.0.0 release

Merge pull request adobe#355 from adobe/staging

edd35fc

Staging -> Main v4.0.0 release

Fix makefile buil-app command (adobe#356)

07bb964

Update dependencies to prod

c9110f2

Add labels and update links

74b07db

Update more links

58a1e26

Merge pull request adobe#357 from emdobrin/main

2a7c7b2

Update dependencies to prod and links

Merge pull request adobe#358 from adobe/main

1cf1b14

downmerge main -> dev

Merge branch 'dev' into feature/upstream-integration-tests

066f467

# Conflicts: # Makefile # Podfile.lock # README.md # Tests/FunctionalTests/NetworkResponseHandlerFunctionalTests.swift

timkimadobe changed the title ~~Update with merge from latest dev changes~~ Update with latest dev changes Jul 11, 2023

timkimadobe added 2 commits July 11, 2023 16:05

Refactor functional tests from dev branch to use new testing utilities

bf50a33

Use longer extension registration timeout

c843a0a

timkimadobe added 5 commits July 11, 2023 16:43

Add per test case teardown network service reset

5162908

Test extending teardown sleep duration

14fb914

Add higher timeout value for startup event validation

b0af799

Add sleep to allow more buffer for listener registration

2f77386

Extend setup timeout to 10s

a8ef865

timkimadobe requested review from addb, cacheung, emdobrin and kevinlind July 12, 2023 23:09

timkimadobe mentioned this pull request Jul 13, 2023

Integration testing workflow update #349

Merged

10 tasks

emdobrin reviewed Jul 13, 2023

View reviewed changes

Restore original teardown sleep time

9ba6beb

Remove sleep from test case

462b42d

timkimadobe commented Jul 13, 2023

View reviewed changes

timkimadobe and others added 6 commits July 13, 2023 15:56

Change order of operations for test teardown

ecfed98

Test unregistering instrumented extension in teardown process

285af81

Get 'eventIndex' from 'report' object for interact response errors an…

b2eabc0

…d warnings (adobe#363) * Look for eventIndex in report object of error and warning in network response * Do not encode eventIndex in EdgeEventError or EdgeEventWarning objects

Add sleep to startup process to allow event hub setup time

b5c97f8

Updating version to 4.1.0.

a2d61de

Add mock network service teardown to all functional tests

f73c05b

timkimadobe force-pushed the feature/upstream-integration-tests branch from ab8dac3 to f73c05b Compare July 15, 2023 00:06

kevinlind and others added 7 commits July 17, 2023 11:53

Merge pull request adobe#366 from adobe/version-4.1.0-update

8a2b1e7

Updating version to 4.1.0

Sleep after EventHub.shared.start in API unit tests

b43e2c8

Merge pull request adobe#369 from kevinlind/fix-api-unit-test

bde2acf

[CI] Sleep after EventHub.shared.start in API unit tests

Wait for sendEvent to return before continuing functional test. (adob…

2bf8b01

…e#370)

Merge branch 'dev' into feature/upstream-integration-tests

ea1825a

# Conflicts: # Tests/FunctionalTests/NetworkResponseHandlerFunctionalTests.swift # Tests/UnitTests/EdgePublicAPITests.swift

Trigger CI workflow

a8c3d22

emdobrin reviewed Jul 19, 2023

View reviewed changes

Revert changes in TestBase teardown

8fe73a8

timkimadobe commented Jul 19, 2023

View reviewed changes

emdobrin approved these changes Jul 19, 2023

View reviewed changes

emdobrin merged commit a43186b into adobe:feature/upstream-integration-tests Jul 19, 2023

Conversation

timkimadobe commented Jul 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

codecov Bot commented Jul 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timkimadobe Jul 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Successful test setup

Failing test setup

Uh oh!

emdobrin Jul 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

timkimadobe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timkimadobe Jul 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Successful test setup

Failing test setup

Uh oh!

timkimadobe commented Jul 15, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emdobrin Jul 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timkimadobe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

timkimadobe commented Jul 11, 2023 •

edited

Loading

codecov Bot commented Jul 11, 2023 •

edited

Loading

timkimadobe Jul 13, 2023 •

edited

Loading

emdobrin Jul 19, 2023 •

edited

Loading

timkimadobe Jul 13, 2023 •

edited

Loading

emdobrin Jul 19, 2023 •

edited

Loading