[FSSDK-12394] Add local holdouts to swift-sdk (ref sdk)#628
Open
[FSSDK-12394] Add local holdouts to swift-sdk (ref sdk)#628
Conversation
This commit implements Local Holdouts functionality that allows holdouts to target specific rules instead of all rules within a flag. Changes: - Holdout.swift: Replace includedFlags/excludedFlags with includedRules - HoldoutConfig.swift: Replace flag-level maps with ruleHoldoutsMap - ProjectConfig.swift: Add getGlobalHoldouts() and getHoldoutsForRule() - DefaultDecisionService.swift: Update decision logic for global/local holdouts * getDecisionForFlag() now uses only global holdouts * Added local holdout checks to getVariationFromExperimentRule() * Added local holdout checks to getVariationFromDeliveryRule() Datafile changes: - Global holdouts: includedRules == nil (applies to all rules) - Local holdouts: includedRules == [ruleId, ...] (specific rules only) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Updates existing holdout tests to use includedRules instead of includedFlags/excludedFlags: - HoldoutTests.swift: Update sample data and decode tests * Replace sampleDataWithIncludedFlags with sampleDataWithIncludedRules * Replace sampleDataWithExcludedFlags with sampleDataWithDifferentRules * Add tests for isGlobal property - HoldoutConfigTests.swift: Complete rewrite for new model * Test getGlobalHoldouts() returns only global holdouts * Test getHoldoutsForRule() returns local holdouts for specific rules * Test multiple holdouts can target the same rule * Test rule-to-holdout mapping is built correctly * Remove tests for removed flag-level targeting functionality All tests verify the new Local Holdouts behavior: - Global holdouts: includedRules == nil - Local holdouts: includedRules == [ruleId, ...] Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Added comprehensive integration tests covering: - Global holdout evaluation before all rules - Local holdout evaluation at experiment and delivery rule level - Multiple holdouts targeting same rule - Cross-flag holdout targeting - Global and local holdout interaction - Edge cases (inactive status, non-existent rules, empty includedRules) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…edFlags/excludedFlags Migrated all test files from flag-level to rule-level holdout targeting: - Replaced includedFlags/excludedFlags with includedRules - Updated sample data to use rule IDs instead of flag IDs - Replaced getHoldoutForFlag() calls with getHoldoutsForRule() - Updated ProjectConfigTests to test new rule-level mapping logic Migration strategy: - includedFlags: [] + excludedFlags: [] → omit includedRules (nil = global) - includedFlags: [flagId] → includedRules: [all rule IDs in that flag] - excludedFlags: [flagId] → includedRules: [] (empty = local with no rules) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Mat001
changed the title
…ing holdouts When tests modify project.holdouts, they must also update holdoutConfig.allHoldouts to trigger the internal map rebuilding (ruleHoldoutsMap). Without this, local holdouts are not properly indexed by rule ID and won't be evaluated during decision-making. Added `config.holdoutConfig.allHoldouts = [...]` after each `config.project.holdouts = [...]` assignment in all test files. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Fix testDecideAll_with_holdout_excluded_flags: Changed includedRules from empty array to all feature_1 rule IDs, and updated feature_3 expectations to nil since feature_3 has no rules for local holdouts to target - Fix testDecideAll_with_multiple_holdouts: Removed excludedHoldout which had includedRules=[] (targets no rules), updated feature_3 expectations to nil since local holdouts cannot apply to features with no rules - Fix DecisionListenerTest_Holdouts setUp(): Added missing holdoutConfig.allHoldouts assignment to trigger map rebuild Feature_3 has no experiments or rollout rules, so local holdouts (rule-level targeting) cannot apply to it. Only global holdouts can apply to flags with no rules. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: When local holdouts returned variations, they were being converted to FeatureDecisions with the experiment (not holdout) and source "feature-test" (not "holdout"), causing tests to fail. Root cause: VariationDecision struct didn't carry information about whether the variation came from a holdout. Solution: - Added 'holdout' field to VariationDecision struct - Created DeliveryRuleDecision struct for delivery rules with holdout info - Updated getVariationFromExperimentRule() to set holdout when returning holdout variation - Updated getVariationFromDeliveryRule() to use DeliveryRuleDecision and set holdout field - Updated getVariationForFeatureExperiments() to check for holdout and create FeatureDecision with holdout + source "holdout" instead of experiment + source "feature-test" - Updated getVariationForFeatureRollout() to handle DeliveryRuleDecision and check for holdout This ensures holdout decisions are properly tracked through the decision flow and returned with correct experiment ID and source. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: testGetVariationForFeatureExperiment_NoExperiments and testGetVariationForFeatureExperiment_InvalidExperimentIds were failing because users weren't bucketing into global holdouts. Root cause: Tests used mockBucketValue: 500 (from setUp) but sampleHoldoutGlobal has endOfRange: 500, meaning the valid range is 0-499 (exclusive of 500). Bucket value 500 is outside this range. Solution: Create new MockDecisionService instances in these tests with mockBucketValue: 400, which is within the global holdout range. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
coverage: 93.828% (+0.06%) from 93.766% — mpirnovar-swift-local-holdouts-fssdk-12394 into master |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
Issues
https://optimizely-ext.atlassian.net/browse/FSSDK-12368