Update shared testing utilities for integration tests#334
Conversation
… for dual mode This allows for sharing common test utilities between functional and integration tests
Tests/TestUtils/TestBase.swift
Outdated
| wait(FunctionalTestConst.Defaults.WAIT_TIMEOUT) | ||
| } | ||
|
|
||
| return TestBase.networkService.getMatchedResponseForUrlAndHttpMethod(networkRequest: networkRequest) |
There was a problem hiding this comment.
I would recommend not to reuse the same responses list for both mocked and real responses as it can lead to issues difficult to debug. You can use two lists one for mockedNetworkResponses and one for serverNetworkResponse to remove the ambiguity
There was a problem hiding this comment.
I'm just wondering if we want to support a hybrid use-case where both real and mocked responses would be used in the assertion logic for a test case?
If not I think we can create an all or nothing mock or real network response data structure networkResponses that is dependent on the state set in mockNetworkService (instead of 2 separate data structs and corresponding mock/real methods etc) and update the mock response setter to short circuit if mock mode is false etc.
What are your thoughts?
There was a problem hiding this comment.
I think we should not mix real with mock network responses and the test class should make the selection upfront. ie integration test class will use real network, functional test class will use mock network
There was a problem hiding this comment.
That makes sense to me, and with the current setup the upfront mode selection should be enforced across the test suite by baking the selection into setUp() method (as seen in UpstreamIntegrationTests.swift)
As a consequence, the networkResponses dictionary will only hold mocks if mockNetworkService is true
and only hold real responses if mockNetworkService is false - that is we don't have a mixed mock and real situation
Create separate data structs for mocked and real network responses Update connectAsync logic structure
Update method docs for TestNetworkService Update mock bool in TestBase Move AnyCodable helper methods to shared test utils
timkimadobe
left a comment
There was a problem hiding this comment.
Thank you for the review @emdobrin! Updated according to feedback with a few outstanding questions
Tests/TestUtils/TestBase.swift
Outdated
| wait(FunctionalTestConst.Defaults.WAIT_TIMEOUT) | ||
| } | ||
|
|
||
| return TestBase.networkService.getMatchedResponseForUrlAndHttpMethod(networkRequest: networkRequest) |
There was a problem hiding this comment.
I'm just wondering if we want to support a hybrid use-case where both real and mocked responses would be used in the assertion logic for a test case?
If not I think we can create an all or nothing mock or real network response data structure networkResponses that is dependent on the state set in mockNetworkService (instead of 2 separate data structs and corresponding mock/real methods etc) and update the mock response setter to short circuit if mock mode is false etc.
What are your thoughts?
With mocked behavior controlled by mockNetworkService flag
emdobrin
left a comment
There was a problem hiding this comment.
Thanks @timkimadobe, reviewed except TestNetworkService for now, this file doesn't show the diff right
Tests/TestUtils/TestBase.swift
Outdated
| wait(FunctionalTestConst.Defaults.WAIT_TIMEOUT) | ||
| } | ||
|
|
||
| return TestBase.networkService.getMatchedResponseForUrlAndHttpMethod(networkRequest: networkRequest) |
There was a problem hiding this comment.
I think we should not mix real with mock network responses and the test class should make the selection upfront. ie integration test class will use real network, functional test class will use mock network
Consolidate awaitRequest logic Simplify NetworkRequest construction Use force unwrap for test constructions
timkimadobe
left a comment
There was a problem hiding this comment.
Thanks for the followup review @emdobrin! Addressed all feedback with latest update, with an outstanding question for the receivedNetworkRequests data structure: https://github.com/adobe/aepsdk-edge-ios/pull/334/files/08696c3ab31a79f08f8fb2a7613685ac361739d9#r1187922499
Tests/TestUtils/TestBase.swift
Outdated
| wait(FunctionalTestConst.Defaults.WAIT_TIMEOUT) | ||
| } | ||
|
|
||
| return TestBase.networkService.getMatchedResponseForUrlAndHttpMethod(networkRequest: networkRequest) |
There was a problem hiding this comment.
That makes sense to me, and with the current setup the upfront mode selection should be enforced across the test suite by baking the selection into setUp() method (as seen in UpstreamIntegrationTests.swift)
As a consequence, the networkResponses dictionary will only hold mocks if mockNetworkService is true
and only hold real responses if mockNetworkService is false - that is we don't have a mixed mock and real situation
| /// Matches outgoing `NetworkRequest`s with their corresponding `HttpConnection` response. | ||
| /// Mock or real server response controlled by `mockNetworkService` | ||
| /// Mocked `HttpConnection` response can be set using `setResponseConnectionFor(networkRequest:responseConnection:)` | ||
| private var networkResponses: [NetworkRequest: HttpConnection] = [:] |
There was a problem hiding this comment.
Instead of this comment why not separating them into two arrays, only one of them being populated at a time depending on the type of tests:
mockedResponseMatchers
receivedNetworkResponses
There was a problem hiding this comment.
I didn't want to separate them into two different data structures because if there's no use case for having both mocked and real responses in a test case simultaneously, having two distinct structures would mean that one of them remains unused throughout the entire test suite, depending on the mode.
Additionally, the logic for updating and retrieving values becomes more complicated. This could either involve separate methods or require the shared getter/setter to employ if/else statements based on the mock mode to use the appropriate data structure.
In the single storage approach, the mock mode determines whether networkResponses holds mocks or real responses. The setters/getters also help prevent misuse by incorporating checks using the mode flag. From a test writing perspective, I believe it's more intuitive to have a unified getResponsesFor(networkRequest:) method, since the mock mode is set during the setup phase, and you already know whether all the responses are mocked or real.
To summarize, I think the mockNetworkService flag effectively handles this functionality toggle. If we want to create mock and real versions of the data structures and methods, we could do so and remove the mockNetworkService flag, as it would become redundant.
There was a problem hiding this comment.
thanks Tim, as discussed we'll handle this in the next PR with more clear boundaries between the two service implementations
| } | ||
|
|
||
| // MARK: Network request response helpers | ||
| func getResponsesFor(networkRequest: NetworkRequest) -> [HttpConnection] { |
There was a problem hiding this comment.
same comment for these helpers, we should clearly separate the two flows:
setMockResponseFor(... ) - sets mock response to be used with mockNetworkService
getReceivedResponseFor() - gets real responses when mockNetworkServices is off
private getMockResponseFor(...) - used only in this class for returning mock responses when mockNetworkService is on
There was a problem hiding this comment.
Please see response for https://github.com/adobe/aepsdk-edge-ios/pull/334/files#r1190191033
timkimadobe
left a comment
There was a problem hiding this comment.
Thanks for the review @emdobrin! Updated according to feedback, with some outstanding questions
| /// Matches outgoing `NetworkRequest`s with their corresponding `HttpConnection` response. | ||
| /// Mock or real server response controlled by `mockNetworkService` | ||
| /// Mocked `HttpConnection` response can be set using `setResponseConnectionFor(networkRequest:responseConnection:)` | ||
| private var networkResponses: [NetworkRequest: HttpConnection] = [:] |
There was a problem hiding this comment.
I didn't want to separate them into two different data structures because if there's no use case for having both mocked and real responses in a test case simultaneously, having two distinct structures would mean that one of them remains unused throughout the entire test suite, depending on the mode.
Additionally, the logic for updating and retrieving values becomes more complicated. This could either involve separate methods or require the shared getter/setter to employ if/else statements based on the mock mode to use the appropriate data structure.
In the single storage approach, the mock mode determines whether networkResponses holds mocks or real responses. The setters/getters also help prevent misuse by incorporating checks using the mode flag. From a test writing perspective, I believe it's more intuitive to have a unified getResponsesFor(networkRequest:) method, since the mock mode is set during the setup phase, and you already know whether all the responses are mocked or real.
To summarize, I think the mockNetworkService flag effectively handles this functionality toggle. If we want to create mock and real versions of the data structures and methods, we could do so and remove the mockNetworkService flag, as it would become redundant.
| } | ||
|
|
||
| // MARK: Network request response helpers | ||
| func getResponsesFor(networkRequest: NetworkRequest) -> [HttpConnection] { |
There was a problem hiding this comment.
Please see response for https://github.com/adobe/aepsdk-edge-ios/pull/334/files#r1190191033
|
Thank you! As discussed, I'll address the cleanup of the mock vs real requests split in the TestBase and TestNetworkService in a followup PR 🥳 |
* Move shared test files * Refactor FunctionalTestBase and FunctionalTestNetworkService to allow for dual mode This allows for sharing common test utilities between functional and integration tests * Renaming all usages of FunctionalTestBase to updated name * Rename mock mode bool * Update method docs and cleanup code comments * Update initializer param Create separate data structs for mocked and real network responses Update connectAsync logic structure * Move XCTestCase+AnyCodableAsserts to shared test utils Update method docs for TestNetworkService Update mock bool in TestBase Move AnyCodable helper methods to shared test utils * Update TestConstants name and usages * Consolidate networkResponses into single data struct With mocked behavior controlled by mockNetworkService flag * Update FunctionalTestConst usages * Temporarily moving TestNetworkService for review * Update set get logic for network responses Consolidate awaitRequest logic Simplify NetworkRequest construction Use force unwrap for test constructions * Move NetworkRequest extension to FTNS
* Move shared test files * Refactor FunctionalTestBase and FunctionalTestNetworkService to allow for dual mode This allows for sharing common test utilities between functional and integration tests * Renaming all usages of FunctionalTestBase to updated name * Rename mock mode bool * Update method docs and cleanup code comments * Update initializer param Create separate data structs for mocked and real network responses Update connectAsync logic structure * Move XCTestCase+AnyCodableAsserts to shared test utils Update method docs for TestNetworkService Update mock bool in TestBase Move AnyCodable helper methods to shared test utils * Update TestConstants name and usages * Consolidate networkResponses into single data struct With mocked behavior controlled by mockNetworkService flag * Update FunctionalTestConst usages * Temporarily moving TestNetworkService for review * Update set get logic for network responses Consolidate awaitRequest logic Simplify NetworkRequest construction Use force unwrap for test constructions * Move NetworkRequest extension to FTNS
* Edge Network (Konductor) integration test with GitHub Action workflow (#321) * initial e2e test * update pods for e2e test target * update workflow config settings * implement pods cache to prevent high macos usage time * Fix method typo in functional test helper * test for locationHint * Update integration test case * Remove specific hint value * update action to include build cache step * update buildcache to upload logs for debug * fix flag typo * trying different cache system * try with command line settings * update to include key * fix var bug in makefile command * remove playground * Remove build cache testing * passing env vars to test target working * Update e2e flow action, yaml, and test scheme for konductor env * Fix action command to updated name * Updating action text and defaults * Update make command for e2e test to improve documentation add conditional check warning output for KONDUCTOR_ENV * update test names * update job name * Update env var extraction logic * update test to use edge location hint * update action to have location hint options * fix github action var name * Update makefile env vars and include edge location hint * update makefile integration test docs test colon in test run echo update env var extraction documentation in test file * update test output to use raw string values * test using shell if else for job run id * fix spacing syntax * Update to user proper Edge Network name * Update to EDGE_ENVIRONMENT * Small doc updates * Update documentation to correct links * Update dropdown description * Add quiet flag to xcodebuild command for integration test * Rename integration test make command Reorder position to under tvOS test * Update name to UpstreamIntegrationTests Test empty string option for dropdown * Update action script to use updated make command * Split enums into separate file, add env var extraction inits * run pod install after new target name * Remove FunctionalTestBase inheritance and unused compile sources * Add validation for network request portion * Rename and simplify IntegrationTestNetworkService to only required methods and properties * Update to use raw multiline string instead of resource files * Revert podfile lock version upgrade * Update action job name to align with makefile name * Update target name in makefile * Remove extra newlines Remove marketing version from integration test debug target build settings * Remove implementation specific documentation * Test spacing in makefile * Test remove silent option * test moving back to the bottom * test updating command name * test newline * test name change again * Revert all test name changes - the workflow error was not specifying the correct branch to run off of * Add job failure help text * Add doc comment to on failure step * Revert changes in functional test utils * Split test enums into separate files with shared util method * Clean up comments and update test method * Update EdgeEnvironment enum to have .prod default value Update usage site in test setup * Update to include xcresult visualizer tool * Update with example test failure case * Update env file ID to use helper method Move NetworkTestingDelegate to network file * Omit coverage from test results to save characters * Fix clang flag warning from cocoapods * Remove Xcode report tool step from this PR * Update result bundle path to avoid name conflicts with other tests * Add testing documentation comment hint * Flexible JSON comparison system (#332) * Implement JSON comparison system * Update to use AEPServices AnyCodable * Move assertion helpers to AnyCodableUtils Update assertion methods to accept file and line args for inline errors on test failure Update key path logic to pretty print * Create flexible validation system, with exact match pathing * Exact match wildcard example * Update general wildcard logic to apply to all elements Rename arguments and usage sites * Complete revamp of flexible json comparison system * Convert AnyCodable test assertion helpers to protocol with default implementations Update test classes to adhere to new protocol and update usages Update flexible assertion to use single method with parameter for default mode Clean up code and update documentation for helper methods Fix bug with escaped keys and add unit test case * Apply swift lint * Extract AnyCodable array extension into separate file * Remove unused test setup methods * Remove redundant test cases covered by AnyCodable unit tests * Switch from protocol to XCTestCase extension Update usages Update documentation text * Update keyPathAsString signature and usages * Simplify implementation of AnyCodable array comparison * Remove unused EventSpec * Update filenames Remove Bool return from assertEqual methods Refactor to allow single public API for assertEqual * Update flexible comparison APIs * Add additional test cases for AnyCodable unit tests * Apply swift lint formatting * Update to use positive case * Update method order Update method signature styling * Update actual condition * Update flexible text setup to propagate file and line Upgrade getCapturedRegexGroups to a test failure Upgrade alternate path index errors to TEST ERROR, add file and line * Fix incorrect find and replace * Update regex capture to handle special cases Add test cases that validate special cases * Extract regex logic into shared function * Update shared testing utilities for integration tests (#334) * Move shared test files * Refactor FunctionalTestBase and FunctionalTestNetworkService to allow for dual mode This allows for sharing common test utilities between functional and integration tests * Renaming all usages of FunctionalTestBase to updated name * Rename mock mode bool * Update method docs and cleanup code comments * Update initializer param Create separate data structs for mocked and real network responses Update connectAsync logic structure * Move XCTestCase+AnyCodableAsserts to shared test utils Update method docs for TestNetworkService Update mock bool in TestBase Move AnyCodable helper methods to shared test utils * Update TestConstants name and usages * Consolidate networkResponses into single data struct With mocked behavior controlled by mockNetworkService flag * Update FunctionalTestConst usages * Temporarily moving TestNetworkService for review * Update set get logic for network responses Consolidate awaitRequest logic Simplify NetworkRequest construction Use force unwrap for test constructions * Move NetworkRequest extension to FTNS * Integration tests - TestNetworkService mock and server API split (#337) * Move FTNS to shared test utils * Rename FTNS to TNS * Initial split of mock and server test network service implementations Before removal of network logic from TestBase * Migrate TestBase network APIs to TestNetworkService Move mock or server specific APIs to respective child classes * Update log source name Update reset test expectations logic * Remove resetting network service logic from test base * Rename delayed network response param and var Remove outdated method docs for return value * Update setup logic Update reset test expectations usage * Update network service to static immutable var Update usages to Self * Remove todo as task is wont do * Update mock response API param name * Update integration test to use static network service Also add helper comments to the purpose of the different setup methods * Rename Server to Real TNS Update base class inheritance for both mock and real TNS Update base TNS to be a shared network request helper Update usages within mock and real TNS to use new shared helper as instance var Add passthrough APIs for both mock and real TNS to access helper methods as needed Refactor static networkService in test class to instance to remove need for Self prefix Update associated setUp logic Add mock prefix to networkService in functional test for clarity * Rename mock and real network service classes * Clean up implementation Update doc comment Add implementation note for isEqual * Move shared network service logic to helper Update usage sites Remove unused import * Remove unused imports * Move NetworkRequest flatten body method into NetworkRequest extension Update usages * Update access level * Update doc class names * Remove unneeded commented code * Move testbase debug flag to per case setup Update doc comment * Refactor CompletionHandlerFunctionalTests to use MockNetworkService * Refactor EdgeConsentTests to use MockNetworkService * Add networkService reset * Add NetworkService reset * Refactor EdgePublicAPITests to use MockNetworkService * Refactor AEPEdgePathOverwriteTests Add test flow doc comments * Refactor IdentityStateFunctionalTests Move debug flag set to after setup * Refactor NoConfigFunctionalTests Update doc comment for what method is used to determine equality * Refactor SampleFunctionalTests * Apply lint autocorrect to PR files * Remove unneeded commented code * Integration test cases (#346) * Implementation notes * WIP invalid datastream test * Add test cases Complex XDM, complex data Preset location hint expected error dataset ID expected error invalid location hint * Update setExpectation API to accept only NetworkRequest Update docs Update usages of updated API Update networkService API usages in integration test class * Clean up code comments * Fix for unpassed params * Update class docs Add test note on how JSON comparison system works * Apply swift lint autocorrect * Rename vars and add additional assert on response count Add changes lost in rebase * Updated flexible JSON comparison notes Refactored assertEdgeResonseHandle API and usages Refactored location hint test case to use single variable for location hint value * Update network service class docs * Update first test case to have two event validation * Remove unused API and refactor used API to call helper directly * Update assertEdgeResponseHandle signature and usages Create new helper methods to construct interact URLs with location hint * Add strict count assertion for all responses Update matchedResponses name to be uniform across test suite * Update assertEdgeResponseError method to use getDispatchedEventsWith directly Revert refactor of assertEdgeResponseHandle and usages Add expectedCount argument to both APIs and update assertion logic to check all events Add org ID related exact match assertions and exact match paths (non-wildcard) Add exact match requirement for error type URL * Remove unused API (actually) * Update test cases to use direct assertions * Refactor assert*Match APIs to non-nil expected AnyCodable * Update assertExpectedEvents API to allow for custom timeout period * Update to use conditional check on location hint if initial value is set Simplify 2x event handle test case Add longer timeout for event expectations * Update test case setup Remove outdated API comment * Remove unused helper API * Refactor functional tests from dev branch to use new testing utilities * Use longer extension registration timeout * Add per test case teardown network service reset * Test extending teardown sleep duration * Add higher timeout value for startup event validation * Add sleep to allow more buffer for listener registration * Extend setup timeout to 10s * Restore original teardown sleep time * Remove sleep from test case * Change order of operations for test teardown * Test unregistering instrumented extension in teardown process * Add sleep to startup process to allow event hub setup time * Add mock network service teardown to all functional tests * Trigger CI workflow * Revert changes in TestBase teardown * Integration testing workflow update (#349) * Add new integration test job * Update device for integration to 8 * Update integration test job name * Remove extra space * Remove code coverage upload from integration job * Test job conditional in circleci workflow * Add non conditional step * Update triggering branches * Update initiating branch name condition to staging * test fetching branch name from fork * remove spaces from command * Test extract both base and head branch names * Add semicolons * Update extraction commands * Only fetch base branch name Make integration setup steps non conditional * Update job trigger conditions * Update simulator for integration test to 14 * Update integration test workflow to use conditional at job level * Update integration test job Xcode version * Fix conditional in integration test job * Move checkout step to non-conditional level * Test current branch name also triggers integration test workflow * Test branch name 2 * Remove test branch name * Add main branch to check * Update deployment version for integration target to 11 * Make integration test make command result output consistent with other tests Add removal of existing old results part of the job like other tests * Refactor to remove branch name conditionals from job level and use workflow job branch filter * Test workflow job filter when set on current branch * Update filter criteria * Remove test branch filter * Reorder jobs so conditional ones come after mandatory * Update integration test target in podfile --------- Co-authored-by: Emilia Dobrin <33132425+emdobrin@users.noreply.github.com>
Description
This PR updates the FunctionalTestBase and FunctionalTestNetworkService to become a shared testing utility, mainly:
There is also an example test case using these new methods and logic in
UpstreamIntegrationTests.swift, also taking advantage of the flexible JSON validation to perform assertions against the location hint response event payloadRelated Issue
Motivation and Context
A lot of the network request, response, and event capture logic is shared between functional and integration tests, so it would be helpful to be able to easily toggle the mocked/real network request logic and maintain all the shared testing utilities in one place.
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: