Skip to content

[FFL-1695] test: add variant-key dependent flag conformance fixtures #45

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.

Sign up for GitHub

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

Merged
typotter merged 4 commits into from
Oct 23, 2026
Merged

[FFL-1695] test: add variant-key dependent flag conformance fixtures #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a992368
Add variant-key dependent flag fixtures
typotter Oct 9, 2026
a7da18a
Avoid claiming existing downstream drift
typotter Oct 21, 2026
1341b71
Add negative dependency membership cases
typotter Oct 21, 2026
b562bd0
Move dependent flags into a feature suite
typotter Oct 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ ffe-system-test-data/
├── ufc-config.json # Shared rules-based configuration, including malformed test inputs
├── evaluation-cases/
│ └── test-*.json # Evaluation test case files
├── evaluation-suites/ # Self-contained, explicitly activated feature suites
│ └── dependent-flags/
│ ├── ufc-config.json # UFC containing only dependent-flag fixtures
│ └── cases/ # Evaluation cases for this feature
├── precomputed-assignments/
│ ├── README.md # Precomputed assignment fixture format
│ └── cases/ # Precomputed assignment fixtures, discovered recursively
Expand Down Expand Up @@ -518,6 +522,17 @@ fixture defining the intended behavior.
| `test-special-characters.json` | Flag keys/values with special characters |
| `test-string-with-special-characters.json` | String values with special characters |

## Feature Evaluation Suites

Feature-specific UFC syntax is isolated from the shared root configuration in
self-contained `evaluation-suites/<suite>/` directories. Consumers explicitly
activate the suites they support and use each suite's adjacent `ufc-config.json`
and `cases/` directory.

| Suite | Description |
|-------|-------------|
| `dependent-flags` | Direct and transitive dependencies, short-circuiting, repeated prerequisites, failure propagation, depth limiting, and cycles |

## Origin

These fixtures originated in the Go SDK (`dd-trace-go`). This repository now
Expand All @@ -536,8 +551,10 @@ for the integration checklist.
Fixture pull requests also run the canonical evaluation suite from Go, Java,
JavaScript, Python, Ruby, .NET, PHP, and `libdatadog` against both the
pull-request merge base and proposed head. Each job classifies the result as
compatible, a new regression, an improvement, or existing downstream drift and
uploads the two logs.
compatible, a new regression, an improvement, or inconclusive and uploads the
two logs. A result is inconclusive when both revisions fail: whole-suite exit
codes alone cannot prove that every failure in the proposed fixture set already
existed at the merge base.

These compatibility jobs are intentionally advisory and are allowed to fail.
They must not be configured as required merge checks. Their purpose is to show
Expand Down
10 changes: 5 additions & 5 deletions ci/run-downstream-conformance.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ elif [[ $base_status -ne 0 && $head_status -eq 0 ]]; then
classification=improvement
summary="The base fixture fails and the proposed fixture passes."
else
classification=existing-drift
summary="Both fixture revisions fail; the downstream repository was already incompatible with the pull-request base."
classification=inconclusive
summary="Both fixture revisions fail; whole-suite exit codes cannot determine whether the proposed fixtures add an incompatibility."
fi

printf '\nclassification=%s\n' "$classification"
Expand All @@ -119,9 +119,9 @@ if [[ -n ${GITHUB_STEP_SUMMARY:-} ]]; then
} >>"$GITHUB_STEP_SUMMARY"
fi

# Fail only when the pull request introduces a downstream regression. Existing
# drift remains visible in the job summary and artifacts without blocking an
# unrelated fixture change.
# Fail only when the base passes and the pull request introduces a downstream
# regression. Inconclusive results remain visible in the job summary and
# artifacts without claiming that every head failure existed at the base.
if [[ $classification == new-regression ]]; then
exit 1
fi
Expand Down
112 changes: 112 additions & 0 deletions evaluation-suites/dependent-flags/cases/dependent-flags.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
[
{
"description": "A root flag matches a direct prerequisite by variant key.",
"attributes": {},
"defaultValue": "caller-default",
"flag": "dependent-direct-root",

@aarsilv aarsilv Nov 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 Comment from Claude working with Aaron Silverman:

[P2] Follow-up for this suite, as it stands on main (b57eac6): some cases have no telemetry assertions, and some behaviors have no case.

  • Six of the 14 cases have no expectations, so the harness performs no telemetry assertions for them: dependent-direct, dependent-transitive, dependent-memoized, dependent-atomic-error, dependent-over-depth and dependent-cycle. For example, an SDK that sends the leaf exposure before the missing prerequisite aborts dependent-atomic-error still passes.
  • No case covers a configuration that contains a disabled prerequisite. The design forbids adding one; a case would pin what SDKs do if one arrives anyway.
  • No case covers a prerequisite with doLog: false, or a root with doLog: false whose prerequisite logs. Exposure logging applies per flag.
  • No case covers a rule that matches but whose allocation selects no split, so the root falls through.
  • No case covers non-STRING prerequisites (BOOLEAN, JSON). These check that the comparison uses the variant key, not the value.
  • Only dependent-memoized exercises a successful NOT_ONE_OF comparison, and it has no telemetry expectations.
  • No case covers the default depth (no evaluatorParams) or an invalid maxDependencyDepth. These need their own suite directories, because evaluatorParams applies to the whole configuration.

Suggested change: add expectations to the six cases, and add cases for the missing behaviors in a follow-up PR. The cycle and depth expectations need the evaluators to agree first on which intermediate prerequisites get an evaluation event.

"result": {
"reason": "TARGETING_MATCH",
"value": "direct-match"
},
"targetingKey": "dependent-direct",
"variationType": "STRING"
},
{
"description": "ONE_OF does not match when the prerequisite variant key is absent from the allowed list.",
"attributes": {},
"defaultValue": "caller-default",
"flag": "dependent-one-of-miss-root",

@aarsilv aarsilv Nov 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 Comment from Claude working with Aaron Silverman:

[P1] At current main (b57eac6), this case expects no prerequisite exposure, which conflicts with the worked example in the dependent-flags design. We would like to follow the example.

The design's example uses a rule that requires both a user attribute (country) and a prerequisite flag's value. For a user who matches the attribute but whose prerequisite evaluates to the other variant, the root serves its fallback, and the example lists both the root and the prerequisite for exposure handling (subject to the usual logging rules and deduplication): the prerequisite decided the root's result. The design's numbered rule about rejected rules can be read more broadly, so the design is ambiguous here; the example and its stated reasoning are the clearer guide.

#58 added telemetry expectations to this case and to dependent-not-one-of-hit. Both now expect only the root exposure (noUnmatchedEvents: true), although the prerequisite's own comparison rejects the rule.

Suggested change:

  • When a flagEvaluation condition is the condition that rejects the rule, that prerequisite (and the prerequisites on its own selected path) gets an exposure, subject to its doLog, when the root succeeds.
  • Keep "no exposure" when a different condition rejects the rule (dependent-inapplicable-rule, dependent-transitive-inapplicable-rule), and when the root fails.
  • Add {"flag": "dependent-leaf-on"} to the exposures of dependent-one-of-miss and dependent-not-one-of-hit on main.

All evaluators and DataDog/system-tests#8051 follow the current expectation, so this also needs evaluator updates (libdatadog, openfeature-js-client, Go, Java, .NET) and the matching isPresentInAssignment meaning in precomputed responses.

"result": {
"reason": "STATIC",
"value": "one-of-fallback"
},
"targetingKey": "dependent-one-of-miss",
"variationType": "STRING"
},
{
"description": "NOT_ONE_OF does not match when the prerequisite variant key is present in the excluded list.",
"attributes": {},
"defaultValue": "caller-default",
"flag": "dependent-not-one-of-hit-root",
"result": {
"reason": "STATIC",
"value": "not-one-of-fallback"
},
"targetingKey": "dependent-not-one-of-hit",
"variationType": "STRING"
},
{
"description": "A depth-two tree evaluates its leaf and intermediate prerequisite before matching the root.",
"attributes": {},
"defaultValue": "caller-default",
"flag": "dependent-transitive-root",
"result": {
"reason": "TARGETING_MATCH",
"value": "transitive-match"
},
"targetingKey": "dependent-transitive",
"variationType": "STRING"
},
{
"description": "A false attribute condition short-circuits before an otherwise fatal missing prerequisite and selects the root fallback.",
"attributes": {
"gate": "closed"
},
"defaultValue": "caller-default",
"flag": "dependent-short-circuit-root",
"result": {
"reason": "STATIC",
"value": "short-circuit-fallback"
},
"targetingKey": "dependent-short-circuit",
"variationType": "STRING"
},
{
"description": "Two membership conditions against the same prerequisite evaluate successfully.",
"attributes": {},
"defaultValue": "caller-default",
"flag": "dependent-memoized-root",
"result": {
"reason": "TARGETING_MATCH",
"value": "memoized-match"
},
"targetingKey": "dependent-memoized",
"variationType": "STRING"
},
{
"description": "A successful prerequisite followed by a missing prerequisite aborts the graph and returns the caller default.",
"attributes": {},
"defaultValue": "atomic-default",
"flag": "dependent-atomic-error-root",

@aarsilv aarsilv Nov 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 Comment from Claude working with Aaron Silverman:

[P2] Please pin the root error code in this case and in the depth and cycle cases. When a prerequisite fails, the root should report the general evaluation error, and the prerequisite's own evaluation event should keep the specific code.

Today the implementations in this stack disagree, and the shared evaluation suite does not decide:

We propose GENERAL on the root. FLAG_NOT_FOUND on a root that exists is misleading: OpenFeature reports a flag-not-found error, and metrics tag error.type=flag_not_found for a real flag. GENERAL on the root also matches the no-assignment case that #60 added.

Suggested change on main: add "errorCode": "GENERAL" to the result of dependent-atomic-error-root, dependent-depth-three-root and dependent-cycle-a. Add expectations.evaluationEvents to the atomic-error case, so the missing prerequisite's own event carries FLAG_NOT_FOUND.

"result": {
"reason": "ERROR",
"value": "atomic-default"
},
"targetingKey": "dependent-atomic-error",
"variationType": "STRING"
},
{
"description": "With maxDependencyDepth 2, a third dependency edge aborts the graph and returns the caller default.",
"attributes": {},
"defaultValue": "depth-default",
"flag": "dependent-depth-three-root",
"result": {
"reason": "ERROR",
"value": "depth-default"
},
"targetingKey": "dependent-over-depth",
"variationType": "STRING"
},
{
"description": "A dependency cycle aborts the graph and returns the caller default.",
"attributes": {},
"defaultValue": "cycle-default",
"flag": "dependent-cycle-a",
"result": {
"reason": "ERROR",
"value": "cycle-default"
},
"targetingKey": "dependent-cycle",
"variationType": "STRING"
}
]
Loading