Skip to content

fix: Make TestData value_for_all return its value when targeting is off - #490

Open
keelerm84 wants to merge 1 commit into
mainfrom
mk/SDK-3477/value-for-all-v1
Open

keelerm84 wants to merge 1 commit into
mainfrom
mk/SDK-3477/value-for-all-v1

Conversation

@keelerm84

@keelerm84 keelerm84 commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

Summary

This is the FDv1 counterpart of #489, which fixes the same bug in TestDataV2.

TestData::FlagBuilder#value_for_all replaced the variations with the single value and called variation_for_all(0), but left the off variation unchanged. A new test flag is a boolean flag with off variation 1, so the built flag had variations: [value] and offVariation: 1. With targeting off, the index was out of range and the flag failed to evaluate. The method is documented to return the value regardless of whether targeting is on or off.

value_for_all now also sets the off variation to 0. variation_for_all is unchanged. A new spec builds the flag and checks that offVariation is 0 with targeting on and off.

Opened from termag


Note

Overview
Fixes TestData FlagBuilder#value_for_all so the configured value is returned when targeting is off, matching its documented behavior (FDv1 parity with the TestDataV2 fix in #489).

Previously the method collapsed variations to a single entry and set fallthrough to index 0, but left the default boolean offVariation at 1. With targeting off, evaluation pointed at an invalid variation index and could fail. The implementation now also calls off_variation(0) after variation_for_all(0).

A new spec asserts offVariation is 0 for flags built with value_for_all, including when on(false).

Reviewed by Cursor Bugbot for commit 77411c6. Bugbot is set up for automated code reviews on this repo. Configure here.

@keelerm84
keelerm84 marked this pull request as ready for review November 5, 2026 19:14
@keelerm84
keelerm84 requested a review from a team as a code owner November 5, 2026 19:14

This branch has not been deployed

No deployments
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.

1 participant