feat: Report suppressed failures #687
Merged
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
3 tasks
saig0
force-pushed
the
260-suppressed-failures
branch
2 times, most recently
from
5959e58 to
2503943
Compare
remcowesterhoud
requested changes
Jul 19, 2023
There was a problem hiding this comment.
Great work @saig0! But much changes == much review comments 😄 Please have a look at them 👀
src/main/scala/org/camunda/feel/impl/interpreter/EvaluationFailureCollector.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala
Outdated
Show resolved
Hide resolved
|
@remcowesterhoud thank you for the detailed review. 🏅 I addressed all your comments. Please have another look. 🍰 |
remcowesterhoud
approved these changes
Jul 20, 2023
Adjust the Clirr check to ignore private generate functions. These functions contain a "$" symbol in their name. Previously, the ignore rule was defined only for one function. Change the rule to include all generate functions.
Extend the evaluation context to collect evaluation failures. The collector is created with the context and is passed if a new context is created. Allows to retrieve all evaluation failures by accessing the collector from the context.
Currently, the engine API returns an Either as the result of an evaluation. The Either contains the result value if the valuation was successful. Otherwise, it contains the failure. To expose the suppressed failures as part of the result, we need a new API. We can't change the existing API because of the backward-compatibility. Instead of duplicating the existing API in the engine class, the new API moved into a separate class and package. This should keep the API and the engine more clean. To expose and promote the new API, we need a new builder that returns the new API. The existing builder returns the old API.
Extend the existing engine API to return an evaluation result. Reorder the public API methods in the class and mark them as deprecated. Adjust the root context to create a new context for each evaluation. This is required to collect the suppressed failures of the evaluation.
Create a new test class to verify that suppressed failures are reported for different cases.
Report suppressed failures in the interpreter (but for not all cases yet). The suppressed failures replace the previous logging statements.
Replace the logging in the built-in function by reporting these failures as suppressed failures.
The failure messages changed slightly. Adjust the expected failure messages in the existing test cases.
These functions are not used anymore.
Report a suppressed failure if a condition (e.g. conjunction, disjunction, if-then-else, etc.) is not a boolean value.
saig0
force-pushed
the
260-suppressed-failures
branch
from
2490737 to
435f3b5
Compare
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
Related issues
closes #260