test(openfeature): add null targeting key test coverage#5523
test(openfeature): add null targeting key test coverage#5523leoromanovsky wants to merge 6 commits intomasterfrom
Conversation
…ags_spec - Add 3 new test cases: static flag (returns value), sharded flag (returns TARGETING_KEY_MISSING), rule-match flag (returns value) - All 3 scenarios pass empty context hash to simulate nil targeting key - Confirms Ruby FFI correctly delegates null targeting key handling to libdatadog
|
Thank you for updating Change log entry section 👏 Visited at: 2026-03-27 21:29:50 UTC |
|
✨ Fix all issues with BitsAI or with Cursor
|
… for FFE team CODEOWNERS visibility
…lable Configuration.new(json) is defined by the C extension as a singleton method. Without the extension, Ruby's default Class#new calls initialize with 0 args, causing ArgumentError.
BenchmarksBenchmark execution time: 2026-04-13 17:24:38 Comparing candidate commit b4fc266 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 45 metrics, 1 unstable metrics.
|
| RSpec.describe 'Datadog Provider OF.2: Optional Targeting Key' do | ||
| before do | ||
| skip 'Requires native libdatadog extension' unless Datadog::Core::FeatureFlags::Configuration.method(:new).arity == 1 | ||
| end |
There was a problem hiding this comment.
question (non-blocking): These tests call the native C extension directly (Configuration.new → get_assignment), but spec:open_feature doesn't have a .enhance([:compile]) dependency in the Rakefile — unlike spec:core_with_libdatadog_api (Rakefile:260) which does.
Right now the extension is available because the batch seed happens to place a compile-triggering task (like core_with_libdatadog_api) in the same CI job, and it runs first. A different seed could put open_feature in a batch without one, and the skip guard here would silently skip all three tests.
Would it make sense to add something like this to the Rakefile?
Rake::Task['spec:open_feature'].enhance(["compile:libdatadog_api.#{RUBY_VERSION[/\d+.\d+/]}_#{RUBY_PLATFORM}"])|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4fc266283
ℹ️ 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".
|
|
||
| context 'with nil targeting key' do | ||
| it 'evaluates static flag successfully' do | ||
| result = config.get_assignment('static-flag', :string, {}) |
There was a problem hiding this comment.
Cover explicit nil targeting_key in OF.2 tests
These examples are labeled as “nil targeting key” coverage, but they pass contexts without targeting_key ({} / {'email' => ...}), which only exercises the missing key path. This can give false confidence for OF.2 null handling: if the Ruby→FFI context conversion ever diverges between missing and explicit nil, this spec would still pass while the null case regresses. Add at least one call with {'targeting_key' => nil, ...} to assert the advertised behavior.
Useful? React with 👍 / 👎.
What does this PR do?
Adds test coverage for OpenFeature spec OF.2 (optional targeting key) to prove libdatadog handles nil targeting key correctly via the Ruby FFI layer.
Motivation:
The OpenFeature spec (Requirement 3.1.1, OF.2) defines targeting key as optional. The correct behavior (matching libdatadog's reference implementation) is:
""empty string"", returns valueid="", returns valuenull/missingTARGETING_KEY_MISSINGerror, returns defaultidfallback), returns value or falls throughRuby's evaluator delegates to libdatadog which already handles this correctly — the C extension passes
NULLwhentargeting_keyis absent from the context hash. This PR adds explicit test coverage proving the behavior.Change log entry
No customer-visible change. Test-only — adds coverage for optional targeting key behavior that already works correctly.
Additional Notes:
spec/datadog/open_feature/so@DataDog/feature-flagging-and-experimentation-sdkis auto-requested via CODEOWNERSConfiguration#get_assignmentdirectly to exercise the Rust FFI layer without mocksQuestion for reviewers: The existing
evaluation_engine_spec.rbmocksNativeEvaluator, so there's no integration spec testing the full OpenFeature provider → native evaluator → libdatadog path with real flag configs. Should we add one?How to test the change?
bundle exec rspec spec/datadog/open_feature/null_targeting_key_spec.rb3 scenarios:
STATICreasonTARGETING_KEY_MISSINGerrorTARGETING_MATCHreasonNext Steps
test-case-null-targeting-key.jsontoffe-system-test-datashared fixtures (separate fixture centralization project)