feat: The invocation of a non-existing function returns null #692
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If a function is invoked with wrong parameter, the invocation should return null (instead of failing the evaluation).
A function invocation returns null if the function is invoked with wrong parameters, or no function exists with this name.
Change the name of some test cases to bring the focus on the invocation of functions.
Add new test cases for a function invocation that return null.
If a function invocation returns null, we want to verify that a failure is reported. A user of the API can access the evaluation failures to understand why an evaluation returned null. So, we should verify the reported failures. To verify the failures, we need a new base test class that returns the whole evaluation result, instead of only the return value. Create a new test matcher to increase the readability of the tests and better failure messages.
Migrate the test class from FeelIntegrationTest to FeelEngineTest. The new base test class allows using the EvaluationResultMatchers that can check for reported failures and produces better failure messages.
Replace the internal test methods by using FeelEngineTest and EvaluationResultMatchers.
Align the failure message if no function was found.
Move test case from SuppressedFailuresTest to InterpreterFunctionTest that bundles all function related test cases. Remove test cases from SuppressedFailuresTest that are already covered in InterpreterFunctionTest.
nicpuppa
approved these changes
Jul 25, 2023
| import org.camunda.feel.api.{EvaluationFailure, EvaluationFailureType, EvaluationResult, FailedEvaluationResult, SuccessfulEvaluationResult} | ||
| import org.scalatest.matchers.{MatchResult, Matcher} | ||
|
|
||
| trait EvaluationResultMatchers { |
There was a problem hiding this comment.
I like it so much, made test cases way more readable 💪
|
@nicpuppa thank you for your fast review. 🍰
Yes. Let's migrate the other tests step-by-step when we're touching them. 🚀
No. But I will add more details about the handling of |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
null.Related issues
closes #670