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

Jump to bottom

Add Equal and Not Equal tests for Tuples with null fields. #3

Merged
merged 4 commits into from
Apr 22, 2024
Merged

Add Equal and Not Equal tests for Tuples with null fields. #3

merged 4 commits into from
Apr 22, 2024

Conversation

suyashkumar
Copy link
Contributor

@suyashkumar suyashkumar commented Mar 7, 2024 •
edited
Loading

This change adds equal and not equal tests for Tuples that have missing values, based on clarifications and discussions in the Zulip Chat.

We may wish to make a change to add cases like this for Equivalent and Not Equivalent too, but I limited this PR to the discussion at hand.

Questions:

  • We don't yet have any CI for these tests yet, so what testing or validation do we wish to do with these before submitting? I have manually run these in the Java engine successfully (except for ones where Bryn said the Java behavior is incorrect).
  • I wasn't sure about the test naming convention, so took a random stab at it.

@suyashkumar suyashkumar marked this pull request as ready for review March 7, 2024 22:43
@suyashkumar suyashkumar changed the title Add Equal and Not Equal tests for Tuples with missing values. Add Equal and Not Equal tests for Tuples containing null fields. Mar 7, 2024
@suyashkumar suyashkumar changed the title Add Equal and Not Equal tests for Tuples containing null fields. Add Equal and Not Equal tests for Tuples with null fields. Mar 7, 2024
@marciantemark marciantemark added the CQLTest: CQL-test-code Actual tests stored or changed in this repo. Closed through PR and no HL7 balloting required. label Mar 21, 2024
@brynrhodes brynrhodes mentioned this pull request Mar 29, 2024
Open
suyashkumar
suyashkumar commented Mar 29, 2024
View reviewed changes
Copy link
Contributor Author

@suyashkumar suyashkumar left a comment

Choose a reason for hiding this comment

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

Thanks!

tests/cql/CqlComparisonOperatorsTest.xml Show resolved Hide resolved
tests/cql/CqlComparisonOperatorsTest.xml Show resolved Hide resolved
@suyashkumar
Copy link
Contributor Author

@brynrhodes I made the changes as discussed, let me know if you've got any other thoughts! Thanks!

@suyashkumar
Copy link
Contributor Author

@JPercival @EvanMachusakNCQA if y'all have a moment to give a second review in addition to Bryn's, that'd be great--thanks!

@EvanMachusakNCQA
Copy link
Contributor

We pass every test except EquivEqCM1M01 because we don't support mixed-unit equivalence, so we're good.

@EvanMachusakNCQA EvanMachusakNCQA merged commit 5e3c367 into cqframework:main Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brynrhodes brynrhodes brynrhodes approved these changes

@EvanMachusakNCQA EvanMachusakNCQA EvanMachusakNCQA approved these changes

@JPercival JPercival Awaiting requested review from JPercival

Assignees

@suyashkumar suyashkumar

Labels
CQLTest: CQL-test-code Actual tests stored or changed in this repo. Closed through PR and no HL7 balloting required.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@suyashkumar @EvanMachusakNCQA @brynrhodes @marciantemark