test(e2e): add Behave step definitions for context merging#593
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces BDD test steps and environment setup for testing context merging logic in the OpenFeature Python SDK. It includes a custom provider to capture merged contexts and steps to populate context at various precedence levels (API, Transaction, Client, Invocation, and Hooks). Feedback focuses on correctly handling the "targeting_key" as a first-class property rather than a generic attribute during both context population and assertion, suggesting the use of the SDK's built-in merge method for consistency.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #593 +/- ##
=======================================
Coverage 98.34% 98.34%
=======================================
Files 45 45
Lines 2483 2483
=======================================
Hits 2442 2442
Misses 41 41
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
gruebel
left a comment
There was a problem hiding this comment.
thanks for the work 🍻 overall looks good, but I added a few comments
|
Hey @balgaly ... will you be picking this back up? Altnernatively, @karnyadavdev may want to take it over since they opened something similar as a duplicate. |
|
@toddbaert, sounds good. I can take this over and push the remaining fixes |
Add the missing E2E step definitions so the contextMerging.feature scenarios from the OpenFeature spec run against python-sdk. Fixes open-feature#500 Changes: - Bump spec submodule to 130df3eb so contextMerging.feature is copied in during the `poe e2e` task. - Add tests/features/environment.py with a before_scenario hook that resets provider/hook/API-context/transaction-context state, so scenarios cannot leak state between features. - Add tests/features/steps/context_merging_steps.py: - RetrievableContextProvider captures the merged EvaluationContext it receives, so assertions can inspect what the SDK merged. - Step definitions for all scenarios in contextMerging.feature: single-level insert, multi-level insert, and per-key overwrite precedence across API / Transaction / Client / Invocation / Before Hooks. - Client-level context is set via direct attribute assignment on OpenFeatureClient.context (no new setter), since merging already honors client.context (openfeature/client.py:422-429). Runs clean: 4 features / 50 scenarios / 233 steps. Signed-off-by: gruebel <anton.gruebel@gmail.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
correctly reset api state on shutdown Signed-off-by: gruebel <anton.gruebel@gmail.com>
Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
…pen-feature#601) * chore(deps): update pre-commit hook tox-dev/pyproject-fmt to v2.21.2 * upper bound toml-fmt-common till fixed Signed-off-by: gruebel <anton.gruebel@gmail.com> --------- Signed-off-by: gruebel <anton.gruebel@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: gruebel <anton.gruebel@gmail.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
…#604) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
…en-feature#605) * chore(deps): update googleapis/release-please-action action to v5 * fix config Signed-off-by: gruebel <anton.gruebel@gmail.com> --------- Signed-off-by: gruebel <anton.gruebel@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: gruebel <anton.gruebel@gmail.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
…en-feature#606) * chore(deps): update pre-commit hook pre-commit/mirrors-mypy to v2 * update config Signed-off-by: gruebel <anton.gruebel@gmail.com> --------- Signed-off-by: gruebel <anton.gruebel@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: gruebel <anton.gruebel@gmail.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
* chore(deps): update dependency prek to >=0.4.3,<0.5.0 * adjust CI Signed-off-by: gruebel <anton.gruebel@gmail.com> --------- Signed-off-by: gruebel <anton.gruebel@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: gruebel <anton.gruebel@gmail.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
…n-feature#595) * feat!: make set_provider non-blocking, add set_provider_and_wait Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * fix: ruff format signature collapse in api.py Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * fix: use threading.Event in error event test to avoid flaky busy-wait Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * fixup: pr feedback and additional checks Signed-off-by: Todd Baert <todd.baert@dynatrace.com> * fix: check active registration in stale-init guard, not _provider_status Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * fixup: edge shutdown race Signed-off-by: Todd Baert <todd.baert@dynatrace.com> --------- Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
* Isolate provider event handlers Signed-off-by: Lucas-FManager <265058144+Lucas-FManager@users.noreply.github.com> * Address event handler review feedback Signed-off-by: Lucas-FManager <265058144+Lucas-FManager@users.noreply.github.com> * test: cover event dispatch noop path Signed-off-by: Lucas-FManager <265058144+Lucas-FManager@users.noreply.github.com> * fixup: drain executor at exit and relax non-blocking test timing margin Signed-off-by: Todd Baert <todd.baert@dynatrace.com> --------- Signed-off-by: Lucas-FManager <265058144+Lucas-FManager@users.noreply.github.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: Lucas-FManager <265058144+Lucas-FManager@users.noreply.github.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
fix flaky event handler test Signed-off-by: gruebel <anton.gruebel@gmail.com>
…5.15 (open-feature#608) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
* chore(main): release 0.10.0 Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com> * docs: clarify non-blocking set_provider behavior in changelog Signed-off-by: Todd Baert <todd.baert@dynatrace.com> --------- Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
Signed-off-by: gruebel <anton.gruebel@gmail.com>
924048f to
987eb2f
Compare
…-context-merging-e2e # Conflicts: # uv.lock
Fixes #500
Per @gruebel's note on #565 — the actual fix for #500 is the missing E2E tests, not a new setter, since Python idiomatically uses direct attribute access. The merging precedence in the SDK is already correct (openfeature/client.py:422-429); this PR proves it with Gherkin scenarios from the spec.
Summary
poe e2etask.Client-level context is set via direct attribute assignment on OpenFeatureClient.context rather than a new setter. This matches the Pythonic pattern called out in #565 review and requires no API surface change.
Test plan
Supersedes
Closes #590 in favor of this approach.