Skip to content

Commit c2aa044

Browse files
Mat001claude
andcommitted
test: Add comprehensive tests for holdout implementation and fix linting
- Added 3 unit tests for GetHoldoutsForFlag() in config_test.go - Created holdout_test.go with 7 comprehensive mapper tests: - Empty holdouts - Global holdouts with exclusions - Specific holdouts with inclusions - Non-running holdouts filtering - Mixed global and specific holdouts - Audience conditions mapping - Variations and traffic allocation mapping - Fixed govet shadow linting error in holdout_service.go line 105 All tests passing. Improves code coverage for new holdout functionality. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 79642d7 commit c2aa044

File tree

3 files changed

+329
-1
lines changed

3 files changed

+329
-1
lines changed

pkg/config/datafileprojectconfig/config_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,3 +728,64 @@ func TestGetAttributeByKeyWithDirectMapping(t *testing.T) {
728728
assert.Nil(t, err)
729729
assert.Equal(t, attribute, actual)
730730
}
731+
732+
func TestGetHoldoutsForFlagWithHoldouts(t *testing.T) {
733+
flagKey := "test_flag"
734+
holdout1 := entities.Holdout{
735+
ID: "holdout_1",
736+
Key: "test_holdout_1",
737+
Status: entities.HoldoutStatusRunning,
738+
}
739+
holdout2 := entities.Holdout{
740+
ID: "holdout_2",
741+
Key: "test_holdout_2",
742+
Status: entities.HoldoutStatusRunning,
743+
}
744+
745+
flagHoldoutsMap := make(map[string][]entities.Holdout)
746+
flagHoldoutsMap[flagKey] = []entities.Holdout{holdout1, holdout2}
747+
748+
config := &DatafileProjectConfig{
749+
flagHoldoutsMap: flagHoldoutsMap,
750+
}
751+
752+
actual := config.GetHoldoutsForFlag(flagKey)
753+
assert.Len(t, actual, 2)
754+
assert.Equal(t, holdout1, actual[0])
755+
assert.Equal(t, holdout2, actual[1])
756+
}
757+
758+
func TestGetHoldoutsForFlagWithNoHoldouts(t *testing.T) {
759+
flagKey := "test_flag"
760+
flagHoldoutsMap := make(map[string][]entities.Holdout)
761+
762+
config := &DatafileProjectConfig{
763+
flagHoldoutsMap: flagHoldoutsMap,
764+
}
765+
766+
actual := config.GetHoldoutsForFlag(flagKey)
767+
assert.Len(t, actual, 0)
768+
assert.Equal(t, []entities.Holdout{}, actual)
769+
}
770+
771+
func TestGetHoldoutsForFlagWithDifferentFlag(t *testing.T) {
772+
flagKey := "test_flag"
773+
otherFlagKey := "other_flag"
774+
holdout := entities.Holdout{
775+
ID: "holdout_1",
776+
Key: "test_holdout_1",
777+
Status: entities.HoldoutStatusRunning,
778+
}
779+
780+
flagHoldoutsMap := make(map[string][]entities.Holdout)
781+
flagHoldoutsMap[otherFlagKey] = []entities.Holdout{holdout}
782+
783+
config := &DatafileProjectConfig{
784+
flagHoldoutsMap: flagHoldoutsMap,
785+
}
786+
787+
// Request different flag - should return empty
788+
actual := config.GetHoldoutsForFlag(flagKey)
789+
assert.Len(t, actual, 0)
790+
assert.Equal(t, []entities.Holdout{}, actual)
791+
}
Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
/****************************************************************************
2+
* Copyright 2025, Optimizely, Inc. and contributors *
3+
* *
4+
* Licensed under the Apache License, Version 2.0 (the "License"); *
5+
* you may not use this file except in compliance with the License. *
6+
* You may obtain a copy of the License at *
7+
* *
8+
* http://www.apache.org/licenses/LICENSE-2.0 *
9+
* *
10+
* Unless required by applicable law or agreed to in writing, software *
11+
* distributed under the License is distributed on an "AS IS" BASIS, *
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. *
13+
* See the License for the specific language governing permissions and *
14+
* limitations under the License. *
15+
***************************************************************************/
16+
17+
package mappers
18+
19+
import (
20+
"testing"
21+
22+
datafileEntities "github.com/optimizely/go-sdk/v2/pkg/config/datafileprojectconfig/entities"
23+
"github.com/optimizely/go-sdk/v2/pkg/entities"
24+
"github.com/stretchr/testify/assert"
25+
)
26+
27+
func TestMapHoldoutsEmpty(t *testing.T) {
28+
rawHoldouts := []datafileEntities.Holdout{}
29+
featureMap := map[string]entities.Feature{}
30+
31+
holdoutList, holdoutIDMap, flagHoldoutsMap := MapHoldouts(rawHoldouts, featureMap)
32+
33+
assert.Empty(t, holdoutList)
34+
assert.Empty(t, holdoutIDMap)
35+
assert.Empty(t, flagHoldoutsMap)
36+
}
37+
38+
func TestMapHoldoutsGlobalHoldout(t *testing.T) {
39+
// Global holdout: no includedFlags, applies to all flags except excluded
40+
rawHoldouts := []datafileEntities.Holdout{
41+
{
42+
ID: "holdout_1",
43+
Key: "global_holdout",
44+
Status: "Running",
45+
ExcludedFlags: []string{"feature_2"},
46+
Variations: []datafileEntities.Variation{
47+
{ID: "var_1", Key: "variation_1"},
48+
},
49+
TrafficAllocation: []datafileEntities.TrafficAllocation{
50+
{EntityID: "var_1", EndOfRange: 10000},
51+
},
52+
},
53+
}
54+
55+
featureMap := map[string]entities.Feature{
56+
"feature_1": {ID: "feature_1", Key: "feature_1"},
57+
"feature_2": {ID: "feature_2", Key: "feature_2"},
58+
"feature_3": {ID: "feature_3", Key: "feature_3"},
59+
}
60+
61+
holdoutList, holdoutIDMap, flagHoldoutsMap := MapHoldouts(rawHoldouts, featureMap)
62+
63+
// Verify holdout list and ID map
64+
assert.Len(t, holdoutList, 1)
65+
assert.Len(t, holdoutIDMap, 1)
66+
assert.Equal(t, "holdout_1", holdoutList[0].ID)
67+
assert.Equal(t, "global_holdout", holdoutList[0].Key)
68+
69+
// Global holdout should apply to feature_1 and feature_3, but NOT feature_2 (excluded)
70+
assert.Contains(t, flagHoldoutsMap, "feature_1")
71+
assert.NotContains(t, flagHoldoutsMap, "feature_2")
72+
assert.Contains(t, flagHoldoutsMap, "feature_3")
73+
74+
assert.Len(t, flagHoldoutsMap["feature_1"], 1)
75+
assert.Len(t, flagHoldoutsMap["feature_3"], 1)
76+
}
77+
78+
func TestMapHoldoutsSpecificHoldout(t *testing.T) {
79+
// Specific holdout: has includedFlags, only applies to those flags
80+
rawHoldouts := []datafileEntities.Holdout{
81+
{
82+
ID: "holdout_1",
83+
Key: "specific_holdout",
84+
Status: "Running",
85+
IncludedFlags: []string{"feature_1", "feature_2"},
86+
Variations: []datafileEntities.Variation{
87+
{ID: "var_1", Key: "variation_1"},
88+
},
89+
TrafficAllocation: []datafileEntities.TrafficAllocation{
90+
{EntityID: "var_1", EndOfRange: 10000},
91+
},
92+
},
93+
}
94+
95+
featureMap := map[string]entities.Feature{
96+
"feature_1": {ID: "feature_1", Key: "feature_1"},
97+
"feature_2": {ID: "feature_2", Key: "feature_2"},
98+
"feature_3": {ID: "feature_3", Key: "feature_3"},
99+
}
100+
101+
holdoutList, holdoutIDMap, flagHoldoutsMap := MapHoldouts(rawHoldouts, featureMap)
102+
103+
// Verify holdout list and ID map
104+
assert.Len(t, holdoutList, 1)
105+
assert.Len(t, holdoutIDMap, 1)
106+
107+
// Specific holdout should only apply to feature_1 and feature_2
108+
assert.Contains(t, flagHoldoutsMap, "feature_1")
109+
assert.Contains(t, flagHoldoutsMap, "feature_2")
110+
assert.NotContains(t, flagHoldoutsMap, "feature_3")
111+
112+
assert.Len(t, flagHoldoutsMap["feature_1"], 1)
113+
assert.Len(t, flagHoldoutsMap["feature_2"], 1)
114+
}
115+
116+
func TestMapHoldoutsNotRunning(t *testing.T) {
117+
// Holdout with non-Running status should be filtered out
118+
rawHoldouts := []datafileEntities.Holdout{
119+
{
120+
ID: "holdout_1",
121+
Key: "paused_holdout",
122+
Status: "Paused",
123+
Variations: []datafileEntities.Variation{
124+
{ID: "var_1", Key: "variation_1"},
125+
},
126+
},
127+
}
128+
129+
featureMap := map[string]entities.Feature{
130+
"feature_1": {ID: "feature_1", Key: "feature_1"},
131+
}
132+
133+
holdoutList, holdoutIDMap, flagHoldoutsMap := MapHoldouts(rawHoldouts, featureMap)
134+
135+
// Non-running holdouts should be filtered out
136+
assert.Empty(t, holdoutList)
137+
assert.Empty(t, holdoutIDMap)
138+
assert.Empty(t, flagHoldoutsMap)
139+
}
140+
141+
func TestMapHoldoutsMixed(t *testing.T) {
142+
// Mix of global and specific holdouts
143+
rawHoldouts := []datafileEntities.Holdout{
144+
{
145+
ID: "holdout_global",
146+
Key: "global_holdout",
147+
Status: "Running",
148+
ExcludedFlags: []string{"feature_2"},
149+
Variations: []datafileEntities.Variation{
150+
{ID: "var_global", Key: "variation_global"},
151+
},
152+
TrafficAllocation: []datafileEntities.TrafficAllocation{
153+
{EntityID: "var_global", EndOfRange: 5000},
154+
},
155+
},
156+
{
157+
ID: "holdout_specific",
158+
Key: "specific_holdout",
159+
Status: "Running",
160+
IncludedFlags: []string{"feature_2"},
161+
Variations: []datafileEntities.Variation{
162+
{ID: "var_specific", Key: "variation_specific"},
163+
},
164+
TrafficAllocation: []datafileEntities.TrafficAllocation{
165+
{EntityID: "var_specific", EndOfRange: 10000},
166+
},
167+
},
168+
}
169+
170+
featureMap := map[string]entities.Feature{
171+
"feature_1": {ID: "feature_1", Key: "feature_1"},
172+
"feature_2": {ID: "feature_2", Key: "feature_2"},
173+
}
174+
175+
holdoutList, holdoutIDMap, flagHoldoutsMap := MapHoldouts(rawHoldouts, featureMap)
176+
177+
// Verify both holdouts are in the list
178+
assert.Len(t, holdoutList, 2)
179+
assert.Len(t, holdoutIDMap, 2)
180+
181+
// feature_1: should get global holdout only (not excluded, not specifically included)
182+
assert.Contains(t, flagHoldoutsMap, "feature_1")
183+
assert.Len(t, flagHoldoutsMap["feature_1"], 1)
184+
assert.Equal(t, "global_holdout", flagHoldoutsMap["feature_1"][0].Key)
185+
186+
// feature_2: should get specific holdout only (excluded from global)
187+
assert.Contains(t, flagHoldoutsMap, "feature_2")
188+
assert.Len(t, flagHoldoutsMap["feature_2"], 1)
189+
assert.Equal(t, "specific_holdout", flagHoldoutsMap["feature_2"][0].Key)
190+
}
191+
192+
func TestMapHoldoutsWithAudienceConditions(t *testing.T) {
193+
rawHoldouts := []datafileEntities.Holdout{
194+
{
195+
ID: "holdout_1",
196+
Key: "holdout_with_audience",
197+
Status: "Running",
198+
AudienceIds: []string{"audience_1", "audience_2"},
199+
AudienceConditions: []interface{}{"or", "audience_1", "audience_2"},
200+
Variations: []datafileEntities.Variation{
201+
{ID: "var_1", Key: "variation_1"},
202+
},
203+
TrafficAllocation: []datafileEntities.TrafficAllocation{
204+
{EntityID: "var_1", EndOfRange: 10000},
205+
},
206+
},
207+
}
208+
209+
featureMap := map[string]entities.Feature{
210+
"feature_1": {ID: "feature_1", Key: "feature_1"},
211+
}
212+
213+
holdoutList, _, _ := MapHoldouts(rawHoldouts, featureMap)
214+
215+
// Verify audience conditions are mapped
216+
assert.Len(t, holdoutList, 1)
217+
assert.Equal(t, []string{"audience_1", "audience_2"}, holdoutList[0].AudienceIds)
218+
assert.NotNil(t, holdoutList[0].AudienceConditionTree)
219+
}
220+
221+
func TestMapHoldoutsVariationsMapping(t *testing.T) {
222+
rawHoldouts := []datafileEntities.Holdout{
223+
{
224+
ID: "holdout_1",
225+
Key: "holdout_variations",
226+
Status: "Running",
227+
Variations: []datafileEntities.Variation{
228+
{
229+
ID: "var_1",
230+
Key: "variation_1",
231+
FeatureEnabled: true,
232+
Variables: []datafileEntities.VariationVariable{
233+
{ID: "var_var_1", Value: "value_1"},
234+
},
235+
},
236+
{
237+
ID: "var_2",
238+
Key: "variation_2",
239+
FeatureEnabled: false,
240+
},
241+
},
242+
TrafficAllocation: []datafileEntities.TrafficAllocation{
243+
{EntityID: "var_1", EndOfRange: 5000},
244+
{EntityID: "var_2", EndOfRange: 10000},
245+
},
246+
},
247+
}
248+
249+
featureMap := map[string]entities.Feature{
250+
"feature_1": {ID: "feature_1", Key: "feature_1"},
251+
}
252+
253+
holdoutList, _, _ := MapHoldouts(rawHoldouts, featureMap)
254+
255+
// Verify variations are mapped correctly
256+
assert.Len(t, holdoutList, 1)
257+
assert.Len(t, holdoutList[0].Variations, 2)
258+
assert.Contains(t, holdoutList[0].Variations, "var_1")
259+
assert.Contains(t, holdoutList[0].Variations, "var_2")
260+
261+
// Verify traffic allocation
262+
assert.Len(t, holdoutList[0].TrafficAllocation, 2)
263+
assert.Equal(t, "var_1", holdoutList[0].TrafficAllocation[0].EntityID)
264+
assert.Equal(t, 5000, holdoutList[0].TrafficAllocation[0].EndOfRange)
265+
assert.Equal(t, "var_2", holdoutList[0].TrafficAllocation[1].EntityID)
266+
assert.Equal(t, 10000, holdoutList[0].TrafficAllocation[1].EndOfRange)
267+
}

pkg/decision/holdout_service.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func (h HoldoutService) GetDecision(decisionContext FeatureDecisionContext, user
102102
variation, _, _ := h.bucketer.Bucket(bucketingID, experimentForBucketing, entities.Group{})
103103

104104
if variation != nil {
105-
reason := reasons.AddInfo("User %s is in variation %s of holdout %s.", userContext.ID, variation.Key, holdout.Key)
105+
reason = reasons.AddInfo("User %s is in variation %s of holdout %s.", userContext.ID, variation.Key, holdout.Key)
106106
h.logger.Info(reason)
107107

108108
featureDecision := FeatureDecision{

0 commit comments

Comments
 (0)