Skip to content

Add tests for ProperIn#1394

Closed
antvaset wants to merge 10 commits intomasterfrom
proper-in-tests
Closed

Add tests for ProperIn#1394
antvaset wants to merge 10 commits intomasterfrom
proper-in-tests

Conversation

@antvaset
Copy link
Copy Markdown
Contributor

@antvaset antvaset commented Jul 31, 2024

This PR adds tests for the ProperIn evaluator following the changes to ProperContains in #1393 to align it with the spec. Internally, ProperIn uses the ProperContains logic entirely because the operators are the same except for the order of the arguments.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 31, 2024

Formatting check succeeded!

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 31, 2024

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.37%. Comparing base (5e6b939) to head (ce6c8a2).
⚠️ Report is 78 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1394      +/-   ##
============================================
+ Coverage     64.36%   64.37%   +0.01%     
  Complexity     1921     1921              
============================================
  Files           494      494              
  Lines         28131    28131              
  Branches       5588     5588              
============================================
+ Hits          18106    18110       +4     
+ Misses         7772     7769       -3     
+ Partials       2253     2252       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@antvaset
Copy link
Copy Markdown
Contributor Author

Depends on cqframework/cql-tests#40

@antvaset antvaset marked this pull request as ready for review July 31, 2024 05:25
Base automatically changed from update-proper-contains-evaluator-implementation to master August 1, 2024 20:11
@JPercival JPercival enabled auto-merge (squash) August 1, 2024 20:18
</test>
<test name="ProperIn9">
<expression>'a' properly included in { 'a', null }</expression>
<output>null</output>
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

JPercival and others added 3 commits August 15, 2024 16:05
* WIP

* WIP

* WIP

* Working?

* Fix usage of Java 17 API

* Update checkstyle rules

* Fix static analysis

* Updates to src dirs

* Fixing up missing test sourceSet

* Try another way to specify the antlr directory

* Third different way

* Add some logging

* merge master

* Change toolchain resolution

* Fix tests

* More tweaks to animalsniffer

* Fix formatting

* Trying random stuff

* More random stuff

* Small improvements for Gradle build (#1418)

* Removing references to idea

* Fix duplicative generation

* Remove references to idea project generation

---------

Co-authored-by: Anton Vasetenkov <antvaset@gmail.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Nov 6, 2024

@antvaset antvaset closed this Aug 11, 2025
auto-merge was automatically disabled August 11, 2025 13:00

Pull request was closed

@JPercival JPercival deleted the proper-in-tests branch October 2, 2025 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants