feat: add PARSE_ERROR and whitespace tests for evaluator ref#361
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds test cases for evaluator references ($ref) in flag definitions, covering whitespace variations and error handling for nonexistent references. The changes include new flag configurations in evaluator-refs.json and corresponding Gherkin scenarios in targeting.feature. Feedback was provided to improve the nonexistent evaluator test by using a unique evaluation default value, which ensures the test accurately verifies that the system falls back to the evaluation default rather than the flag's default variant during a parse error.
| Given a String-flag with key "ref-to-nonexistent-evaluator-flag" and a default value "fallback" | ||
| When the flag was evaluated with details | ||
| Then the resolved details value should be "fallback" |
There was a problem hiding this comment.
To ensure the test correctly verifies that the evaluator falls back to the evaluation default value (as required by the OpenFeature specification for a PARSE_ERROR) rather than the flag's defaultVariant, it is better to use a unique value for the default in the evaluation call. Currently, both the flag's defaultVariant (in evaluator-refs.json) and the evaluation default are set to "fallback", making it impossible to distinguish which one is being returned.
Given a String-flag with key "ref-to-nonexistent-evaluator-flag" and a default value "evaluation-default"
When the flag was evaluated with details
Then the resolved details value should be "evaluation-default"
There was a problem hiding this comment.
Should we differentiate here? I saw that other testcases don't differentiate either.
aepfli
left a comment
aepfli
left a comment
There was a problem hiding this comment.
should the title be "feat" everything test related in here, is actually a feature or a fix
leakonvalinka
changed the title
Signed-off-by: Lea Konvalinka <lea.konvalinka@dynatrace.com>
Signed-off-by: Lea Konvalinka <lea.konvalinka@dynatrace.com>
Remove 'error message should contain' step (new step def, adds friction). Fix evaluator suite to use 'a fallback value' instead of 'a default value'. Add missing 'Given an evaluator' to new evaluator scenarios. Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert
force-pushed
the
test/evaluator-ref-fixes
branch
from
f18e2b4 to
e97da10
Compare
4 participants
This PR
Related Issues
see #339
Notes
Follow-up Tasks
How to test