Conversation
evan-gordon
left a comment
evan-gordon
left a comment
There was a problem hiding this comment.
Left a few comments, happy to approve this once they're addressed.
| <output>false</output> | ||
| </test> | ||
| <test name="SimpleEqInt1Int2Long"> | ||
| <expression>10L = 20L</expression> |
There was a problem hiding this comment.
This test name implies that the left argument should be 10 not 10L no?
There was a problem hiding this comment.
Hi @evan-gordon I changed the name to SimpleEqLong1Long2 and expression to 1L = 2L. It's now cleaner and consistent with Int1, Int2 elsewhere in the file.
The tests in https://github.com/cqframework/clinical_quality_language/blob/dfafdb9/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlComparisonOperatorsTest.cql can later be updated to match this.
| <expression>1'cm':2'cm' = 1'cm':2'cm'</expression> | ||
| <output>true</output> | ||
| </test> | ||
| <test name="RatioNotEqual"> |
There was a problem hiding this comment.
In this case the numerators aren't equal, might be worth adding a case for denominators as well.
There was a problem hiding this comment.
I've replaced this with two tests: RatioNotEqualDiffNumerator and RatioNotEqualDiffDenominator.
There was a problem hiding this comment.
Same for RatioNotEquivalent tests
| <expression>0 > 1</expression> | ||
| <output>false</output> | ||
| </test> | ||
| <test name="GreaterLong"> |
There was a problem hiding this comment.
It looks like there are many tests in this file with false as expected value, yet they don't have False in the test name. See also https://github.com/cqframework/clinical_quality_language/blob/dfafdb9/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlComparisonOperatorsTest.cql .
| <output>true</output> | ||
| </test> | ||
| <test name="SimpleNotEqInt1Int2Long"> | ||
| <expression>10L != 20L</expression> |
There was a problem hiding this comment.
Similar to my above comment, shouldn't the left argument be an int given this test name?
There was a problem hiding this comment.
I changed the name to SimpleNotEqLong1Long2 and expression to 1L != 2L
3 participants
This PR adds new conformance tests for comparison operators. I picked all the ones from https://github.com/cqframework/clinical_quality_language/blob/dfafdb9/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlComparisonOperatorsTest.cql which didn't exist yet here.