Skip to content

Commit 1a5dc60

Browse files
Mat001Mat001
committed
Add comprehensive holdout tests to config_test.go
- Tests based on JavaScript and Swift SDK patterns - TestHoldoutConfig_EmptyHoldouts: empty holdout arrays - TestHoldoutConfig_HoldoutMapping: holdout categorization (global/included/excluded) - TestHoldoutConfig_GetHoldout: individual holdout lookup by ID - TestGetHoldoutsForFlag_Logic: flag-specific holdout logic matching JS SDK - TestGetHoldoutsForFlag_Caching: result caching behavior Tests verify reviewer requirements: - Holdout objects exposed (not IDs) - Proper holdout list logic - Config-level holdout management
1 parent 83aa55f commit 1a5dc60

File tree

3 files changed

+257
-4
lines changed

3 files changed

+257
-4
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ lint: ## runs `golangci-lint` linters defined in `.golangci.yml` file
2929
$(GOLINT) run --out-format=tab --tests=false pkg/...
3030

3131
test: ## recursively test source code in pkg without coverage
32-
GO111MODULE=$(GO111MODULE) $(GOTEST) ./pkg/...
32+
GO111MODULE=$(GO111MODULE) $(GOTEST) -count=1 ./pkg/...
3333

3434
benchmark: ## recursively test source code in pkg without coverage
3535
GO111MODULE=$(GO111MODULE) $(GOTEST) -bench=. -run=^a ./pkg/...

‎pkg/config/datafileprojectconfig/config.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,7 @@ type DatafileProjectConfig struct {
5959
sdkKey string
6060
environmentKey string
6161
region string
62-
6362
flagVariationsMap map[string][]entities.Variation
64-
65-
// Holdout related fields
6663
holdoutIDMap map[string]entities.Holdout
6764
globalHoldouts []entities.Holdout
6865
includedHoldouts map[string][]entities.Holdout

‎pkg/config/datafileprojectconfig/config_test.go‎

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,10 @@ import (
2424
"path/filepath"
2525
"testing"
2626

27+
datafileEntities "github.com/optimizely/go-sdk/v2/pkg/config/datafileprojectconfig/entities"
2728
"github.com/optimizely/go-sdk/v2/pkg/entities"
2829
"github.com/optimizely/go-sdk/v2/pkg/logging"
30+
jsoniter "github.com/json-iterator/go"
2931

3032
"github.com/stretchr/testify/assert"
3133
)
@@ -728,3 +730,257 @@ func TestGetAttributeByKeyWithDirectMapping(t *testing.T) {
728730
assert.Nil(t, err)
729731
assert.Equal(t, attribute, actual)
730732
}
733+
734+
// Test holdout functionality based on JavaScript and Swift SDK patterns
735+
func TestHoldoutConfig_EmptyHoldouts(t *testing.T) {
736+
datafile := &datafileEntities.Datafile{
737+
Version: "4",
738+
AccountID: "12345",
739+
ProjectID: "67890",
740+
Revision: "1",
741+
Holdouts: []datafileEntities.Holdout{},
742+
}
743+
744+
jsonBytes, err := jsoniter.Marshal(datafile)
745+
assert.NoError(t, err)
746+
747+
config, err := NewDatafileProjectConfig(jsonBytes, logging.GetLogger("", "test"))
748+
assert.NoError(t, err)
749+
750+
assert.Empty(t, config.holdoutIDMap)
751+
assert.Empty(t, config.globalHoldouts)
752+
assert.Empty(t, config.includedHoldouts)
753+
assert.Empty(t, config.excludedHoldouts)
754+
assert.Empty(t, config.flagHoldoutsMap)
755+
}
756+
757+
func TestHoldoutConfig_HoldoutMapping(t *testing.T) {
758+
datafile := &datafileEntities.Datafile{
759+
Version: "4",
760+
AccountID: "12345",
761+
ProjectID: "67890",
762+
Revision: "1",
763+
FeatureFlags: []datafileEntities.FeatureFlag{
764+
{ID: "flag_1", Key: "feature_1"},
765+
{ID: "flag_2", Key: "feature_2"},
766+
{ID: "flag_3", Key: "feature_3"},
767+
},
768+
Holdouts: []datafileEntities.Holdout{
769+
{
770+
ExperimentCore: datafileEntities.ExperimentCore{
771+
ID: "global_holdout",
772+
Key: "global_ho",
773+
},
774+
Status: datafileEntities.HoldoutStatusRunning,
775+
IncludedFlags: []string{},
776+
ExcludedFlags: []string{},
777+
},
778+
{
779+
ExperimentCore: datafileEntities.ExperimentCore{
780+
ID: "included_holdout",
781+
Key: "included_ho",
782+
},
783+
Status: datafileEntities.HoldoutStatusRunning,
784+
IncludedFlags: []string{"flag_1", "flag_2"},
785+
ExcludedFlags: []string{},
786+
},
787+
{
788+
ExperimentCore: datafileEntities.ExperimentCore{
789+
ID: "excluded_holdout",
790+
Key: "excluded_ho",
791+
},
792+
Status: datafileEntities.HoldoutStatusRunning,
793+
IncludedFlags: []string{},
794+
ExcludedFlags: []string{"flag_3"},
795+
},
796+
},
797+
}
798+
799+
jsonBytes, err := jsoniter.Marshal(datafile)
800+
assert.NoError(t, err)
801+
802+
config, err := NewDatafileProjectConfig(jsonBytes, logging.GetLogger("", "test"))
803+
assert.NoError(t, err)
804+
805+
// Test holdout ID map
806+
assert.Len(t, config.holdoutIDMap, 3)
807+
assert.Contains(t, config.holdoutIDMap, "global_holdout")
808+
assert.Contains(t, config.holdoutIDMap, "included_holdout")
809+
assert.Contains(t, config.holdoutIDMap, "excluded_holdout")
810+
811+
// Test global holdouts (empty IncludedFlags)
812+
assert.Len(t, config.globalHoldouts, 2)
813+
globalIDs := []string{config.globalHoldouts[0].ID, config.globalHoldouts[1].ID}
814+
assert.Contains(t, globalIDs, "global_holdout")
815+
assert.Contains(t, globalIDs, "excluded_holdout")
816+
817+
// Test included holdouts
818+
assert.Len(t, config.includedHoldouts, 2)
819+
assert.Contains(t, config.includedHoldouts, "flag_1")
820+
assert.Contains(t, config.includedHoldouts, "flag_2")
821+
assert.Equal(t, "included_holdout", config.includedHoldouts["flag_1"][0].ID)
822+
assert.Equal(t, "included_holdout", config.includedHoldouts["flag_2"][0].ID)
823+
}
824+
825+
func TestHoldoutConfig_GetHoldout(t *testing.T) {
826+
datafile := &datafileEntities.Datafile{
827+
Version: "4",
828+
AccountID: "12345",
829+
ProjectID: "67890",
830+
Revision: "1",
831+
Holdouts: []datafileEntities.Holdout{
832+
{
833+
ExperimentCore: datafileEntities.ExperimentCore{
834+
ID: "test_holdout",
835+
Key: "test_key",
836+
},
837+
Status: datafileEntities.HoldoutStatusRunning,
838+
IncludedFlags: []string{"flag_1"},
839+
ExcludedFlags: []string{},
840+
},
841+
},
842+
}
843+
844+
jsonBytes, err := jsoniter.Marshal(datafile)
845+
assert.NoError(t, err)
846+
847+
config, err := NewDatafileProjectConfig(jsonBytes, logging.GetLogger("", "test"))
848+
assert.NoError(t, err)
849+
850+
// Test existing holdout
851+
holdout, err := config.GetHoldout("test_holdout")
852+
assert.NoError(t, err)
853+
assert.Equal(t, "test_holdout", holdout.ID)
854+
assert.Equal(t, "test_key", holdout.Key)
855+
assert.Equal(t, entities.HoldoutStatusRunning, holdout.Status)
856+
assert.Equal(t, []string{"flag_1"}, holdout.IncludedFlags)
857+
assert.Empty(t, holdout.ExcludedFlags)
858+
859+
// Test non-existent holdout
860+
_, err = config.GetHoldout("non_existent")
861+
assert.Error(t, err)
862+
assert.Contains(t, err.Error(), `holdout with ID "non_existent" not found`)
863+
}
864+
865+
func TestGetHoldoutsForFlag_Logic(t *testing.T) {
866+
datafile := &datafileEntities.Datafile{
867+
Version: "4",
868+
AccountID: "12345",
869+
ProjectID: "67890",
870+
Revision: "1",
871+
FeatureFlags: []datafileEntities.FeatureFlag{
872+
{ID: "flag_1", Key: "feature_1"},
873+
{ID: "flag_2", Key: "feature_2"},
874+
{ID: "flag_3", Key: "feature_3"},
875+
},
876+
Holdouts: []datafileEntities.Holdout{
877+
{
878+
ExperimentCore: datafileEntities.ExperimentCore{
879+
ID: "global1",
880+
Key: "global_holdout_1",
881+
},
882+
Status: datafileEntities.HoldoutStatusRunning,
883+
IncludedFlags: []string{},
884+
ExcludedFlags: []string{},
885+
},
886+
{
887+
ExperimentCore: datafileEntities.ExperimentCore{
888+
ID: "global_with_exclusion",
889+
Key: "global_excluded",
890+
},
891+
Status: datafileEntities.HoldoutStatusRunning,
892+
IncludedFlags: []string{},
893+
ExcludedFlags: []string{"flag_3"},
894+
},
895+
{
896+
ExperimentCore: datafileEntities.ExperimentCore{
897+
ID: "specific_holdout",
898+
Key: "flag_specific",
899+
},
900+
Status: datafileEntities.HoldoutStatusRunning,
901+
IncludedFlags: []string{"flag_1"},
902+
ExcludedFlags: []string{},
903+
},
904+
},
905+
}
906+
907+
jsonBytes, err := jsoniter.Marshal(datafile)
908+
assert.NoError(t, err)
909+
910+
config, err := NewDatafileProjectConfig(jsonBytes, logging.GetLogger("", "test"))
911+
assert.NoError(t, err)
912+
913+
// Test feature_1: should get global holdouts + specifically included
914+
holdouts := config.GetHoldoutsForFlag("feature_1")
915+
assert.Len(t, holdouts, 3)
916+
holdoutIDs := make([]string, len(holdouts))
917+
for i, h := range holdouts {
918+
holdoutIDs[i] = h.ID
919+
}
920+
assert.Contains(t, holdoutIDs, "global1")
921+
assert.Contains(t, holdoutIDs, "global_with_exclusion")
922+
assert.Contains(t, holdoutIDs, "specific_holdout")
923+
924+
// Test feature_2: should get only global holdouts
925+
holdouts = config.GetHoldoutsForFlag("feature_2")
926+
assert.Len(t, holdouts, 2)
927+
holdoutIDs = make([]string, len(holdouts))
928+
for i, h := range holdouts {
929+
holdoutIDs[i] = h.ID
930+
}
931+
assert.Contains(t, holdoutIDs, "global1")
932+
assert.Contains(t, holdoutIDs, "global_with_exclusion")
933+
934+
// Test feature_3: should get global holdouts minus excluded
935+
holdouts = config.GetHoldoutsForFlag("feature_3")
936+
assert.Len(t, holdouts, 1)
937+
assert.Equal(t, "global1", holdouts[0].ID)
938+
939+
// Test non-existent flag
940+
holdouts = config.GetHoldoutsForFlag("non_existent")
941+
assert.Empty(t, holdouts)
942+
}
943+
944+
func TestGetHoldoutsForFlag_Caching(t *testing.T) {
945+
datafile := &datafileEntities.Datafile{
946+
Version: "4",
947+
AccountID: "12345",
948+
ProjectID: "67890",
949+
Revision: "1",
950+
FeatureFlags: []datafileEntities.FeatureFlag{
951+
{ID: "flag_1", Key: "feature_1"},
952+
},
953+
Holdouts: []datafileEntities.Holdout{
954+
{
955+
ExperimentCore: datafileEntities.ExperimentCore{
956+
ID: "holdout_1",
957+
Key: "test_holdout",
958+
},
959+
Status: datafileEntities.HoldoutStatusRunning,
960+
IncludedFlags: []string{"flag_1"},
961+
ExcludedFlags: []string{},
962+
},
963+
},
964+
}
965+
966+
jsonBytes, err := jsoniter.Marshal(datafile)
967+
assert.NoError(t, err)
968+
969+
config, err := NewDatafileProjectConfig(jsonBytes, logging.GetLogger("", "test"))
970+
assert.NoError(t, err)
971+
972+
// Initially cache should be empty
973+
assert.Empty(t, config.flagHoldoutsMap)
974+
975+
// First call should populate cache
976+
holdouts1 := config.GetHoldoutsForFlag("feature_1")
977+
assert.Len(t, holdouts1, 1)
978+
979+
// Cache should now have entry
980+
assert.Len(t, config.flagHoldoutsMap, 1)
981+
assert.Contains(t, config.flagHoldoutsMap, "flag_1")
982+
983+
// Second call should return cached result
984+
holdouts2 := config.GetHoldoutsForFlag("feature_1")
985+
assert.Equal(t, holdouts1, holdouts2)
986+
}

0 commit comments

Comments
 (0)