Conversation
* 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
* 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
Codecov Report
@@ Coverage Diff @@
## dev #415 +/- ##
=======================================
Coverage 96.77% 96.77%
=======================================
Files 27 27
Lines 1672 1672
=======================================
Hits 1618 1618
Misses 54 54 |
# Conflicts: # AEPEdge.xcodeproj/project.pbxproj
timkimadobe
left a comment
There was a problem hiding this comment.
Thank you for the review @emdobrin! Updated based on feedback, with new PR from fork for the changes
| // 1. Mark the path end (where the value is a `String`), or | ||
| // 2. Contain the asterisk (*) character. | ||
| let pathEndKeys = pathTree?.filter { key, value in | ||
| value is String || key.contains("*") |
There was a problem hiding this comment.
This might not work as expected if the key has * in it eg: awesome*key. Should we check startsWith and EndsWith '*'? applies to Android code as well.
There was a problem hiding this comment.
These pathEndKeys are only used in the context of array index path end keys. It's ok at this stage if keys like awesome*key are captured in this list since the else block handles:
- filtering out all keys that dont strictly have array access style notation via the
arrayIndexValueRegex: "[123]" (awesome*keywould be filtered out here) - sanitization of invalid non integer values: "[awesome*key]" -> "awesomekey" -> cannot be converted to Int and is filtered out
Hopefully this addresses the case mentioned, or were you thinking of something else?
There was a problem hiding this comment.
Yes, I saw that we will drop it while after casting, but I was thinking if we could set a preferred way to do this. Like it has to start with * or end with *. So we don't do not necessary casting and also keep it standardized.
How about "1*2" for the above test?
There was a problem hiding this comment.
I think standardization could definitely be helpful for users of the method; what do you think of emitting information/test errors based on non-standard formats, since we have this information at parse time?
For example, if the * is only allowed at the start or end, then [1*2] would emit a test failure as a setup issue. I feel like standardization without this kind of correction/warning could be less useful? Although I do also see the benefit of format consistency for writers and maintainers of test cases as well
In the current implementation, [1*2] will be interpreted as: wildcard index 12
There was a problem hiding this comment.
Update: The wildcard array format and related logic has been refactored in #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 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
Description
This PR:
The changes are covered in detail in the following feature branch PRs:
1.
Utility updates: #401
Test cases updates: #402
2.
Wildcard index refactor: #416
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: