[FFM-1415] added UT to reproduce bug FFM-1415

evaluation/feature.go

@@ -259,27 +259,26 @@ func (fc FeatureConfig) GetVariationName(target *Target) string {
 	if fc.State == FeatureStateOff {
 		return fc.OffVariation
 	}
-	// TODO: variation to target
-	if target != nil {
-		if fc.VariationToTargetMap != nil && len(fc.VariationToTargetMap) > 0 {
-			for _, variationMap := range fc.VariationToTargetMap {
-				if variationMap.Targets != nil {
-					for _, t := range variationMap.Targets {
-						if target.Identifier == t {
-							return variationMap.Variation
-						}
+
+	if target != nil && len(fc.VariationToTargetMap) > 0 {
+		for _, variationMap := range fc.VariationToTargetMap {
+			if variationMap.Targets != nil {
+				for _, t := range variationMap.Targets {
+					if target.Identifier == t {
+						return variationMap.Variation
 					}
 				}
+			}
 
-				if variationMap.TargetSegments != nil {
-					for _, segmentIdentifier := range variationMap.TargetSegments {
-						segment, ok := fc.Segments[segmentIdentifier]
-						if !ok {
-							log.Errorf("The segment [%s] in variation map can not be found for feature %s in project %s", segmentIdentifier, fc.Feature, fc.Project)
-						} else {
-							if segment.Evaluate(target) {
-								return variationMap.Variation
-							}
+			if len(variationMap.TargetSegments) > 0 {
+				for _, segmentIdentifier := range variationMap.TargetSegments {
+					segment, ok := fc.Segments[segmentIdentifier]
+					if !ok {
+						log.Debugf("The segment [%s] in variation map can not be found for feature %s in project %s",
+							segmentIdentifier, fc.Feature, fc.Project)
+					} else {
+						if segment.Evaluate(target) {
+							return variationMap.Variation
 						}
 					}
 				}

evaluation/feature_test.go

@@ -736,3 +736,201 @@ func TestFeatureConfig_GetSegmentIdentifiers(t *testing.T) {
 		})
 	}
 }
+
+func TestFeatureConfig_GetVariationName(t *testing.T) {
+	trueVariation := Variation{
+		Name:       stringPtr("True"),
+		Value:      "true",
+		Identifier: "true",
+	}
+
+	falseVariation := Variation{
+		Name:       stringPtr("False"),
+		Value:      "false",
+		Identifier: "false",
+	}
+	type fields struct {
+		DefaultServe         Serve
+		Environment          string
+		Feature              string
+		Kind                 string
+		OffVariation         string
+		Prerequisites        []Prerequisite
+		Project              string
+		Rules                ServingRules
+		State                FeatureState
+		VariationToTargetMap []VariationMap
+		Variations           Variations
+		Segments             map[string]*Segment
+	}
+	type args struct {
+		target *Target
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		args   args
+		want   string
+	}{
+		{
+			name: "target is null it should evaluate rule and expect true",
+			fields: fields{
+				DefaultServe: Serve{
+					Variation: stringPtr("true"),
+				},
+				Environment:          "dev",
+				Feature:              "bool-flag",
+				Kind:                 "boolean",
+				OffVariation:         "false",
+				Prerequisites:        nil,
+				Project:              "default",
+				Rules:                nil,
+				State:                "on",
+				VariationToTargetMap: nil,
+				Variations: Variations{
+					trueVariation, falseVariation,
+				},
+				Segments: nil,
+			},
+			args: args{target: nil},
+			want: "true",
+		},
+		{
+			name: "target is not null it should evaluate variationMap with target and expect false",
+			fields: fields{
+				DefaultServe: Serve{
+					Variation: stringPtr("true"),
+				},
+				Environment:   "dev",
+				Feature:       "bool-flag",
+				Kind:          "boolean",
+				OffVariation:  "false",
+				Prerequisites: nil,
+				Project:       "default",
+				Rules:         nil,
+				State:         "on",
+				VariationToTargetMap: []VariationMap{
+					{
+						TargetSegments: nil,
+						Targets:        []string{"harness"},
+						Variation:      "false",
+					},
+				},
+				Variations: Variations{
+					trueVariation, falseVariation,
+				},
+				Segments: nil,
+			},
+			args: args{
+				target: &Target{
+					Identifier: "harness",
+					Name:       "Harness",
+					Anonymous:  nil,
+					Attributes: nil,
+				},
+			},
+			want: "false",
+		},
+		{
+			name: "target is not null it should evaluate variationMap with segment and expect false",
+			fields: fields{
+				DefaultServe: Serve{
+					Variation: stringPtr("true"),
+				},
+				Environment:   "dev",
+				Feature:       "bool-flag",
+				Kind:          "boolean",
+				OffVariation:  "false",
+				Prerequisites: nil,
+				Project:       "default",
+				Rules:         nil,
+				State:         "on",
+				VariationToTargetMap: []VariationMap{
+					{
+						TargetSegments: []string{"beta"},
+						Targets:        []string{"johndoe"},
+						Variation:      "false",
+					},
+				},
+				Variations: Variations{
+					trueVariation, falseVariation,
+				},
+				Segments: Segments{
+					"beta": &Segment{
+						Identifier: "beta",
+						Included:   []string{"harness"},
+					},
+				},
+			},
+			args: args{
+				target: &Target{
+					Identifier: "harness",
+					Name:       "Harness",
+					Anonymous:  nil,
+					Attributes: nil,
+				},
+			},
+			want: "false",
+		},
+		{
+			name: "target is not null it should evaluate variationMap with segments but segment not found and expect true",
+			fields: fields{
+				DefaultServe: Serve{
+					Variation: stringPtr("true"),
+				},
+				Environment:   "dev",
+				Feature:       "bool-flag",
+				Kind:          "boolean",
+				OffVariation:  "false",
+				Prerequisites: nil,
+				Project:       "default",
+				Rules:         nil,
+				State:         "on",
+				VariationToTargetMap: []VariationMap{
+					{
+						TargetSegments: []string{"beta"},
+						Targets:        []string{"johndoe"},
+						Variation:      "false",
+					},
+				},
+				Variations: Variations{
+					trueVariation, falseVariation,
+				},
+				Segments: Segments{
+					"alpha": &Segment{
+						Identifier: "alpha",
+						Included:   []string{"harness"},
+					},
+				},
+			},
+			args: args{
+				target: &Target{
+					Identifier: "harness",
+					Name:       "Harness",
+					Anonymous:  nil,
+					Attributes: nil,
+				},
+			},
+			want: "true",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			fc := FeatureConfig{
+				DefaultServe:         tt.fields.DefaultServe,
+				Environment:          tt.fields.Environment,
+				Feature:              tt.fields.Feature,
+				Kind:                 tt.fields.Kind,
+				OffVariation:         tt.fields.OffVariation,
+				Prerequisites:        tt.fields.Prerequisites,
+				Project:              tt.fields.Project,
+				Rules:                tt.fields.Rules,
+				State:                tt.fields.State,
+				VariationToTargetMap: tt.fields.VariationToTargetMap,
+				Variations:           tt.fields.Variations,
+				Segments:             tt.fields.Segments,
+			}
+			assert.Equalf(t, tt.want, fc.GetVariationName(tt.args.target), "GetVariationName(%v)", tt.args.target)
+		})
+	}
+}

tests/evaluator_test.go

@@ -2,6 +2,7 @@ package tests
 
 import (
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"path/filepath"
 	"reflect"
@@ -16,12 +17,18 @@ import (
 
 const source = "./ff-test-cases/tests"
 
+type test struct {
+	Flag     string      `json:"flag"`
+	Target   *string     `json:"target"`
+	Expected interface{} `json:"expected"`
+}
+
 type testFile struct {
 	Filename string
-	Flag     rest.FeatureConfig     `json:"flag"`
-	Segments []rest.Segment         `json:"segments"`
-	Targets  []evaluation.Target    `json:"targets"`
-	Expected map[string]interface{} `json:"expected"`
+	Flags    []rest.FeatureConfig `json:"flags"`
+	Segments []rest.Segment       `json:"segments"`
+	Targets  []evaluation.Target  `json:"targets"`
+	Tests    []test               `json:"tests"`
 }
 
 func loadFiles() []testFile {
@@ -62,66 +69,60 @@ func loadFile(filename string) (testFile, error) {
 }
 
 func TestEvaluator(t *testing.T) {
-	type args struct {
-		file     string
-		target   string
-		expected interface{}
-		testFile testFile
-	}
-
-	tests := make([]args, 0)
-	data := loadFiles()
-	lruCache, err := repository.NewLruCache(1000)
-	if err != nil {
-		t.Error(err)
-	}
-	repo := repository.New(lruCache)
-	evaluator, err := evaluation.NewEvaluator(repo, nil)
-	if err != nil {
-		t.Error(err)
-	}
-	for _, useCase := range data {
-		useCase.Flag.Feature += useCase.Filename
-		repo.SetFlag(useCase.Flag)
-		for _, segment := range useCase.Segments {
-			repo.SetSegment(segment)
+	t.Parallel()
+	fixtures := loadFiles()
+	for _, fixture := range fixtures {
+		lruCache, err := repository.NewLruCache(1000)
+		if err != nil {
+			t.Error(err)
 		}
-		for identifier, value := range useCase.Expected {
-			tests = append(tests, args{
-				file:     useCase.Filename,
-				target:   identifier,
-				expected: value,
-				testFile: useCase,
-			})
+		repo := repository.New(lruCache)
+		evaluator, err := evaluation.NewEvaluator(repo, nil)
+		if err != nil {
+			t.Error(err)
+		}
+		for _, flag := range fixture.Flags {
+			repo.SetFlag(flag)
+		}
+		for _, segment := range fixture.Segments {
+			repo.SetSegment(segment)
 		}
-	}
 
-	for _, testCase := range tests {
-		t.Run(testCase.testFile.Filename, func(t *testing.T) {
-			var target *evaluation.Target
-			if testCase.target != "_no_target" {
-				for i, val := range testCase.testFile.Targets {
-					if val.Identifier == testCase.target {
-						target = &testCase.testFile.Targets[i]
+		for _, testCase := range fixture.Tests {
+			testName := fmt.Sprintf("test fixture %s with flag %s", fixture.Filename, testCase.Flag)
+			if testCase.Target != nil {
+				testName = fmt.Sprintf("%s and target %s", testName, *testCase.Target)
+			}
+			t.Run(testName, func(t *testing.T) {
+				var target *evaluation.Target
+				if testCase.Target != nil {
+					for i, val := range fixture.Targets {
+						if val.Identifier == *testCase.Target {
+							target = &fixture.Targets[i]
+						}
 					}
 				}
-			}
-			var got interface{}
-			switch testCase.testFile.Flag.Kind {
-			case "boolean":
-				got = evaluator.BoolVariation(testCase.testFile.Flag.Feature, target, false)
-			case "string":
-				got = evaluator.StringVariation(testCase.testFile.Flag.Feature, target, "blue")
-			case "int":
-				got = evaluator.IntVariation(testCase.testFile.Flag.Feature, target, 100)
-			case "number":
-				got = evaluator.NumberVariation(testCase.testFile.Flag.Feature, target, 50.00)
-			case "json":
-				got = evaluator.JSONVariation(testCase.testFile.Flag.Feature, target, map[string]interface{}{})
-			}
-			if !reflect.DeepEqual(got, testCase.expected) {
-				t.Errorf("eval engine got = %v, want %v", got, testCase.expected)
-			}
-		})
+				var got interface{}
+				flag, err := repo.GetFlag(testCase.Flag)
+				if err != nil {
+					t.Errorf("flag %s not found", testCase.Flag)
+				}
+				switch flag.Kind {
+				case "boolean":
+					got = evaluator.BoolVariation(testCase.Flag, target, false)
+				case "string":
+					got = evaluator.StringVariation(testCase.Flag, target, "blue")
+				case "int":
+					got = evaluator.IntVariation(testCase.Flag, target, 100)
+				case "number":
+					got = evaluator.NumberVariation(testCase.Flag, target, 50.00)
+				case "json":
+					got = evaluator.JSONVariation(testCase.Flag, target, map[string]interface{}{})
+				}
+				if !reflect.DeepEqual(got, testCase.Expected) {
+					t.Errorf("eval engine got = %v, want %v", got, testCase.Expected)
+				}
+			})
+		}
 	}
 }