feat(fractional): add nested fractional expression gherkin scenarios#345
feat(fractional): add nested fractional expression gherkin scenarios#345
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the test suite for the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds Gherkin test scenarios for nested JSON Logic expressions within the fractional operator, which is a valuable addition for ensuring evaluator correctness. The new flag definitions in flags/custom-ops.json and the corresponding scenarios in gherkin/targeting.feature are well-structured. I've suggested one improvement to enhance the test coverage for the fractional-nested-var-flag scenario by including cases where the resolved variant is invalid or missing, to ensure robust fallback behavior.
Add two new flag definitions and @fractional-nested tagged scenario outlines to validate nested JSON Logic expressions as bucket variant names in the fractional operator. New flags: - fractional-nested-if-flag: bucket variant name is an if expression that resolves to 'premium' or 'standard' based on context 'tier' - fractional-nested-var-flag: bucket variant name is a var expression that reads 'color' from evaluation context New scenarios (tagged @fractional @fractional-nested): - 'Fractional operator with nested if expression as variant name' - 'Fractional operator with nested var expression as variant name' The @fractional-nested tag allows providers to exclude these scenarios during the transition period before their implementation supports nested expressions: -t 'not @fractional-nested' All rows verified against the Rust flagd-evaluator test suite. Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Per review feedback: add two rows to the nested-var scenario that exercise fallback behavior when the resolved variant name is not present in the flag's variants map. | jon@company.com | yellow | fallback | (variant not in map → error → caller default) | jon@company.com | | fallback | (empty string → not in map → error → caller default) Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
0c493de to
fdcf310
Compare
…rkin suite Adds evaluator counterparts for the four @fractional-nested scenarios that were added to the SDK-level gherkin in #345 and subsequent commits but never mirrored to the evaluator suite: - Nested if expression as variant name (fractional-nested-if-flag) - Nested var expression as variant name (fractional-nested-var-flag) - Nested if expression as weight (fractional-nested-weight-flag) - Fractional operator used as a boolean condition (fractional-as-condition-flag) All scenarios are tagged @fractional-nested so evaluator implementations can opt out during migration with -t 'not @fractional-nested'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Summary
Adds gherkin test coverage for nested JSON Logic expressions as bucket variant names in the
fractionaloperator — tracking open-feature/flagd-evaluator#30 and open-feature/flagd-evaluator#31.New flag definitions
fractional-nested-if-flag— variant name is an{"if": [...]}expression resolved from contextfractional-nested-var-flag— variant name is a{"var": "..."}expression read from contextNew scenarios
Both tagged
@fractional @fractional-nested:Transition support
The dedicated
@fractional-nestedtag lets providers opt out during their migration period:Verification
All rows verified against the Rust flagd-evaluator gherkin test suite (worktree
feat/nested-fractional).