-
Notifications
You must be signed in to change notification settings - Fork 12
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
Functional test AEPTestUtils update (part 2) #156
Functional test AEPTestUtils update (part 2) #156
Conversation
Replace local getPayloadJson method and usages
Replace usage of local getPayloadJson
Map<String, String> flattenedData = TestUtils.flattenMap(eventData); | ||
assertEquals(1, flattenedData.size()); | ||
assertEquals("xdm", flattenedData.get("xdm.testString")); | ||
String expected = "{" + " \"xdm\": {" + " \"testString\": \"xdm\"" + " }" + "}"; |
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.
can we update the value to something like "testValue" instead of "xdm"
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.
Yes! Updated
…to update testValue for clarity
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.
Looks good. Have put some nit comments.
String expected = | ||
"{" + | ||
" \"xdm\": {" + | ||
" \"testString\": \"xdm\"," + |
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: "testString": "stringValue"
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!
String expected = | ||
"{" + | ||
" \"xdm\": {" + | ||
" \"testString\": \"xdm\"" + |
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: "testString": "stringValue"
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! However, in this case do we want to make the value under XDM distinct from the one under data
?
code/edge/src/androidTest/java/com/adobe/marketing/mobile/EdgeFunctionalTests.java
Show resolved
Hide resolved
Map<String, String> flattenedData = TestUtils.flattenMap(eventData); | ||
assertEquals(1, flattenedData.size()); | ||
assertEquals("xdm", flattenedData.get("xdm.testString")); | ||
String expected = "{\"xdm\": {\"testString\": \"testValue\"}}"; |
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: "testString": "stringValue"
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!
" \"testMap\": {" + | ||
" \"key\": \"value\"" + | ||
" }," + | ||
" \"testString\": \"xdmValue\"," + |
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: "testString": "stringValue"
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!
" }," + | ||
" \"xdm\": {" + | ||
" \"_id\": \"STRING_TYPE\"," + | ||
" \"boolObject\": true," + |
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: make it same as testString, testBool, testInt ?
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.
In this case, in order to do a proper type match, the expected
value does need to be the actual type being matched (that is, an actual Bool
, actual Int
etc)
"STRING_TYPE"
is more of a convention to make it more clear from looking at the expected JSON that we are not looking for a specific String value, but just matching type; but this is not determined from the "STRING_TYPE"
value itself, rather the ValueTypeMatch
with the paths specified
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 actually mean the keys. Everywhere we have keys in format "testString", "testBool", here we have "boolObject", "doubleObject" etc...
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.
Oh I see, that's a great point! Will update to use these for the key names, thanks Arjun
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 @addb! Updated based on feedback
I also updated the EdgePathOverwriteTests.java cases to use the "stringValue"
as well
String expected = | ||
"{" + | ||
" \"xdm\": {" + | ||
" \"testString\": \"xdm\"" + |
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! However, in this case do we want to make the value under XDM distinct from the one under data
?
String expected = | ||
"{" + | ||
" \"xdm\": {" + | ||
" \"testString\": \"xdm\"," + |
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!
code/edge/src/androidTest/java/com/adobe/marketing/mobile/EdgeFunctionalTests.java
Show resolved
Hide resolved
Map<String, String> flattenedData = TestUtils.flattenMap(eventData); | ||
assertEquals(1, flattenedData.size()); | ||
assertEquals("xdm", flattenedData.get("xdm.testString")); | ||
String expected = "{\"xdm\": {\"testString\": \"testValue\"}}"; |
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!
" \"testMap\": {" + | ||
" \"key\": \"value\"" + | ||
" }," + | ||
" \"testString\": \"xdmValue\"," + |
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!
" }," + | ||
" \"xdm\": {" + | ||
" \"_id\": \"STRING_TYPE\"," + | ||
" \"boolObject\": true," + |
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.
In this case, in order to do a proper type match, the expected
value does need to be the actual type being matched (that is, an actual Bool
, actual Int
etc)
"STRING_TYPE"
is more of a convention to make it more clear from looking at the expected JSON that we are not looking for a specific String value, but just matching type; but this is not determined from the "STRING_TYPE"
value itself, rather the ValueTypeMatch
with the paths specified
code/edge/src/androidTest/java/com/adobe/marketing/mobile/EdgeFunctionalTests.java
Show resolved
Hide resolved
@@ -796,8 +916,7 @@ public void testSendEvent_twoConsecutiveCalls_appendsReceivedClientSideStore() t | |||
TIMEOUT_MILLIS | |||
); | |||
assertEquals(1, resultRequests.size()); | |||
Map<String, String> requestBody = mockNetworkService.getFlattenedNetworkRequestBody(resultRequests.get(0)); | |||
assertEquals("Expected request body with 12 elements, but found: " + requestBody, 12, requestBody.size()); | |||
assertTypeMatch("{}", resultRequests.get(0).getBodyJson(), new ElementCount(12, Subtree)); |
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: Should we add comments. Asserting body has 12 elements (kv)
…roperty names Add code comments for ElementCount cases to make assertion logic clearer
0e03c6f
into
adobe:feature/functional-test-aeptestutils
* Update EdgeFunctionalTests.java to use AEPTestUtils * Apply lint formatting to EdgeFunctionalTests.java * Update EdgePathOverwriteTests.java to use JSON comparison APIs * Apply lint formatting for EdgePathOverwriteTests.java * Update EdgeFunctionalTests.java to use TestableNetworkRequest Replace local getPayloadJson method and usages * Update EdgePathOverwriteTests.java to use TestableNetworkRequest Replace usage of local getPayloadJson * Update testSendEvent_withXDMDataAndNullData_sendsCorrectRequestEvent to update testValue for clarity * Apply lint formatting for EdgeFunctionalTests.java and EdgePathOverwriteTests.java * Update to use stringValue for test case strings in event payloads * Update EdgeFunctionalTests and TestXDMSchema to use "test" prefixed property names Add code comments for ElementCount cases to make assertion logic clearer * Remove test case comments for JSON assertions with actual expected payloads
* Update IdentityStateFunctionalTests.java to use JSON comparison APIs * Apply lint formatting to IdentityStateFunctionalTests.java * Update NetworkResponseHandlerFunctionalTests.java to use JSON comparison APIs * Apply lint formatting for NetworkResponseHandlerFunctionalTests.java * Update NoConfigFunctionalTests.java to use JSON comparison APIs * Apply lint formatting for NoConfigFunctionalTests.java * Update RestartFunctionalTests.java to use AEPTestUtils * Apply lint formatting to RestartFunctionalTests.java * Update SampleFunctionalTests.java to use JSON comparison APIs * Apply lint formatting for SampleFunctionalTests.java * Update converted test classes to use TestableNetworkRequest and remove local getJsonPayload method * Add missing step to verify the request is not sent because the configuration state is pending and not because the identity state is not set * Functional test AEPTestUtils update (part 2) (#156) * Update EdgeFunctionalTests.java to use AEPTestUtils * Apply lint formatting to EdgeFunctionalTests.java * Update EdgePathOverwriteTests.java to use JSON comparison APIs * Apply lint formatting for EdgePathOverwriteTests.java * Update EdgeFunctionalTests.java to use TestableNetworkRequest Replace local getPayloadJson method and usages * Update EdgePathOverwriteTests.java to use TestableNetworkRequest Replace usage of local getPayloadJson * Update testSendEvent_withXDMDataAndNullData_sendsCorrectRequestEvent to update testValue for clarity * Apply lint formatting for EdgeFunctionalTests.java and EdgePathOverwriteTests.java * Update to use stringValue for test case strings in event payloads * Update EdgeFunctionalTests and TestXDMSchema to use "test" prefixed property names Add code comments for ElementCount cases to make assertion logic clearer * Remove test case comments for JSON assertions with actual expected payloads * Update AEPTestUtils to latest version * Update test case setup for testHandleExperienceEventRequest_withPendingConfigurationState_expectEventsQueueIsBlocked to set an identity state beforehand * Apply lint formatting for NoConfigFunctionalTests.java * Add test case comments for clarity * RestartFunctionalTests.java - update tearDown to reset all test helpers
* Functional test AEPTestUtils update (part 1) (#155) * Replace local test utils with AEPTestUtils * Update ConfigOverridesFunctionalTests.kt to use AEPTestUtils * Remove local test-utils files * Update CompletionHandlerFunctionalTests.java to use AEPTestUtils * Update ConsentStatusChangeFunctionalTests.java to use AEPTestUtils Apply lint formatting * Update ConfigOverridesFunctionalTests.kt to use static assertExactMatch * Update AEPTestUtils to the latest version * Remove unconverted functional tests * Revert "Remove unconverted functional tests" This reverts commit 2d0013a. * Update AEPTestUtils to latest version * Update CompletionHandlerFunctionalTests.java to use TestableNetworkRequest * Update ConsentStatusChangeFunctionalTests.java to use TestableNetworkRequest type Update list get operations to safe versions * Update ConsentStatusChangeFunctionalTests.java to use TestableNetworkRequest * ConsentStatusChangeFunctionalTests.java add collection size check before access * Remove Long conversion for result list size check in ConfigOverridesFunctionalTests.kt * Functional test AEPTestUtils update (part 2) (#156) * Update EdgeFunctionalTests.java to use AEPTestUtils * Apply lint formatting to EdgeFunctionalTests.java * Update EdgePathOverwriteTests.java to use JSON comparison APIs * Apply lint formatting for EdgePathOverwriteTests.java * Update EdgeFunctionalTests.java to use TestableNetworkRequest Replace local getPayloadJson method and usages * Update EdgePathOverwriteTests.java to use TestableNetworkRequest Replace usage of local getPayloadJson * Update testSendEvent_withXDMDataAndNullData_sendsCorrectRequestEvent to update testValue for clarity * Apply lint formatting for EdgeFunctionalTests.java and EdgePathOverwriteTests.java * Update to use stringValue for test case strings in event payloads * Update EdgeFunctionalTests and TestXDMSchema to use "test" prefixed property names Add code comments for ElementCount cases to make assertion logic clearer * Remove test case comments for JSON assertions with actual expected payloads * Functional test AEPTestUtils update (part 3) (#157) * Update IdentityStateFunctionalTests.java to use JSON comparison APIs * Apply lint formatting to IdentityStateFunctionalTests.java * Update NetworkResponseHandlerFunctionalTests.java to use JSON comparison APIs * Apply lint formatting for NetworkResponseHandlerFunctionalTests.java * Update NoConfigFunctionalTests.java to use JSON comparison APIs * Apply lint formatting for NoConfigFunctionalTests.java * Update RestartFunctionalTests.java to use AEPTestUtils * Apply lint formatting to RestartFunctionalTests.java * Update SampleFunctionalTests.java to use JSON comparison APIs * Apply lint formatting for SampleFunctionalTests.java * Update converted test classes to use TestableNetworkRequest and remove local getJsonPayload method * Add missing step to verify the request is not sent because the configuration state is pending and not because the identity state is not set * Functional test AEPTestUtils update (part 2) (#156) * Update EdgeFunctionalTests.java to use AEPTestUtils * Apply lint formatting to EdgeFunctionalTests.java * Update EdgePathOverwriteTests.java to use JSON comparison APIs * Apply lint formatting for EdgePathOverwriteTests.java * Update EdgeFunctionalTests.java to use TestableNetworkRequest Replace local getPayloadJson method and usages * Update EdgePathOverwriteTests.java to use TestableNetworkRequest Replace usage of local getPayloadJson * Update testSendEvent_withXDMDataAndNullData_sendsCorrectRequestEvent to update testValue for clarity * Apply lint formatting for EdgeFunctionalTests.java and EdgePathOverwriteTests.java * Update to use stringValue for test case strings in event payloads * Update EdgeFunctionalTests and TestXDMSchema to use "test" prefixed property names Add code comments for ElementCount cases to make assertion logic clearer * Remove test case comments for JSON assertions with actual expected payloads * Update AEPTestUtils to latest version * Update test case setup for testHandleExperienceEventRequest_withPendingConfigurationState_expectEventsQueueIsBlocked to set an identity state beforehand * Apply lint formatting for NoConfigFunctionalTests.java * Add test case comments for clarity * RestartFunctionalTests.java - update tearDown to reset all test helpers
Description
This PR updates the second part of functional test classes:
EdgeFunctionalTests.java
EdgePathOverwriteTests.java
to use the AEPTestUtils library, with the following changes:
flatten
.get()
is protected with size checksRelated Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: