-
Notifications
You must be signed in to change notification settings - Fork 7
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
Integration test AEPTestUtils migration #158
Integration test AEPTestUtils migration #158
Conversation
…rtEquals Import the junit.Assert.assertEquals method so prefix is not required
…icitly use AnyOrderMatch where required Fix testSendEvent_withPriorState_receivesExpectedStateStoreEventHandle test setup by adding expectation assertion
…idDatastreamIdOverride_receivesErrorResponse
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #158 +/- ##
======================================
Coverage ? 84.69%
Complexity ? 407
======================================
Files ? 30
Lines ? 1639
Branches ? 237
======================================
Hits ? 1388
Misses ? 160
Partials ? 91
Flags with carried forward coverage won't be shown. Click here to find out more. |
expected = JSONObject(expectedLocationHintJSON), | ||
actual = JSONObject(locationHintResult.eventData), | ||
exactMatchPaths = listOf("payload[*].scope", "payload[*].hint") | ||
assertTypeMatch( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, we can do assertExactMatch here and typeMatch the "payload[*].ttlSeconds".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Updated
expected = JSONObject(expectedLocationHintJSON), | ||
actual = JSONObject(locationHintResult.eventData), | ||
exactMatchPaths = listOf("payload[*].scope") | ||
assertTypeMatch( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, we can do assertExactMatch here and typeMatch the "payload[*].ttlSeconds".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this case we should be ok to keep typeMatch here since 2/3 properties use type match
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @cacheung! Updated based on feedback
expected = JSONObject(expectedLocationHintJSON), | ||
actual = JSONObject(locationHintResult.eventData), | ||
exactMatchPaths = listOf("payload[*].scope") | ||
assertTypeMatch( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this case we should be ok to keep typeMatch here since 2/3 properties use type match
expected = JSONObject(expectedLocationHintJSON), | ||
actual = JSONObject(locationHintResult.eventData), | ||
exactMatchPaths = listOf("payload[*].scope", "payload[*].hint") | ||
assertTypeMatch( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Updated
} | ||
] | ||
} | ||
""".trimIndent() | ||
""" | ||
|
||
// Unsafe access used since testSendEvent_receivesExpectedEventHandles guarantees existence | ||
val stateStoreEvent = TestSetupHelper.getEdgeEventHandles(expectedHandleType = TestConstants.EventSource.STATE_STORE) | ||
.last() | ||
|
||
// Exact match used here to strictly validate `payload` array element count == 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: update comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated! I also updated the test case logic to actually apply the collection element count restriction on payload
and also the elements under it, to better reflect the test case intent
realNetworkService.assertAllNetworkRequestExpectations() | ||
val matchingResponses = realNetworkService.getResponsesFor(networkRequest = invalidNetworkRequest) | ||
|
||
val matchingResponses = realNetworkService.getResponsesFor(invalidNetworkRequest) | ||
|
||
Assert.assertEquals(1, matchingResponses?.size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow similar usages everywhere.
Import all the required functions like the configOverrideIntegrationTests and update the usages.
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated! Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | ||
] | ||
} | ||
""".trimIndent() | ||
""" | ||
|
||
// Unsafe access used since testSendEvent_receivesExpectedEventHandles guarantees existence | ||
val stateStoreEvent = TestSetupHelper.getEdgeEventHandles(expectedHandleType = TestConstants.EventSource.STATE_STORE) | ||
.last() | ||
|
||
// Exact match used here to strictly validate `payload` array element count == 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated! I also updated the test case logic to actually apply the collection element count restriction on payload
and also the elements under it, to better reflect the test case intent
realNetworkService.assertAllNetworkRequestExpectations() | ||
val matchingResponses = realNetworkService.getResponsesFor(networkRequest = invalidNetworkRequest) | ||
|
||
val matchingResponses = realNetworkService.getResponsesFor(invalidNetworkRequest) | ||
|
||
Assert.assertEquals(1, matchingResponses?.size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated! Thank you
Description
This PR updates the integration test classes:
ConfigOverridesIntegrationTests.kt
UpstreamIntegrationTests.kt
to use the AEPTestUtils library, with the following changes:
typeMatchPaths
andexactMatchPaths
2. JUnit
assertEquals
is imported directly3.
assertExactMatch
andassertTypeMatch
from JSON comparison is imported directly4. Remove
.trimIndent()
from JSON strings5. Remove
JSON
suffix from var names6. Use
"STRING_TYPE"
convention in expected JSON strings to make value type match intent clearerSpecific changes:
testSendEvent_withPriorState_receivesExpectedStateStoreEventHandle
: Added event expectation to test setup to properly await event processing before beginning test logicRelated Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: