-
Notifications
You must be signed in to change notification settings - Fork 111
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
Implement remaining 'denotes' expectations for conformance test runner #999
Implement remaining 'denotes' expectations for conformance test runner #999
Conversation
@@ -59,7 +60,7 @@ private fun IonReader.walk(): List<String> { | |||
while (true) { | |||
next() | |||
val currentType = type | |||
if (type == null) { | |||
if (currentType == null) { |
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.
🗺️ This is a minor drive-by improvement that stops the IDE from complaining later on that currentType
could be null
.
@@ -141,20 +142,20 @@ private fun TestCaseSupport.denotesModelContent(modelContent: AnyElement, reader | |||
assertEquals(IonType.STRING, reader.type, failureContext) | |||
assertEquals(modelContent.stringValue, reader.stringValue(), failureContext) | |||
} | |||
is SexpElement -> when (modelContent.head) { | |||
is SeqElement -> when (modelContent.head) { |
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.
🗺️ Switch to SeqElement
ensures that the denotes
clause works even if the test is written in JSON instead of Ion. That might not be very relevant in ion-java, but it was an easy change to get some more completeness.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## ion-11-encoding #999 +/- ##
==================================================
Coverage ? 70.59%
Complexity ? 7095
==================================================
Files ? 203
Lines ? 28242
Branches ? 5037
==================================================
Hits ? 19937
Misses ? 6717
Partials ? 1588 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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, one comment:
Co-authored-by: Tyler Gregg <[email protected]>
Issue #, if available:
None.
Description of changes:
Implements
denotes
for float, decimal, timestamp, and for structs that don't have duplicate field names.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.