Repository navigation
test: honor FFE evaluation case platform skip/include filters - #8159
leoromanovsky merged 3 commits into
Conversation
Apply canonical _skip and _include annotations before Remote Config setup and refresh the checked-in corpus snapshot from ffe-system-test-data#56. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 <noreply@localhost>
|
|
typotter
changed the title
Use the existing system-tests library identity directly for FFE corpus case selection instead of maintaining a redundant identity mapping. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 <noreply@localhost>
There was a problem hiding this comment.
🔵 Needs a closer look
The expanded shared configuration and corpus have not been exercised against SDK containers, leaving cross-SDK compatibility for human validation.
No open issues 🎉
What changed in this PR
This PR adds platform-aware case selection to the FFE dynamic evaluation runner and refreshes its canonical test corpus.
Changes:
- Uses
context.library.nameto apply per-case_skipand_includefilters before Remote Config setup, provider startup, or evaluation. - Skips case files when no cases apply to the current platform.
- Updates the corpus to commit
5534027e, including split-feature and experimentation opt-out fixtures.
| File | Description |
|---|---|
tests/parametric/test_ffe/test_dynamic_evaluation.py |
Filters cases by SDK platform before setup and evaluation. |
tests/parametric/test_ffe/ffe-system-test-data/ufc-config.json |
Adds activation timestamps and split-feature configurations. |
tests/parametric/test_ffe/ffe-system-test-data/SOURCE.md |
Records the updated canonical source commit. |
tests/parametric/test_ffe/ffe-system-test-data/evaluation-cases/test-case-split-features.json |
Adds split-feature destination and selection cases. |
tests/parametric/test_ffe/ffe-system-test-data/evaluation-cases/test-case-experimentation-opt-out.json |
Adds platform exclusions and future-experiment opt-out expectations. |
tests/parametric/test_ffe/ffe-system-test-data/evaluation-cases/test-case-experimentation-opt-out-future.json |
Adds opt-out cutoff and activation-time cases. |
tests/parametric/test_ffe/ffe-system-test-data/evaluation-cases/test-case-experimentation-opt-out-do-log.json |
Adds opt-out cases for non-logging allocations. |
tests/parametric/test_ffe/ffe-system-test-data/evaluation-cases/test-case-empty-flag.json |
Excludes PHP from empty-flag cases. |
tests/parametric/test_ffe/ffe-system-test-data/evaluation-cases/test-case-disabled-flag.json |
Excludes PHP from disabled-flag cases. |
Review effort: Balanced
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: ebc0b78187
ℹ️ 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".
| platform = context.library.name | ||
| test_cases = _filter_test_cases_for_platform(test_cases, platform) |
There was a problem hiding this comment.
When the corpus contains _skip or _include, this runtime branch silently omits cases based only on the library name—for example, the new PHP disabled/empty-flag cases and Python invalid-attribute cases—while the single pytest node can still report success. This conflicts with .cursor/rules/pr-review.mdc, which requires language-only exclusions to be represented as separately addressable tests with manifest entries so downstream activation/easy-win tooling does not infer that skipped behavior passes; parameterize cases into stable node IDs and record the exclusions in the corresponding manifests instead.
AGENTS.md reference: AGENTS.md:L34-L36
Useful? React with 👍 / 👎.
leoromanovsky
left a comment
leoromanovsky
left a comment
There was a problem hiding this comment.
My understanding is we will remove these skips as support is added.
Summary
_skipand_includeannotations before Remote Config setup, provider startup, or evaluationStack and dependency
leo/ffe-canonical-20261021)Validation
ruff check tests/parametric/test_ffe/test_dynamic_evaluation.pyruff format --check tests/parametric/test_ffe/test_dynamic_evaluation.pymypy tests/parametric/test_ffe/test_dynamic_evaluation.pyTEST_THE_TEST: 15 passed_skip, and_includeselection semanticscaa6d1ac3e5fbda0ce57ef3ffff3772901f3f466Caveats
e5a64a8tocaa6d1ac; this includes intervening canonical corpus additions as well as the applicability annotations from AppSec addresses don't have semi-colons #56.