feat: add disabled flag evaluation test scenarios#376
Conversation
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
jonathannorris
force-pushed
the
feat/disabled-flag-evaluation
branch
from
776534e to
e13251e
Compare
![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 introduces new flag definitions and Gherkin test scenarios to verify the behavior of disabled flags, ensuring they return a 'DISABLED' reason and the provided default value. The review feedback highlights several necessary improvements to the test suite: adding missing selectors to point to the correct flag source files, correcting a reference to a non-existent flag in the bulk evaluation scenario, and expanding test coverage to include object-type flags. Additionally, it was noted that 'fallback value' should be used instead of 'default' in the Gherkin tables to maintain consistency with project conventions.
There was a problem hiding this comment.
Pull request overview
Adds new flag definitions and Gherkin scenarios to validate the upcoming “disabled flag evaluates successfully with reason=DISABLED and code default” behavior across single-flag, bulk, and selector-based evaluations in the flagd testbed.
Changes:
- Added
gherkin/disabled.featurewith scenarios for disabled single-flag evaluation, bulk evaluation inclusion, and cross-flagset selection. - Added
flags/disabled-flags.jsoncontaining disabled flag definitions (including a cross-flagset key). - Updated
flags/selector-flags.jsonto include an enabledcross-flagset-flagfor selector-based testing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| gherkin/disabled.feature | Introduces disabled-flag evaluation scenarios (single, bulk, selector-based). |
| flags/selector-flags.json | Adds cross-flagset-flag enabled definition for selector scenarios. |
| flags/disabled-flags.json | Adds disabled flag definitions used by the new disabled evaluation scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
|
should we also add this to the evaluator gherkin tests - currently this is the big suite, but i feel like we still want to slowely migrate to the core specific tests, to be not bothered with connections, etc. when testing the core logic |
Mirrors the top-level disabled flag coverage in the evaluator-only suite per Simon's review feedback on #376. Adds disabled-{boolean, string,integer,float,object}-flag fixtures to testkit-flags.json and a new disabled.feature asserting reason=DISABLED with absent value. Relates to: open-feature/flagd#1965 Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert
force-pushed
the
feat/disabled-flag-evaluation
branch
from
11cb483 to
53697b2
Compare
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert
force-pushed
the
feat/disabled-flag-evaluation
branch
from
9eb3647 to
87928ac
Compare
|
I've tested this end-to-end with a locally built flagd against a locally updated Java provider. 👍 . Will merge/release. |
Summary
flags/disabled-flags.json: 5 typed disabled flags (boolean, string, integer, float, object) +cross-flagset-flagflags/selector-flags.json: addscross-flagset-flag(ENABLED) andmetadata.flagSetId: "selector-set"for cross-flag-set coverageevaluator/flags/testkit-flags.json: appends the 5 disabled flag definitionsgherkin/disabled.feature: provider-level scenarios (@in-process @rpc)reason=DISABLEDand the caller-provided defaultevaluator/gherkin/disabled.feature: matching evaluator-level scenarioflagd/Dockerfile: bump base image toflagd:v0.16.0Motivation
Part of open-feature/flagd#1965. ADR: docs/architecture-decisions/disabled-flag-evaluation.md.
Previously a disabled flag returned
reason=ERROR/errorCode=FLAG_DISABLED. As of open-feature/flagd#1968 it returnsreason=DISABLEDwith the caller's default value; this PR adds the testbed coverage to verify that across providers.Related
Part of: open-feature/flagd#1965
Closes: #375