Skip to content

Conversation

@leoromanovsky
Copy link
Contributor

@leoromanovsky leoromanovsky commented Jan 16, 2026

Adds a new flag definition and test case to verify that flag evaluation succeeds when targeting key is an empty string. The flag should still match the allocation and return the expected value.

This tests that tracers properly handle empty targeting keys during evaluation (separate from exposure event handling).

Motivation

SDK Specification

OF.7
Empty string is a valid targeting key
Empty string shall be accepted as a targeting key and evaluation should proceed as normal.

While developing another system test to validate that exposures are being sent for empty targeting keys, found that in Java and NodeJS implementations an empty targeting key was causing evaluations to return the programmatic default.

__ Test_FFE_EXP_5_Missing_Targeting_Key.test_ffe_exp_5_missing_targeting_key ___

self = <tests.ffe.test_exposures.Test_FFE_EXP_5_Missing_Targeting_Key object at 0x7f5b1d4b90d0>

    def test_ffe_exp_5_missing_targeting_key(self):
        """EXP.5: Test that empty targeting key generates exposure with subject.id = ''."""
        assert self.response.status_code == 200, f"Flag evaluation failed: {self.response.text}"
    
        result = json.loads(self.response.text)
>       assert result["value"] == "value-a", f"Expected 'value-a', got '{result['value']}'"
E       AssertionError: Expected 'value-a', got 'default'
E       assert 'default' == 'value-a'
E         - value-a
E         + default

tests/ffe/test_exposures.py:958: AssertionError
------------------------------ Captured log call -------------------------------
03:49:31.761 INFO     weblog POST http://localhost:7777/ffe -> 200
- generated xml file: /home/runner/work/system-tests/system-tests/logs_feature_flagging_and_experimentation/reportJunit.xml -
=========================== short test summary info ============================
FAILED tests/ffe/test_exposures.py::Test_FFE_EXP_5_Missing_Targeting_Key::test_ffe_exp_5_missing_targeting_key
========== 1 failed, 12 passed, 2122 deselected in 124.34s (0:02:04) ===========
Error: Process completed with exit code 1.

Changes

  • tests/parametric/test_ffe/test-case-of-7-empty-targeting-key.json: New test case file for OF.7
  • tests/parametric/test_ffe/flags-v1.json: Added empty-targeting-key-flag fixture
  • tests/parametric/test_ffe/test_dynamic_evaluation.py:
    • Added inline skip in test_ffe_flag_evaluation for Java/Node.js on OF.7 test case
    • Added dedicated test_ffe_of7_empty_targeting_key test with @bug decorators

Decisions

Why both inline skip AND dedicated test?

  • Inline skip: Safeguard to prevent parametrized test failures for known-buggy libraries
  • Dedicated test with @bug decorators: Provides visibility in test reporting, registers skipped tests centrally, canonical pattern for tracking known bugs

This dual approach ensures the bugs are tracked properly while preventing CI failures.

Next Steps

Language: Java
JIRA: FFL-1729
Fix: open-feature/java-sdk#1807
Action: Merge upstream PR, then remove @bug decorator
────────────────────────────────────────
Language: Node.js
JIRA: FFL-1730
Fix: https://github.com/DataDog/openfeature-js-client/pull/new/leo/allow-empty-targeting-key
Action: Merge internal PR, then remove @bug decorator
Once fixes are merged:

  1. Remove @bug decorators from test_ffe_of7_empty_targeting_key
  2. Remove inline skip from test_ffe_flag_evaluation
  3. Remove dedicated test

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed and the CI green, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • Anything but tests/ or manifests/ is modified ? I have the approval from R&P team
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added, removed or renamed?

@github-actions
Copy link
Contributor

github-actions bot commented Jan 16, 2026

CODEOWNERS have been resolved as:

tests/parametric/test_ffe/test-case-of-7-empty-targeting-key.json       @DataDog/feature-flagging-and-experimentation-sdk @DataDog/system-tests-core
tests/parametric/test_ffe/flags-v1.json                                 @DataDog/feature-flagging-and-experimentation-sdk @DataDog/system-tests-core
tests/parametric/test_ffe/test_dynamic_evaluation.py                    @DataDog/feature-flagging-and-experimentation-sdk @DataDog/system-tests-core

Adds a new flag definition and test case to verify that flag evaluation
succeeds when targeting key is an empty string. The flag should still
match the allocation and return the expected value.

This tests that tracers properly handle empty targeting keys during
evaluation (separate from exposure event handling).
These libraries have known bugs where their OpenFeature SDKs reject
empty targeting keys with a truthiness check:
- Java: FFL-1729 (fix pending: open-feature/java-sdk#1807)
- Node.js: FFL-1730 (fix: https://github.com/DataDog/openfeature-js-client/pull/new/leo/allow-empty-targeting-key)
@leoromanovsky leoromanovsky force-pushed the leo/ffe-empty-targeting-key-test branch from 3573388 to f3ce7cf Compare January 17, 2026 04:36
- Added test_ffe_of7_empty_targeting_key with @bug decorators for
  Java (FFL-1729) and Node.js (FFL-1730)
- Keeps inline skip in parametrized test as safeguard
- Provides visibility in test reporting for known bugs
@leoromanovsky leoromanovsky marked this pull request as ready for review January 17, 2026 05:22
@leoromanovsky leoromanovsky requested review from a team as code owners January 17, 2026 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants