-
Notifications
You must be signed in to change notification settings - Fork 124
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
Update ProperContains evaluator implementation #1393
Update ProperContains evaluator implementation #1393
Conversation
Formatting check succeeded! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1393 +/- ##
============================================
+ Coverage 63.70% 63.73% +0.03%
+ Complexity 2666 2665 -1
============================================
Files 493 493
Lines 27750 27774 +24
Branches 5513 5521 +8
============================================
+ Hits 17677 17703 +26
Misses 7830 7830
+ Partials 2243 2241 -2 ☔ View full report in Codecov by Sentry. |
This looks good @antvaset , my only question is is there a corresponding PR against the cql-tests repository to update these tests? |
Depends on cqframework/cql-tests#40 |
Hi @brynrhodes, thanks for having a look! I've opened cqframework/cql-tests#40 in cql-tests to add the new tests for both |
Note also cqframework/cql-tests#48 and #1394. |
This PR includes the following changes related to ProperContains:
ProperContainsNullRightFalse
andProperContainsNullRightTrue
tests to make sure they are parsed asProperContains(List<T>, T)
(to match the test name/group). Withoutnull as String
, the expressionswere parsed as
ProperIncludes(List<T>, List<T>)
.Set the expected values of the library-based
ProperContainsTimeNull
andProperInTimeNull
tests to null to match those of the XML-basedProperContainsTimeNull
(link) andProperInTimeNull
(link).Update implementation of ProperContains evaluator to match the spec (link).
Add tests covering the edge cases in the ProperContains logic.