Skip to content
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.

Sign up for GitHub

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

Add tests for ProperIn #1394

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Add tests for ProperIn #1394

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1f3e097
Update ProperContains evaluator implementation
antvaset Jul 30, 2024
78383ff
Align library-based tests with XML-based tests
antvaset Jul 30, 2024
7b7b7db
Unskip test
antvaset Jul 30, 2024
03db5ab
Add tests for ProperIn
antvaset Jul 31, 2024
9e6e470
Merge branch 'master' into proper-in-tests
antvaset Aug 1, 2024
8983b9c
Merge branch 'master' into proper-in-tests
antvaset Aug 1, 2024
2174744
Merge branch 'master' into proper-in-tests
JPercival Aug 2, 2024
afe9392
Merge branch 'master' into proper-in-tests
JPercival Aug 15, 2024
6af4b68
Swap Gradle language from Groovy to Kotlin (#1413)
JPercival Nov 6, 2024
ce6c8a2
Merge branch 'master' into proper-in-tests
antvaset Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 42 additions & 2 deletions Src/java/engine-fhir/src/test/resources/org/hl7/fhirpath/cql/CqlListOperatorsTest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,12 +720,52 @@
</test>
</group>
<group name="ProperIn">
<test name="ProperIn1">
<expression>'a' properly included in null as List&lt;String&gt;</expression>
<output>false</output>
</test>
<test name="ProperIn2">
<expression>'a' properly included in {}</expression>
<output>false</output>
</test>
<test name="ProperIn3">
<expression>'a' properly included in { 'a' }</expression>
<output>false</output>
</test>
<test name="ProperIn4">
<expression>null as String properly included in { null }</expression>
<output>false</output>
</test>
<test name="ProperInNullRightFalse">
<expression>null properly included in {'s', 'u', 'n'}</expression>
<expression>null as String properly included in {'s', 'u', 'n'}</expression>
<output>false</output>
</test>
<test name="ProperIn5">
<expression>null as String properly included in { null, null }</expression>
<output>false</output>
</test>
<test name="ProperInNullRightTrue">
<expression>null properly included in {'s', 'u', 'n', null}</expression>
<expression>null as String properly included in {'s', 'u', 'n', null}</expression>
<output>true</output>
</test>
<test name="ProperIn6">
<expression>'a' properly included in { 'a', 'b' }</expression>
<output>true</output>
</test>
<test name="ProperIn7">
<expression>'a' properly included in { 'a', 'a' }</expression>
<output>false</output>
</test>
<test name="ProperIn8">
<expression>'c' properly included in { 'a', 'b' }</expression>
<output>false</output>
</test>
<test name="ProperIn9">
<expression>'a' properly included in { 'a', null }</expression>
<output>null</output>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this should be true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @brynrhodes my reading of the spec https://cql.hl7.org/04-logicalspecification.html#proper-in:

For the T, List overload, this operator returns [true] if the given element is in the given list, and it is not the only element in the list, using equality semantics, with the exception that null elements are considered equal. If the first argument is null, the result is true if the list contains any null elements and at least one other element, and false otherwise. If the second argument is null, the result is false.

is that:

null properly included in { 'a', null } = true // nulls are equal and there is at least one non-null in the list (spec is explicit)
null properly included in { null, null } = false // nulls are equal but there are no non-nulls in the list (spec is explicit)
'a' properly included in { 'a', 'b' } = true // the list has at least one element for which Equals(x, y) evaluates to true and at least one for which Equals(x, y) evaluates to false (spec is explicit)
'a' properly included in { 'a', 'b', null } = true // same as above
'a' properly included in { 'a', 'a' } = false // the list has at least one element for which Equals(x, y) evaluates to true, no elements for which Equals(x, y) evaluates to false, and no elements for which Equals(x, y) evaluates to null (spec is only explicit that the result is not true)
'a' properly included in { 'a', null } = null // the list has at least one element for which Equals(x, y) evaluates to true, no elements for which Equals(x, y) evaluates to false, but there are elements for which Equals(x, y) evaluates to null (spec is only explicit that the result is not true)

This is consistent with the existing null-valued ProperInTimeNull test in the same file that covers the ProperIn(T, Interval<T>) overload, indicating that we can return nulls from ProperIn, not just true and false.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also whichever we go with, I'll add the short explainers as comments to the XMLs.

</test>
<test name="ProperIn10">
<expression>'a' properly included in { 'a', 'b', null }</expression>
<output>true</output>
</test>
<test name="ProperInTimeTrue">
Expand Down
13 changes: 9 additions & 4 deletions ...java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/ProperInEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@
T, Interval : The type of T must be the same as the point type of the interval.

For the T, List overload, this operator returns if the given element is in the given list,
and it is not the only element in the list, using equivalence semantics.
If the list-valued argument is null, it should be treated as an empty list.
and it is not the only element in the list, using equality semantics, with the exception
that null elements are considered equal.
If the first argument is null, the result is true if the list contains any null elements
and at least one other element, and false otherwise.
If the second argument is null, the result is false.

For the T, Interval overload, this operator returns true if the given point is greater than
the starting point, and less than the ending point of the interval, as determined by the Start and End operators.
If precision is specified and the point type is a date/time type, comparisons used in the operation are performed
at the specified precision.
If precision is specified and the point type is a Date, DateTime, or Time type, comparisons used in the operation
are performed at the specified precision.
If the first argument is null, the result is null.
If the second argument is null the result is false.
*/

import org.opencds.cqf.cql.engine.execution.State;
Expand Down
4 changes: 2 additions & 2 deletions ...a/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlListOperatorsTest.cql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ define ProperContainsTimeTrue: { @T15:59:59, @T20:59:59.999, @T20:59:49.999 } pr
define ProperContainsTimeNull: { @T15:59:59.999, @T20:59:59.999, @T20:59:49.999 } properly includes @T15:59:59

//ProperIn
define ProperInNullRightFalse: null properly included in {'s', 'u', 'n'}
define ProperInNullRightTrue: null properly included in {'s', 'u', 'n', null}
define ProperInNullRightFalse: null as String properly included in {'s', 'u', 'n'}
define ProperInNullRightTrue: null as String properly included in {'s', 'u', 'n', null}
define ProperInTimeTrue: @T15:59:59 properly included in { @T15:59:59, @T20:59:59.999, @T20:59:49.999 }
define ProperInTimeNull: @T15:59:59 properly included in { @T15:59:59.999, @T20:59:59.999, @T20:59:49.999 }

Expand Down
Loading