JSON comparison parity - test cases#402
Merged
timkimadobe merged 5 commits intoadobe:feature/json-comparison-parityfrom Sep 29, 2023
Merged
JSON comparison parity - test cases#402timkimadobe merged 5 commits intoadobe:feature/json-comparison-parityfrom
timkimadobe merged 5 commits intoadobe:feature/json-comparison-parityfrom
Conversation
Splits coverage into parameterized and special case tests, and update provides more comprehensive systematic coverage
Codecov Report
@@ Coverage Diff @@
## feature/json-comparison-parity #402 +/- ##
===============================================================
Coverage 96.77% 96.77%
===============================================================
Files 27 27
Lines 1671 1671
===============================================================
Hits 1617 1617
Misses 54 54 |
addb
reviewed
Sep 20, 2023
Tests/UpstreamIntegrationTests/AnyCodableAssertsParameterizedTests.swift
Outdated
Show resolved
Hide resolved
addb
reviewed
Sep 20, 2023
Tests/UpstreamIntegrationTests/AnyCodableAssertsParameterizedTests.swift
Show resolved
Hide resolved
… boolean cases Update index helper comments based on changed values
timkimadobe
commented
Sep 20, 2023
Contributor
Author
timkimadobe
left a comment
There was a problem hiding this comment.
Thanks so much for the review @addb! Updated based on feedback
Tests/UpstreamIntegrationTests/AnyCodableAssertsParameterizedTests.swift
Outdated
Show resolved
Hide resolved
Tests/UpstreamIntegrationTests/AnyCodableAssertsParameterizedTests.swift
Show resolved
Hide resolved
addb
reviewed
Sep 21, 2023
Tests/UpstreamIntegrationTests/AnyCodableAssertsParameterizedTests.swift
Outdated
Show resolved
Hide resolved
addb
reviewed
Sep 28, 2023
| (expected: #"{ "\\": 1 }"#, actual: #"{ "\\": 1 }"#), // Backslash | ||
| (expected: #"{ "\\\\": 1 }"#, actual: #"{ "\\\\": 1 }"#), // Double backslash | ||
| (expected: #"{ ".": 1 }"#, actual: #"{ ".": 1 }"#), // Dot | ||
| (expected: #"{ "k.1.2.3": 1 }"#, actual: #"{ "k.1.2.3": 1 }"#), // Dat in key |
Contributor
There was a problem hiding this comment.
typo in comment
// Dot in key
Contributor
Author
There was a problem hiding this comment.
Thank you! Fixed
addb
reviewed
Sep 29, 2023
| (path: "[*]", expected: true, actual: false, format: { #"[\#($0)]"# }), | ||
| ] | ||
| let testCases = rawCases.map { tuple in | ||
| let expectedAnyCodable: AnyCodable? = getAnyCodable(tuple.format(tuple.expected)) |
Contributor
There was a problem hiding this comment.
For all related tests.
nit For Readability: can we also create actualAnyCodable:
let actualAnyCodable: AnyCodable? = getAnyCodable(tuple.format(tuple.actual))
Contributor
Author
There was a problem hiding this comment.
Thank you! I think this is a great change, updated all test cases that have inline actual creation
addb
approved these changes
Sep 29, 2023
Contributor
addb
left a comment
There was a problem hiding this comment.
LGTM. Just few minor comments.
timkimadobe
commented
Sep 29, 2023
Contributor
Author
timkimadobe
left a comment
There was a problem hiding this comment.
Thanks so much for the review @addb! Updated based on feedback
| (expected: #"{ "\\": 1 }"#, actual: #"{ "\\": 1 }"#), // Backslash | ||
| (expected: #"{ "\\\\": 1 }"#, actual: #"{ "\\\\": 1 }"#), // Double backslash | ||
| (expected: #"{ ".": 1 }"#, actual: #"{ ".": 1 }"#), // Dot | ||
| (expected: #"{ "k.1.2.3": 1 }"#, actual: #"{ "k.1.2.3": 1 }"#), // Dat in key |
Contributor
Author
There was a problem hiding this comment.
Thank you! Fixed
| (path: "[*]", expected: true, actual: false, format: { #"[\#($0)]"# }), | ||
| ] | ||
| let testCases = rawCases.map { tuple in | ||
| let expectedAnyCodable: AnyCodable? = getAnyCodable(tuple.format(tuple.expected)) |
Contributor
Author
There was a problem hiding this comment.
Thank you! I think this is a great change, updated all test cases that have inline actual creation
10 tasks
timkimadobe
added a commit
that referenced
this pull request
Oct 12, 2023
* JSON comparison parity (utility only) (#401) * Remove custom helper enums and refactor usages Update documentation and code logic to match Android implementation * Apply lint autocorrect * Revert "Apply lint autocorrect" This reverts commit 7f38d19. * Apply lint autocorrect * Reduce wordiness of file and line param descriptions * Update documentation null to nil * Update file and line param descriptions * Add method documentation to all helper methods Update param name of extractArrayStyleComponents to be more intuitively descriptive * JSON comparison parity - test cases (#402) * Achieves parity with the Android Edge test setup for JSON comparison Splits coverage into parameterized and special case tests, and update provides more comprehensive systematic coverage * Add additional test cases to validate type mismatch detection and all boolean cases Update index helper comments based on changed values * Update failure cases to check for unintended type casting * Fix comment typo * Extract inline actual AnyCodable creation to separate line for clarity * Refactor wildcard index logic (#416) * Refactor wildcard index access to flatten logic since different condition and replacement groups are not needed * Reformat method documentation to bring them all together * Rename variables to better reflect what each one stores * Refactor wildcard array index extraction to strictly validate format Extract wildcard array index extraction logic into separate function Update usages, test cases, and method documentation * Update test failure message to include example format * Rename `extractArrayStyleComponents` and update method docs (#418) * Rename the extractArrayStyleComponents -> extractArrayFormattedComponents to better reflect actual behavior Update method documentation to reflect the rename and to better explain the logic * Update parameter description * Update method description * Include information about removing escape character
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR splits test coverage into parameterized and unit test cases, and provides more comprehensive, systematic coverage of the JSON comparison feature. It brings the iOS implementation in line with the latest Android changes.
Please see adobe/aepsdk-edge-android#79 for reference, specifically:
Question for reviewers:
The parameterized tests use
XCTContext.runActivityto create separate test cases, however errors in Xcode are presented differently; now the left side panel Test navigator that shows the individual test cases serves as an overall pass/fail indicator for the whole group of parameterized tests - is this an acceptable change? Are there any other format or logging updates that are required in this case?and individual parameterized test cases can be seen in the Report navigator.
The following is an example of the console output that would be seen in Xcode and CI systems:
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: