Repository navigation
test(ffe): import canonical feature suites - #8260
Conversation
Extend the unified fixture snapshot to include rules-based feature suites and run dependent flags through the shared evaluator harness without maintaining copied fixtures. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 <noreply@localhost>
|
|
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 3780629 | Docs | View more details | Give us feedback! |
Import the latest dependent-flags corpus, preserve required result assertions, and keep importer unit tests out of this harness change. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 <noreply@localhost>
There was a problem hiding this comment.
🟡 Changes recommended
Requiring evaluation-suites breaks four existing updater tests whose temporary snapshots lack feature suites, so their fixtures and expected counts and output must be updated to reach the intended assertions.
1 open issue
What changed in this PR
Extends the canonical FFE fixture snapshot to include feature suites while keeping dependent-flags coverage separate from the always-on core tests.
Changes:
- Import, validate, and count feature-suite cases alongside the core corpus.
- Refresh the snapshot to
c8cc8dawith expanded dependent-flags fixtures. - Share result assertions and platform filtering while preserving the existing dependent-flags test node.
| File | Description |
|---|---|
utils/scripts/update_ffe_fixtures.py |
Imports and validates feature suites and includes their case counts. |
tests/parametric/test_ffe/test_dynamic_evaluation.py |
Uses canonical dependent-flags fixtures and a shared assertion helper. |
tests/parametric/test_ffe/ffe-system-test-data/SOURCE.md |
Records the refreshed upstream commit. |
tests/parametric/test_ffe/ffe-system-test-data/evaluation-suites/dependent-flags/ufc-config.json |
Adds configurations for additional dependency scenarios. |
tests/parametric/test_ffe/ffe-system-test-data/evaluation-suites/dependent-flags/cases/dependent-flags.json |
Imports expanded canonical dependent-flags cases. |
tests/parametric/test_ffe/dependent-flags/cases/dependent-flags.json |
Removes the separately maintained case copy. |
Review effort: Balanced
Keep fixture imports compatible with rules-based corpora that do not yet define optional feature suites. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 <noreply@localhost>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 48baa6d15a
ℹ️ 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".
leoromanovsky
left a comment
leoromanovsky
left a comment
There was a problem hiding this comment.
🤖 Review submitted by Codex at @leoromanovsky’s explicit request:
Approved. Please address the fixture-update issue in this thread, so an accepted snapshot cannot remove the dependent-flags suite and break test collection. The thread includes the reproduction and a tested guard that preserves generic root-only validation.
Keep core dynamic evaluation tests collectable when the optional dependent-flags corpus is unavailable. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 <noreply@localhost>
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: done
and removed
mergequeue-status: waiting
labels
Motivation
Leo's canonical fixture snapshot import in #8139 established
ffe-system-test-dataas the source of truth, while the dependent-flags harness from #8190 still carries a copied suite undertests/parametric/test_ffe/dependent-flags. Feature-specific rules-based fixtures should use the same canonical import path without being mixed into the always-on core corpus.Changes
evaluation-suites/alongside the canonical root UFC and evaluation cases, while continuing to exclude precomputed assignments and repository documentation.DataDog/ffe-system-test-data@c8cc8da239341748dc8c30c2145c0f0b5432c958.ffe-system-test-data/evaluation-suites/dependent-flagsand apply the corpus_skip/_includefiltering there.This supersedes the fixture/import architecture in #8049. Per-SDK activation and telemetry PRs can now be restacked as small consumers of this harness.
Validation:
python3 utils/scripts/update_ffe_fixtures.py --ref c8cc8da239341748dc8c30c2145c0f0b5432c958— 381 cases loaded; deterministic rerun reportedchanged: false./format.sh— passed, including mypy, import policy, Ruff, YAML, shell, parser, and Node checksWorkflow
system-tests-reviewers: ask to one of youre co-worker familiar with the tested feature.system-tests-core(slack)🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟