dmn-tck / tck

Decision Model and Notation Technology Compatibility Kit
https://dmn-tck.github.io/tck
51 stars 36 forks source link

DMN 1.5 - feel 'in' additions for '=' and '!='. Plus ... #668

Open StrayAlien opened 6 months ago

StrayAlien commented 6 months ago

uncommenting null endpoint ranges tests as per: https://github.com/dmn-tck/tck/pull/393#issuecomment-1396960083.

The '=' and '!=' get exercised for each data type.

Those null range tests were commented out due to pre-1.5 ranges not permitting expressions as endpoints. They may or may not be correct in the light of the new changes.

It is not clear to me whether [null..10] is the same as <=10, or a null endpoint should now be treated as an error condition.

opatrascoiu commented 5 months ago

I believe some tests involving ranges with null endpoints do not follow the semantics of DMN 1.5. For example,

null_001_a: 5 in (null..10]

According to 10.3.2.7 Ranges the operational semantics is delegated to the semantics of existing relational and boolean operators:

The semantics of comparison expressions involving ranges (grammar rules 49c and 49d) is defined in Table 55, Table 54, Table 52, and Table 50.

According to these tables

5 in (null..10] <=> 5 > null and 5 <= 10 <=> null and true <=> null (3-value logic)

According to Table 55, condition 5 in (null..10] is not equivalent with 5 in <=10.

StrayAlien commented 5 months ago

I have to admit ... I am now really pretty confused at to how null in ranges and the 'unary comparison' style psuedo-ranges are supposed to behave. And what is equal to what, and what 'undefined' is. And whether attempting to get a value that is undefined actually should be an error .. and give null .. etc.

Like, we have tests in the equivalence suite asserting that (<= 10) = (null..10]. In 1.4, the start range of <=10 was null. Now it is 'undefined' .. whatever that is.

Raised: https://github.com/dmn-tck/tck/issues/663 .. to discuss.

opatrascoiu commented 1 month ago

@StrayAlien I checked the latest status of the PR. We are very close to merging :)

I believe the expected value for tests null_001_a and null_001_bshould be null based on the rationale above: https://github.com/dmn-tck/tck/pull/668#issuecomment-2145284047

All the others look good to me.

SimonRinguette commented 1 week ago

I agree with Octavian interpretation of the result of null_001_a and null_001_b. Ok with the other changes

baldimir commented 1 week ago

@StrayAlien could you please take a look, based on the last comments from Simon and Octavian?