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 - 1156-range-function #658

Closed StrayAlien closed 2 months ago

StrayAlien commented 5 months ago

The new range function gets exercised. The spec is very specific about what can be passed to this function as a string. These tests seek to cover all that.

Like:

I understand there is a PR in already for the range function but (apols @SimonRinguette) I am throwing in these tests as they delve deeper into the permitted grammar, plus error conditions and named params and types.

These tests only do some basic sanity checking on ranges and do not test range operation. We have other tests for asserting range correctness. Many of the tests here use equivalence to determine correctness - that is, if the parsed range is equal to a literal range with the same endpoints and inclusivity then it is the same so no range checking required.

baldimir commented 5 months ago

Discussed on the meeting - let's this review @dmn-tck/contributors. There is another one from @SimonRinguette https://github.com/dmn-tck/tck/pull/620. There may be some overlap of tests.

SimonRinguette commented 5 months ago

I have reviewed the tests. They are much more complete than the PR i've originally submitted... thank you @StrayAlien.

Feel free to close my original PR and use this one instead, I validated the logic on the Trisotech engine.

StrayAlien commented 5 months ago

Thanks @SimonRinguette. Appreciated. I have a few more tests to add in here as well - on their way.

opatrascoiu commented 3 months ago

It would be great if the assertions to 'instance of XXX' were replaced with assertions on the flags & values for endpoints to make sure we build the range objects in the same way :)

StrayAlien commented 3 months ago

hey @opatrascoiu . Thanks. The "instance of" tests are just one aspect of the assertions here - they're also supported with equality tests like range("[18..21]") = [18..21]. So, IMO, no need to test the flags and values here as we already have other tests in the TCK for range equality that do assert on those things. If that makes sense.

baldimir commented 2 months ago

Discussed on the meeting, this one can get merged. Merging.