Skip to content
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

[Backport 1.16] fix: Unary-test behavior with special input variable ? #906

Merged
merged 11 commits into from
Aug 27, 2024

Conversation

saig0
Copy link
Member

@saig0 saig0 commented Aug 26, 2024

Description

Backport of #887 for 1.16.

Most changes are the same as in the original. However, I had to adjust the code to align with the error behavior in version 1.16.. Instead of returning null (e.g. if a variable doesn't exist), the evaluation fails or returns false. The last commit highlights the different behavior for unary-tests compared to version 1.17+.

Related issues

closes #864

saig0 added 11 commits August 23, 2024 14:49
Add test cases to explicitly verify that a unary-test expression returns the correct output (true/false/null).

Restructure existing tests cases and group them into the different behaviors.

(cherry picked from commit 866ce00)
Introduce a new type for not recoverable errors. Compared to the existing error, this new error should stop the evaluation instead of continue the evaluation with null.

(cherry picked from commit 04abac9)
Correct the behavior of unary-test expression to return only true if an expression with the special variable `?` evaluates to true.

Identify the usage by returning a fatal error if `?` is used outside of an unary-test.

(cherry picked from commit 61cb169)
Adjust all operations to return a fatal error instead of returning null. This is required to limit the access to the special input symbol `?` outside an unary-tests.

Restructure some helper methods to restrict the handling of fatal errors to a few places. This should help to reduce possible problems with fatal errors.

Move some methods to group different aspects together.

(cherry picked from commit c967208)
(cherry picked from commit 1ba21ca)
(cherry picked from commit f171ebf)
Add a test case to verify that the input variable is not available outside of an unary-test expression.

(cherry picked from commit 8354d1b)
Add a string representation for Val types to produce better failure messages.
The failure messages changed because of the new string representation of Val types.
The unary-test behaves differently compared to newer versions. Instead of returning null, it returns false or fail the evaluation. The reason is the improved null-friendly error handling since version 1.17.

Adjust the expected result based on the current behavior.
@saig0 saig0 requested a review from remcowesterhoud August 26, 2024 11:55
Copy link
Contributor

@remcowesterhoud remcowesterhoud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

@remcowesterhoud remcowesterhoud merged commit 3d75a46 into 1.16 Aug 27, 2024
2 checks passed
@remcowesterhoud remcowesterhoud deleted the 864-backport-1.16 branch August 27, 2024 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants