Fix evaluation of numeric attributes that are not passed in as strings or float64s (#53)

* failing test

* tests passing

* bump version

* tab instead of space

* feedback from PR

* buffer time for test

* expect 6 poller calls with explanation why

Author: aarsilv

Makefile

@@ -22,7 +22,7 @@ test-data:
 	git clone -b ${branchName} --depth 1 --single-branch ${githubRepoLink} ${testDataDir}
 
 test: test-data
-	go test ./...
+	go test -v ./...
 
 lint:
 	golangci-lint run

eppoclient/initclient.go

@@ -4,7 +4,7 @@ package eppoclient
 
 import "net/http"
 
-var __version__ = "4.0.0"
+var __version__ = "4.0.1"
 
 // InitClient is required to start polling of experiments configurations and create
 // an instance of EppoClient, which could be used to get assignments information.

eppoclient/poller_test.go

@@ -17,16 +17,14 @@ func (m *CallbackMock) CallbackFn() {
 }
 
 func Test_PollerPoll_InvokesCallbackUntilStoped(t *testing.T) {
-	expected := 5
-
 	callbackMock := CallbackMock{}
 	callbackMock.On("CallbackFn").Return()
 
 	var poller = newPoller(1*time.Second, callbackMock.CallbackFn)
 	poller.Start()
-	time.Sleep(5 * time.Second)
+	time.Sleep(5 * time.Second + 500 * time.Millisecond) // half second buffer to allow polling thread to execute
 	poller.Stop()
-
+	expected := 6 // One call for start(), and then another call each second for 5 seconds before stopped at 5.5 seconds
 	callbackMock.AssertNumberOfCalls(t, "CallbackFn", expected)
 }
 

eppoclient/rules.go

@@ -159,38 +159,6 @@ func isOne(attributeValue interface{}, s string) bool {
 	}
 }
 
-func promoteInt(i interface{}) int64 {
-	switch i := i.(type) {
-	case int:
-		return int64(i)
-	case int8:
-		return int64(i)
-	case int16:
-		return int64(i)
-	case int32:
-		return int64(i)
-	case int64:
-		return i
-	}
-	panic(fmt.Errorf("unexpected type passed to promoteInt: %T", i))
-}
-
-func promoteUint(i interface{}) uint64 {
-	switch i := i.(type) {
-	case uint:
-		return uint64(i)
-	case uint8:
-		return uint64(i)
-	case uint16:
-		return uint64(i)
-	case uint32:
-		return uint64(i)
-	case uint64:
-		return i
-	}
-	panic(fmt.Errorf("unexpected type passed to promoteUint: %T", i))
-}
-
 func evaluateSemVerCondition(subjectValue *semver.Version, conditionValue *semver.Version, condition condition) bool {
 	comp := subjectValue.Compare(conditionValue)
 	switch condition.Operator {
@@ -227,15 +195,61 @@ func evaluateNumericCondition(subjectValue float64, conditionValue float64, cond
 // Returns a float64 and nil error on success, or 0 and an error on failure.
 func toFloat64(val interface{}) (float64, error) {
 	switch v := val.(type) {
-	case float64:
-		return v, nil
+	case float32, float64:
+		return promoteFloat(v), nil 
+	case int, int8, int16, int32, int64:
+		return float64(promoteInt(v)), nil
+	case uint, uint8, uint16, uint32, uint64:
+		return float64(promoteUint(v)), nil
 	case string:
 		floatVal, err := strconv.ParseFloat(v, 64)
 		if err != nil {
 			return 0, fmt.Errorf("cannot convert string '%s' to float64: %w", v, err)
 		}
 		return floatVal, nil
 	default:
-		return 0, errors.New("value is neither a float64 nor a convertible string")
+		return 0, errors.New("value is neither a number nor a convertible string")
+	}
+}
+
+func promoteInt(i interface{}) int64 {
+	switch i := i.(type) {
+	case int:
+		return int64(i)
+	case int8:
+		return int64(i)
+	case int16:
+		return int64(i)
+	case int32:
+		return int64(i)
+	case int64:
+		return i
+	}
+	panic(fmt.Errorf("unexpected type passed to promoteInt: %T", i))
+}
+
+func promoteUint(i interface{}) uint64 {
+	switch i := i.(type) {
+	case uint:
+		return uint64(i)
+	case uint8:
+		return uint64(i)
+	case uint16:
+		return uint64(i)
+	case uint32:
+		return uint64(i)
+	case uint64:
+		return i
+	}
+	panic(fmt.Errorf("unexpected type passed to promoteUint: %T", i))
+}
+
+func promoteFloat(f interface{}) float64 {
+	switch f := f.(type) {
+	case float32:
+		return float64(f)
+	case float64:
+		return f
 	}
+	panic(fmt.Errorf("unexpected type passed to promoteFloat: %T", f))
 }

eppoclient/rules_test.go

@@ -225,14 +225,14 @@ func Test_isNotOneOf_Fail(t *testing.T) {
 	assert.Equal(t, expected, result)
 }
 
-func Test_evaluateNumericcondition_Success(t *testing.T) {
+func Test_evaluateNumericcondition_Fail(t *testing.T) {
 	expected := false
 	result := evaluateNumericCondition(40, 30.0, condition{Operator: "LT", Value: 30.0})
 
 	assert.Equal(t, expected, result)
 }
 
-func Test_evaluateNumericcondition_Fail(t *testing.T) {
+func Test_evaluateNumericcondition_Success(t *testing.T) {
 	expected := true
 	result := evaluateNumericCondition(25, 30.0, condition{Operator: "LT", Value: 30.0})
 
@@ -281,3 +281,45 @@ func Test_isNotNull_attributePresent(t *testing.T) {
 		})
 	assert.True(t, result)
 }
+
+func Test_handles_all_numeric_types(t *testing.T) {
+	condition := condition{Operator: "GT", Attribute: "powerLevel", Value: "9000"}
+	// Floats
+	assert.True(t, condition.matches(Attributes{ "powerLevel": 9001.0}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": 9000.0}) )
+	assert.True(t, condition.matches(Attributes{ "powerLevel": float64(9001)}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": float64(-9001.0)}) )
+	assert.True(t, condition.matches(Attributes{ "powerLevel": float32(9001)}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": float32(8999)}) )
+	// Signed Integers
+	assert.True(t, condition.matches(Attributes{ "powerLevel": 9001}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": 9000}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": int8(1)}) )
+	assert.True(t, condition.matches(Attributes{ "powerLevel": int16(9001)}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": int16(-9002)}) )
+	assert.True(t, condition.matches(Attributes{ "powerLevel": int32(10000)}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": int32(0)}) )
+	assert.True(t, condition.matches(Attributes{ "powerLevel": int64(9001)}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": int64(8999)}) )
+	// Unsigned Integers
+	assert.False(t, condition.matches(Attributes{ "powerLevel": uint8(1)}) )
+	assert.True(t, condition.matches(Attributes{ "powerLevel": uint16(9001)}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": uint16(8999)}) )
+	assert.True(t, condition.matches(Attributes{ "powerLevel": uint32(10000)}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": uint32(0)}) )
+	assert.True(t, condition.matches(Attributes{ "powerLevel": uint64(9001)}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": uint64(8999)}) )
+	// Strings
+	assert.True(t, condition.matches(Attributes{ "powerLevel": "9001"}) )
+	assert.True(t, condition.matches(Attributes{ "powerLevel": "9000.1"}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": "9000"}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": ".2"}) )
+}
+
+func Test_invalid_numeric_types(t *testing.T) {
+	condition := condition{Operator: "GT", Attribute: "powerLevel", Value: "9000"}
+	assert.False(t, condition.matches(Attributes{ "powerLevel": "empty"}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": ""}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": false}) )
+	assert.False(t, condition.matches(Attributes{ "powerLevel": true}) )
+}

eppoclient/utils_test.go

@@ -1,6 +1,7 @@
 package eppoclient
 
 import (
+	"math"
 	"testing"
 )
 
@@ -12,11 +13,21 @@ func TestToFloat64(t *testing.T) {
 		expectErr bool
 	}{
 		{"Float64Input", 123.456, 123.456, false},
-		{"StringInputValid", "789.012", 789.012, false},
+		{"Float32Input", float32(123.456), 123.456, false},
+		{"Int8Input", int8(123), 123.0, false},
+		{"Int16Input", int16(123), 123.0, false},
+		{"Int32Input", int16(123), 123.0, false},
+		{"Int64Input", int16(123), 123.0, false},
+		{"UInt8Input", uint8(123), 123.0, false},
+		{"UInt16Input", uint16(123), 123.0, false},
+		{"UInt32Input", uint16(123), 123.0, false},
+		{"UInt64Input", uint16(123), 123.0, false},
+		{"StringIntInputValid", "789", 789.0, false},
+		{"StringFloatInputValid", "789.012", 789.012, false},
+		{"StringNegativeInputValid", "-789.012", -789.012, false},
 		{"StringInputInvalid", "abc", 0, true},
 		{"SemVerInputInvalid", "1.2.3", 0, true},
 		{"BoolInput", true, 0, true},
-		{"IntInput", 123, 0, true},
 	}
 
 	for _, tt := range tests {
@@ -26,7 +37,8 @@ func TestToFloat64(t *testing.T) {
 				t.Errorf("ToFloat64(%v) error = %v, expectErr %v", tt.input, err, tt.expectErr)
 				return
 			}
-			if !tt.expectErr && result != tt.expected {
+			closeEnough := math.Abs(result - tt.expected)/tt.expected < 0.00001
+			if !tt.expectErr && !closeEnough {
 				t.Errorf("ToFloat64(%v) = %v, want %v", tt.input, result, tt.expected)
 			}
 		})