Closed yesamer closed 2 months ago
@yesamer @baldimir
I'm not 100% sure of the underlying problem.
From the spec, I see that both context
and context merge
requires a list
of contexts.
I agree that the list could contain only one single item, but regardless the dmn model should define it as list, e.g.
[context_parameter]
instead of context_parameter
.
WIth this fix, we basically
If my understanding is correct, it would mean that users could create an invalid model, that works only with our implementation.
Does this make sense ? Am I wrong on some assumption ?
@gitgabrio I agree with you, but the point is that the TCK tests expect that kind of behavior. That means other vendors that successfully perform the TCK tests already implemented that. So, the alternative to merging this PR is to raise this point to the TCK meeting and double-check if those tests are correct or not, considering that the specs don't define those methods specifically.
@gitgabrio I agree with you, but the point is that the TCK tests expect that kind of behavior. That means other vendors that successfully perform the TCK tests already implemented that. So, the alternative to merging this PR is to raise this point to the TCK meeting and double-check if those tests are correct or not, considering that the specs don't define those methods specifically.
Yup, exactly. IIUC the process, those tests does not comes from the committee itself, but from the tck group that, again IIUC, does not have any real ownership on the specs. If those tests does not reflect the actual spec, then they are invalid, and I'm not sure why they are there and why, if, that behavior has been implemented. If all the above is true, IMO, those tests (and all the others that does not reflect the actual spec) should be deleted, or at least ignored.
Ok, let's put this on hold and raise the point at the TCK meeting
PR job #1
was: UNSTABLE
Possible explanation: This should be test failures
Please look here: https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-5997/1/display/redirect
Test results:
Those are the test failures:
PR job #3
was: UNSTABLE
Possible explanation: This should be test failures
Please look here: https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-5997/3/display/redirect
Test results:
Those are the test failures:
Is this ready for review after the discussions please? The coercion from list with a single item to single item and vice versa is defined in the spec in 10.3.2.9.4 Type conversions. The TCK tests are valid.
@baldimir Still a WIP
PR job #5
was: UNSTABLE
Possible explanation: This should be test failures
Please look here: https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-5997/5/display/redirect
Test results:
Those are the test failures:
@gitgabrio @baldimir Ready to be reviewed
Closes: https://github.com/apache/incubator-kie-issues/issues/1330
According to the Specs:
10.3.2.9.4 Type conversions to singleton list: When the type of the expression is T and the target type is List the expression is converted to a singleton list. from singleton list: When the type of the expression is List, the value of the expression is a singleton list and the target type is T, the expression is converted by unwrapping the first element.
This PR implements the second part of that paragraph: passing any single element to any function that requires a list, the code will automatically coerce it to a List with a single element.
As a consequence, I removed all the methods in the function that take the single element when expecting a list.