Skip to content

Commit 25509a7

Browse files
Merge pull request #81 from harness/FFM-1415
[FFM-1415] added UT to reproduce bug FFM-1415
2 parents e802897 + fec7a46 commit 25509a7

File tree

4 files changed

+277
-79
lines changed

4 files changed

+277
-79
lines changed

evaluation/feature.go

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -259,27 +259,26 @@ func (fc FeatureConfig) GetVariationName(target *Target) string {
259259
if fc.State == FeatureStateOff {
260260
return fc.OffVariation
261261
}
262-
// TODO: variation to target
263-
if target != nil {
264-
if fc.VariationToTargetMap != nil && len(fc.VariationToTargetMap) > 0 {
265-
for _, variationMap := range fc.VariationToTargetMap {
266-
if variationMap.Targets != nil {
267-
for _, t := range variationMap.Targets {
268-
if target.Identifier == t {
269-
return variationMap.Variation
270-
}
262+
263+
if target != nil && len(fc.VariationToTargetMap) > 0 {
264+
for _, variationMap := range fc.VariationToTargetMap {
265+
if variationMap.Targets != nil {
266+
for _, t := range variationMap.Targets {
267+
if target.Identifier == t {
268+
return variationMap.Variation
271269
}
272270
}
271+
}
273272

274-
if variationMap.TargetSegments != nil {
275-
for _, segmentIdentifier := range variationMap.TargetSegments {
276-
segment, ok := fc.Segments[segmentIdentifier]
277-
if !ok {
278-
log.Errorf("The segment [%s] in variation map can not be found for feature %s in project %s", segmentIdentifier, fc.Feature, fc.Project)
279-
} else {
280-
if segment.Evaluate(target) {
281-
return variationMap.Variation
282-
}
273+
if len(variationMap.TargetSegments) > 0 {
274+
for _, segmentIdentifier := range variationMap.TargetSegments {
275+
segment, ok := fc.Segments[segmentIdentifier]
276+
if !ok {
277+
log.Debugf("The segment [%s] in variation map can not be found for feature %s in project %s",
278+
segmentIdentifier, fc.Feature, fc.Project)
279+
} else {
280+
if segment.Evaluate(target) {
281+
return variationMap.Variation
283282
}
284283
}
285284
}

evaluation/feature_test.go

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,3 +736,201 @@ func TestFeatureConfig_GetSegmentIdentifiers(t *testing.T) {
736736
})
737737
}
738738
}
739+
740+
func TestFeatureConfig_GetVariationName(t *testing.T) {
741+
trueVariation := Variation{
742+
Name: stringPtr("True"),
743+
Value: "true",
744+
Identifier: "true",
745+
}
746+
747+
falseVariation := Variation{
748+
Name: stringPtr("False"),
749+
Value: "false",
750+
Identifier: "false",
751+
}
752+
type fields struct {
753+
DefaultServe Serve
754+
Environment string
755+
Feature string
756+
Kind string
757+
OffVariation string
758+
Prerequisites []Prerequisite
759+
Project string
760+
Rules ServingRules
761+
State FeatureState
762+
VariationToTargetMap []VariationMap
763+
Variations Variations
764+
Segments map[string]*Segment
765+
}
766+
type args struct {
767+
target *Target
768+
}
769+
tests := []struct {
770+
name string
771+
fields fields
772+
args args
773+
want string
774+
}{
775+
{
776+
name: "target is null it should evaluate rule and expect true",
777+
fields: fields{
778+
DefaultServe: Serve{
779+
Variation: stringPtr("true"),
780+
},
781+
Environment: "dev",
782+
Feature: "bool-flag",
783+
Kind: "boolean",
784+
OffVariation: "false",
785+
Prerequisites: nil,
786+
Project: "default",
787+
Rules: nil,
788+
State: "on",
789+
VariationToTargetMap: nil,
790+
Variations: Variations{
791+
trueVariation, falseVariation,
792+
},
793+
Segments: nil,
794+
},
795+
args: args{target: nil},
796+
want: "true",
797+
},
798+
{
799+
name: "target is not null it should evaluate variationMap with target and expect false",
800+
fields: fields{
801+
DefaultServe: Serve{
802+
Variation: stringPtr("true"),
803+
},
804+
Environment: "dev",
805+
Feature: "bool-flag",
806+
Kind: "boolean",
807+
OffVariation: "false",
808+
Prerequisites: nil,
809+
Project: "default",
810+
Rules: nil,
811+
State: "on",
812+
VariationToTargetMap: []VariationMap{
813+
{
814+
TargetSegments: nil,
815+
Targets: []string{"harness"},
816+
Variation: "false",
817+
},
818+
},
819+
Variations: Variations{
820+
trueVariation, falseVariation,
821+
},
822+
Segments: nil,
823+
},
824+
args: args{
825+
target: &Target{
826+
Identifier: "harness",
827+
Name: "Harness",
828+
Anonymous: nil,
829+
Attributes: nil,
830+
},
831+
},
832+
want: "false",
833+
},
834+
{
835+
name: "target is not null it should evaluate variationMap with segment and expect false",
836+
fields: fields{
837+
DefaultServe: Serve{
838+
Variation: stringPtr("true"),
839+
},
840+
Environment: "dev",
841+
Feature: "bool-flag",
842+
Kind: "boolean",
843+
OffVariation: "false",
844+
Prerequisites: nil,
845+
Project: "default",
846+
Rules: nil,
847+
State: "on",
848+
VariationToTargetMap: []VariationMap{
849+
{
850+
TargetSegments: []string{"beta"},
851+
Targets: []string{"johndoe"},
852+
Variation: "false",
853+
},
854+
},
855+
Variations: Variations{
856+
trueVariation, falseVariation,
857+
},
858+
Segments: Segments{
859+
"beta": &Segment{
860+
Identifier: "beta",
861+
Included: []string{"harness"},
862+
},
863+
},
864+
},
865+
args: args{
866+
target: &Target{
867+
Identifier: "harness",
868+
Name: "Harness",
869+
Anonymous: nil,
870+
Attributes: nil,
871+
},
872+
},
873+
want: "false",
874+
},
875+
{
876+
name: "target is not null it should evaluate variationMap with segments but segment not found and expect true",
877+
fields: fields{
878+
DefaultServe: Serve{
879+
Variation: stringPtr("true"),
880+
},
881+
Environment: "dev",
882+
Feature: "bool-flag",
883+
Kind: "boolean",
884+
OffVariation: "false",
885+
Prerequisites: nil,
886+
Project: "default",
887+
Rules: nil,
888+
State: "on",
889+
VariationToTargetMap: []VariationMap{
890+
{
891+
TargetSegments: []string{"beta"},
892+
Targets: []string{"johndoe"},
893+
Variation: "false",
894+
},
895+
},
896+
Variations: Variations{
897+
trueVariation, falseVariation,
898+
},
899+
Segments: Segments{
900+
"alpha": &Segment{
901+
Identifier: "alpha",
902+
Included: []string{"harness"},
903+
},
904+
},
905+
},
906+
args: args{
907+
target: &Target{
908+
Identifier: "harness",
909+
Name: "Harness",
910+
Anonymous: nil,
911+
Attributes: nil,
912+
},
913+
},
914+
want: "true",
915+
},
916+
}
917+
for _, tt := range tests {
918+
t.Run(tt.name, func(t *testing.T) {
919+
fc := FeatureConfig{
920+
DefaultServe: tt.fields.DefaultServe,
921+
Environment: tt.fields.Environment,
922+
Feature: tt.fields.Feature,
923+
Kind: tt.fields.Kind,
924+
OffVariation: tt.fields.OffVariation,
925+
Prerequisites: tt.fields.Prerequisites,
926+
Project: tt.fields.Project,
927+
Rules: tt.fields.Rules,
928+
State: tt.fields.State,
929+
VariationToTargetMap: tt.fields.VariationToTargetMap,
930+
Variations: tt.fields.Variations,
931+
Segments: tt.fields.Segments,
932+
}
933+
assert.Equalf(t, tt.want, fc.GetVariationName(tt.args.target), "GetVariationName(%v)", tt.args.target)
934+
})
935+
}
936+
}

tests/evaluator_test.go

Lines changed: 61 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package tests
22

33
import (
44
"encoding/json"
5+
"fmt"
56
"io/ioutil"
67
"path/filepath"
78
"reflect"
@@ -16,12 +17,18 @@ import (
1617

1718
const source = "./ff-test-cases/tests"
1819

20+
type test struct {
21+
Flag string `json:"flag"`
22+
Target *string `json:"target"`
23+
Expected interface{} `json:"expected"`
24+
}
25+
1926
type testFile struct {
2027
Filename string
21-
Flag rest.FeatureConfig `json:"flag"`
22-
Segments []rest.Segment `json:"segments"`
23-
Targets []evaluation.Target `json:"targets"`
24-
Expected map[string]interface{} `json:"expected"`
28+
Flags []rest.FeatureConfig `json:"flags"`
29+
Segments []rest.Segment `json:"segments"`
30+
Targets []evaluation.Target `json:"targets"`
31+
Tests []test `json:"tests"`
2532
}
2633

2734
func loadFiles() []testFile {
@@ -62,66 +69,60 @@ func loadFile(filename string) (testFile, error) {
6269
}
6370

6471
func TestEvaluator(t *testing.T) {
65-
type args struct {
66-
file string
67-
target string
68-
expected interface{}
69-
testFile testFile
70-
}
71-
72-
tests := make([]args, 0)
73-
data := loadFiles()
74-
lruCache, err := repository.NewLruCache(1000)
75-
if err != nil {
76-
t.Error(err)
77-
}
78-
repo := repository.New(lruCache)
79-
evaluator, err := evaluation.NewEvaluator(repo, nil)
80-
if err != nil {
81-
t.Error(err)
82-
}
83-
for _, useCase := range data {
84-
useCase.Flag.Feature += useCase.Filename
85-
repo.SetFlag(useCase.Flag)
86-
for _, segment := range useCase.Segments {
87-
repo.SetSegment(segment)
72+
t.Parallel()
73+
fixtures := loadFiles()
74+
for _, fixture := range fixtures {
75+
lruCache, err := repository.NewLruCache(1000)
76+
if err != nil {
77+
t.Error(err)
8878
}
89-
for identifier, value := range useCase.Expected {
90-
tests = append(tests, args{
91-
file: useCase.Filename,
92-
target: identifier,
93-
expected: value,
94-
testFile: useCase,
95-
})
79+
repo := repository.New(lruCache)
80+
evaluator, err := evaluation.NewEvaluator(repo, nil)
81+
if err != nil {
82+
t.Error(err)
83+
}
84+
for _, flag := range fixture.Flags {
85+
repo.SetFlag(flag)
86+
}
87+
for _, segment := range fixture.Segments {
88+
repo.SetSegment(segment)
9689
}
97-
}
9890

99-
for _, testCase := range tests {
100-
t.Run(testCase.testFile.Filename, func(t *testing.T) {
101-
var target *evaluation.Target
102-
if testCase.target != "_no_target" {
103-
for i, val := range testCase.testFile.Targets {
104-
if val.Identifier == testCase.target {
105-
target = &testCase.testFile.Targets[i]
91+
for _, testCase := range fixture.Tests {
92+
testName := fmt.Sprintf("test fixture %s with flag %s", fixture.Filename, testCase.Flag)
93+
if testCase.Target != nil {
94+
testName = fmt.Sprintf("%s and target %s", testName, *testCase.Target)
95+
}
96+
t.Run(testName, func(t *testing.T) {
97+
var target *evaluation.Target
98+
if testCase.Target != nil {
99+
for i, val := range fixture.Targets {
100+
if val.Identifier == *testCase.Target {
101+
target = &fixture.Targets[i]
102+
}
106103
}
107104
}
108-
}
109-
var got interface{}
110-
switch testCase.testFile.Flag.Kind {
111-
case "boolean":
112-
got = evaluator.BoolVariation(testCase.testFile.Flag.Feature, target, false)
113-
case "string":
114-
got = evaluator.StringVariation(testCase.testFile.Flag.Feature, target, "blue")
115-
case "int":
116-
got = evaluator.IntVariation(testCase.testFile.Flag.Feature, target, 100)
117-
case "number":
118-
got = evaluator.NumberVariation(testCase.testFile.Flag.Feature, target, 50.00)
119-
case "json":
120-
got = evaluator.JSONVariation(testCase.testFile.Flag.Feature, target, map[string]interface{}{})
121-
}
122-
if !reflect.DeepEqual(got, testCase.expected) {
123-
t.Errorf("eval engine got = %v, want %v", got, testCase.expected)
124-
}
125-
})
105+
var got interface{}
106+
flag, err := repo.GetFlag(testCase.Flag)
107+
if err != nil {
108+
t.Errorf("flag %s not found", testCase.Flag)
109+
}
110+
switch flag.Kind {
111+
case "boolean":
112+
got = evaluator.BoolVariation(testCase.Flag, target, false)
113+
case "string":
114+
got = evaluator.StringVariation(testCase.Flag, target, "blue")
115+
case "int":
116+
got = evaluator.IntVariation(testCase.Flag, target, 100)
117+
case "number":
118+
got = evaluator.NumberVariation(testCase.Flag, target, 50.00)
119+
case "json":
120+
got = evaluator.JSONVariation(testCase.Flag, target, map[string]interface{}{})
121+
}
122+
if !reflect.DeepEqual(got, testCase.Expected) {
123+
t.Errorf("eval engine got = %v, want %v", got, testCase.Expected)
124+
}
125+
})
126+
}
126127
}
127128
}

0 commit comments

Comments
 (0)