Refactor functional tests to use JSON comparison (part 2)#438
Refactor functional tests to use JSON comparison (part 2)#438
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## dev #438 +/- ##
===========================================
- Coverage 96.47% 85.66% -10.81%
===========================================
Files 27 27
Lines 1730 1730
===========================================
- Hits 1669 1482 -187
- Misses 61 248 +187 |
|
Rebase required for this PR to show the actual diff, @kevinlind can you help with a quick review afterwards? |
# Conflicts: # Tests/FunctionalTests/AEPEdgeFunctionalTests.swift # Tests/FunctionalTests/CompletionHandlerFunctionalTests.swift # Tests/FunctionalTests/Edge+ConsentTests.swift # Tests/FunctionalTests/EdgeDatastreamConfigOverrideTests.swift
There was a problem hiding this comment.
Thanks for the review @kevinlind! Updated based on feedback - please note that the current implementation relies on the changes from: adobe/aepsdk-testutils-ios#27 being available (these changes have been merged into feature/latest already)
emdobrin
left a comment
There was a problem hiding this comment.
One small comment from me, looks great otherwise
| } | ||
| } | ||
| let expectedJSON = "{}" | ||
| assertTypeMatch(expected: expectedJSON, actual: resultNetworkRequests[0], pathOptions: KeyMustBeAbsent(paths: "events.path")) |
There was a problem hiding this comment.
This test should check that the network request does not include the key "request" and its value under "events" since this is an internal key that Edge extension processes and removes from the event sent upstream. events.request.path must be absent
There was a problem hiding this comment.
Oh I see, if I'm understanding correctly, the current validation path events.path is incorrect - and this was not the path validated by the original test logic either - the actual path should be events.request.path?
I've updated the path accordingly, please let me know what you think!
There was a problem hiding this comment.
Right, the intention of the original test was to check that the internal key was not added in the network request. One small correction though, I think we need to check for this path instead events[0].request.path.
There was a problem hiding this comment.
Ok thank you! Updated path value
|
Important The latest updates have changed the |
This test should check that the network request does not include the key "request" and its value under "events" since this is an internal key that Edge extension processes and removes from the event sent upstream.
timkimadobe
left a comment
There was a problem hiding this comment.
Thanks for the review @emdobrin! Updated based on feedback
| } | ||
| } | ||
| let expectedJSON = "{}" | ||
| assertTypeMatch(expected: expectedJSON, actual: resultNetworkRequests[0], pathOptions: KeyMustBeAbsent(paths: "events.path")) |
There was a problem hiding this comment.
Oh I see, if I'm understanding correctly, the current validation path events.path is incorrect - and this was not the path validated by the original test logic either - the actual path should be events.request.path?
I've updated the path accordingly, please let me know what you think!
Note
These changes are a continuation of #437
Description
This PR updates the next portion of Edge functional tests to use the JSON comparison system. Broadly, it replaces dictionary flattening with JSON comparison. Specific changes:
assertExactMatch/assertTypeMatch- that is, the refactor should preserve the same level of strictness for the payload being testedFileManager.default.clearCache()is used,FileManager.default.removeAdobeCacheDirectory()was also added (given Core's new persistence handling)Questions for reviewers
func testProcessResponseOnSuccess_WhenOneEventHandle_emptyEventHandleType_dispatchesEvent()assertExactMatchwith type validation mode for keytypewas used since in the original test case logic, only the presence of 5 keys was validated (implicitly requiring thetypekey's presence) - is this an acceptable change in the scope of the test logic?Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: