-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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.
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
[incubator-kie-issues-1752] TCK failure-Lambda function returns null #6261
base: main
Are you sure you want to change the base?
Conversation
kie-dmn/kie-dmn-feel/src/test/java/org/kie/dmn/feel/lang/types/GenFnTypeTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor points to address (formatting)
@@ -1,4 +1,5 @@ | |||
/** | |||
/* | |||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bncriju Please remove line 2
@@ -0,0 +1,228 @@ | |||
/* | |||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bncriju Please remove line 2
} | ||
if (o instanceof FEELFunction oFn) { | ||
List<List<Param>> parameters = oFn.getParameters(); | ||
if(parameters.isEmpty()){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if(parameters.isEmpty()){ | |
if (parameters.isEmpty()) { |
Please use the same formatting style of the code base
IntStream.range(0, argsGen.size()).allMatch(i -> fnT.argsGen.get(i).conformsTo(this.argsGen.get(i))) && | ||
this.returnGen.conformsTo(fnT.returnGen); | ||
IntStream.range(0, argsGen.size()).allMatch(i -> fnT.argsGen.get(i).conformsTo(this.argsGen.get(i))) && | ||
this.returnGen.conformsTo(fnT.returnGen); | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bncriju Please remove those 2 whitespaces
List<List<Param>> parameters = oFn.getParameters(); | ||
if(parameters.isEmpty()){ | ||
//this is used to consider function as parameter | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HI @bncriju
on second thought, I think we need to implement more check here, i.e. if the expected input/output of the current GenFnType are compatible with invoke methods of the provided function
# Conflicts: # kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/ast/RangeNode.java
PR job Reproducerbuild-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-drools -u #6261 --skipParallelCheckout NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution Please look here: https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6261/17/display/redirect Test results:
Those are the test failures: org.kie.dmn.feel.runtime.functions.BaseFEELFunctionTest.testIsCompatibleTrueExpecting value to be true but was false |
@bncriju Can you please check why the builds are failing? |
Checked the build failures and found that this required changes in kogito runtimes repo. apache/incubator-kie-kogito-runtimes#3860 raised |
Fixes apache/incubator-kie-issues#1752
Added the fix for this problem and added test class and test methods to verify the same.
After and before the changes:
< "compliance-level-3/0092-feel-lambda","0092-feel-lambda-test-01","014","SUCCESS",""