test(openfeature): use canonical FFE fixtures - #19390
Conversation
Codeowners resolved asResolved from the full PR diff against |
|
Circular import analysis
|
leoromanovsky
added
the
changelog/no-changelog
## Motivation Use the shared FFE fixture corpus. This prevents copied Go fixtures from drifting from other SDKs. The same migration is merged in [Java](DataDog/dd-trace-java#11355) and [libdatadog](DataDog/libdatadog#1979). Related migrations are open for [Python](DataDog/dd-trace-py#19390), [JavaScript](DataDog/dd-trace-js#8441), [Ruby](DataDog/dd-trace-rb#5742), and [.NET](DataDog/dd-trace-dotnet#8616). ## Changes and Decisions - Add `DataDog/ffe-system-test-data` as an OpenFeature test submodule. - Read all canonical configuration and evaluation cases from the submodule. - Parse and validate flags independently so one invalid flag does not reject valid neighbors. - Assert canonical values and reasons, including temporal defaults and missing-flag errors. - Initialize submodules in Go test workflows and configure weekly Dependabot updates. Co-authored-by: leo.romanovsky <leo.romanovsky@datadoghq.com>
Dependency direction analysis
|
BenchmarksBenchmark execution time: 2026-08-14 06:04:58 Comparing candidate commit 7a20072 in PR branch Found 0 performance improvements and 9 performance regressions! Performance is the same for 608 metrics, 10 unstable metrics. scenario:httppropagationextract-datadog_tracecontext_tracestate_not_propagated_on_trace_id_no_match
scenario:httppropagationinject-ids_only
scenario:iastaspects-add_aspect
scenario:iastaspects-lower_aspect
scenario:iastaspectsospath-ospathbasename_aspect
scenario:iastaspectssplit-rsplit_aspect
scenario:span-start
scenario:telemetryaddmetric-1-count-metric-1-times
scenario:tracer-small
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19a708bce3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| EvaluationError::FlagConfigurationInvalid => { | ||
| ResolutionDetails::empty(Reason::Default) |
There was a problem hiding this comment.
When an enabled flag contains a condition parse failure other than an invalid regex—for example, GT with a string, ONE_OF with a scalar, IS_NULL with a non-boolean, or an invalid semantic version—the pinned libdatadog evaluator collapses the per-flag failure to FlagConfigurationInvalid. Because the new pre-scan only records regex and variation-value errors, this arm now makes the provider report DEFAULT without an error, whereas the parent implementation reported ERROR/PARSE_ERROR. Preserve the parse status for these cases or classify all equivalent parser failures before changing this mapping.
AGENTS.md reference: AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
| test_id = f"{fixture_file.replace('.json', '')}_{i}_{test_case.get('targetingKey', 'no_key')}" | ||
| all_test_cases.append((fixture_file, test_case, test_id)) | ||
|
|
||
| assert all_test_cases, f"No FFE JSON fixtures found in {FIXTURES_DIR}" |
There was a problem hiding this comment.
In a fresh clone without --recurse-submodules, this directory is absent or empty, so collection reaches this assertion with no cases and the documented scripts/run-tests workflow cannot run the OpenFeature suite. The new tests/suitespec.yml variables cover the generated GitLab OpenFeature job, but neither scripts/run-tests nor another repository setup hook initializes the submodule, leaving local and other non-GitLab test runs broken unless contributors know an undocumented prerequisite. Have the standard runner initialize the pinned submodule or explicitly handle and document the requirement.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
brettlangdon
left a comment
brettlangdon
left a comment
There was a problem hiding this comment.
can we get docs updates and agents.md/something llm focused updated as well?
I don't think this should have a big impact on anyone, since these fixtures should only get loaded when running the openfeature test suite, but I want it to be clear to people how to resolve the issue with the fact that they won't have the fixtures by default (no one is going to actually use git submodule command, especially if it isn't for their own files)
the concern is mostly, if someone changes something that impacts openfeature and they try to validate the test suite locally, then they will fail because the fixtures aren't present, what can we do to avoid that?
brettlangdon
left a comment
brettlangdon
left a comment
There was a problem hiding this comment.
Rejecting only to prevent merging until we figure out the process we want in the other SDKs, but other than small improvements to local devex this lgtm
## Motivation Use the shared FFE fixture corpus so the .NET evaluator is checked against the same behavior as the other tracer implementations. This reduces fixture drift and gives us a repeatable way to expose and correct evaluator bugs when new canonical cases are added. The same fixture corpus is used by [Java](DataDog/dd-trace-java#11355), [libdatadog](DataDog/libdatadog#1979), [Go](DataDog/dd-trace-go#4753), [Python](DataDog/dd-trace-py#19390), [JavaScript](DataDog/dd-trace-js#8441), and [Ruby](DataDog/dd-trace-rb#5742). ## Changes - Replace the legacy copied fixtures with a generated, checked-in snapshot from `DataDog/ffe-system-test-data`. - Record the exact upstream commit in `SOURCE.md`. - Add a script that fetches, validates, and copies the canonical configuration and evaluation cases. - Add a weekly and manually dispatchable workflow that opens a signed draft dependency PR only when fixture contents have changed. - Parse flags independently so malformed flags do not reject valid neighbors. - Return `FLAG_NOT_FOUND` for missing flags and classify temporal, static, and split allocations. - Assert canonical values and reasons through the existing .NET unit-test suite. ## Fixture update flow When we add or change shared evaluator behavior, I imagine the flow working like this: 1. Add the new configuration and evaluation cases to [`DataDog/ffe-system-test-data`](https://github.com/DataDog/ffe-system-test-data) and review the expected behavior there. 2. The weekly updater, or a manually dispatched run for a specific ref, fetches the canonical repository and compares its fixture contents with the checked-in .NET snapshot. 3. If nothing changed, the workflow exits without opening or updating a PR. 4. If fixtures changed, the workflow copies them into this repository, records the source commit, and opens a signed draft PR with the normal dependency labels. 5. The .NET unit tests run against the updated cases. New tests may intentionally fail when they catch an evaluator bug or unsupported behavior. 6. Address those evaluator failures in the same dependency PR, keeping the fixture expectations unchanged unless the shared expectation itself is incorrect. 7. Merge the update once the .NET evaluator satisfies the new canonical cases. This keeps new behavior explicit and reviewable: fixture changes land in the canonical repository first, and each tracer then gets a visible compatibility PR rather than silently changing at build time. ## Decisions - `DataDog/ffe-system-test-data` remains the canonical source of shared evaluator behavior. - Keep the generated snapshot checked in so local and CI unit tests do not require network access or submodule initialization. - Use a scheduled dependency-update workflow instead of a git submodule. - Treat failures introduced by new canonical fixtures as useful regression signals and fix the evaluator as part of accepting the update. - Do not create a PR when the canonical fixture contents are unchanged, even if the upstream repository has unrelated commits.
2 participants
Description
Motivation
Use the shared FFE fixture corpus so Python does not maintain a drifting copy of evaluator behavior. This also corrects two evaluator bugs exposed by the canonical cases: invalid regular expressions and variant values that do not match the declared flag type must resolve with
PARSE_ERROR, while other invalid flag configurations retain the provider's existing default behavior.The initial submodule approach made fresh clones and local test runs depend on recursive submodule setup. This revision follows the checked-in snapshot workflow merged in
dd-trace-dotnet: tests work immediately after cloning, while an automated job keeps the snapshot linked to the canonical repository.Canonical fixture implementations are also merged in
dd-trace-java,dd-trace-go, andlibdatadog.Changes
ffe-system-test-datasubmodule with ordinary checked-in fixture files at the same source commit,f3da9ae.mainand opens a signed draft PR only when allowed fixture contents change.PARSE_ERRORfor invalid regex and declared-variant-type mismatches without invalidating the rest of the UFC.Decisions
dd-trace-py, so this repository controls which upstream files become part of its test tree.SOURCE.mdcontaining the exact upstream commit instead of maintaining Git metadata in the test directory.Testing
scripts/run-tests --venv 1cdebe0 -- -s -- tests/internal/test_update_ffe_fixtures.py— 12 passed on Python 3.13.scripts/run-tests --venv 14fc413 -- -s -- tests/openfeature/test_provider_fixtures.py tests/openfeature/test_provider.py— 673 passed, 2 skipped on Python 3.13.scripts/lint checks— passed.f3da9ae; the second run reportedchanged=false.main(ea8b5cc) and confirmed it exposes the expected follow-up evaluator gaps rather than bundling them into this migration.Risks
The snapshot adds checked-in test data, but it is test-only. The scheduled workflow receives repository write access only through the scoped STS policy and uses it to create a signed draft PR when fixture contents change.
Additional Notes
The fixture update flow is:
DataDog/ffe-system-test-data.main, validates and compares the allowed snapshot contents, and exits without a PR when nothing changed.