Skip to content

Commit

Permalink
fix issue with old test, add extra test for local eval
Browse files Browse the repository at this point in the history
  • Loading branch information
neilkakkar committed Mar 15, 2024
1 parent 814f817 commit c512d08
Show file tree
Hide file tree
Showing 4 changed files with 184 additions and 5 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ jobs:
run: go build .

- name: Test
run: go test -v .
run: go test -timeout 30s -v .
156 changes: 154 additions & 2 deletions feature_flags_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package posthog

import (
"bytes"
"encoding/json"
"fmt"
"log"
"net/http"
"net/http/httptest"
"reflect"
"strings"
"time"

"testing"
)
Expand Down Expand Up @@ -613,9 +616,13 @@ func TestFeatureFlagNullComeIntoPlayOnlyWhenDecideErrorsOut(t *testing.T) {

defer server.Close()

// TODO: Make this nicer, right now if all local evaluation requests fail, we block
// on waiting for atleast one request to happen before returning flags,
// which can be suboptimal
client, _ := NewWithConfig("Csyjlnlun3OzyNJAafdlv", Config{
PersonalApiKey: "some very secret key",
Endpoint: server.URL,
PersonalApiKey: "some very secret key",
Endpoint: server.URL,
DefaultFeatureFlagsPollingInterval: 5 * time.Second,
})
defer client.Close()

Expand Down Expand Up @@ -3232,3 +3239,148 @@ func TestComplexCohortsWithNegationLocally(t *testing.T) {
t.Error("Should match")
}
}

func TestFlagWithTimeoutExceeded(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if strings.HasPrefix(r.URL.Path, "/decide") {
time.Sleep(1 * time.Second)
w.Write([]byte(fixture("test-decide-v2.json")))
} else if strings.HasPrefix(r.URL.Path, "/api/feature_flag/local_evaluation") {
w.Write([]byte(fixture("feature_flag/test-flag-group-properties.json")))
} else if strings.HasPrefix(r.URL.Path, "/batch/") {
// Ignore batch requests
} else {
t.Error("Unknown request made by library")
}
}))
defer server.Close()

client, _ := NewWithConfig("Csyjlnlun3OzyNJAafdlv", Config{
PersonalApiKey: "some very secret key",
Endpoint: server.URL,
FeatureFlagRequestTimeout: 10 * time.Millisecond,
})
defer client.Close()

isMatch, err := client.IsFeatureEnabled(
FeatureFlagPayload{
Key: "enabled-flag",
DistinctId: "-",
},
)

if err == nil {
t.Error("Expected error")
}
if !strings.Contains(err.Error(), "context deadline exceeded") {
t.Error("Expected context deadline exceeded error")
}
if isMatch != nil {
t.Error("Flag shouldn't match")
}

// get all flags with no local evaluation possible
variants, err := client.GetAllFlags(
FeatureFlagPayloadNoKey{
DistinctId: "-",
Groups: Groups{"company": "posthog"},
},
)

if err == nil {
t.Error("Expected error")
}
if !strings.Contains(err.Error(), "context deadline exceeded") {
t.Error("Expected context deadline exceeded error")
}

if variants == nil || len(variants) != 0 {
t.Error("Flag shouldn't match")
}

// get all flags with partial local evaluation possible
variants, err = client.GetAllFlags(
FeatureFlagPayloadNoKey{
DistinctId: "-",
Groups: Groups{"company": "posthog"},
PersonProperties: NewProperties().Set("region", "USA"),
},
)

if err == nil {
t.Error("Expected error")
}
if !strings.Contains(err.Error(), "context deadline exceeded") {
t.Error("Expected context deadline exceeded error")
}

if variants == nil || len(variants) != 1 || variants["simple-flag"] != true {
t.Error("should return locally evaluated flag")
}

// get all flags with full local evaluation possible
variants, err = client.GetAllFlags(
FeatureFlagPayloadNoKey{
DistinctId: "-",
Groups: Groups{"company": "posthog"},
PersonProperties: NewProperties().Set("region", "USA"),
GroupProperties: map[string]Properties{"company": NewProperties().Set("name", "Project Name 1")},
},
)

if err != nil {
t.Error("Unexpected error")
}
fmt.Println(variants)

if variants == nil || len(variants) != 2 || variants["simple-flag"] != true || variants["group-flag"] != true {
t.Error("should return locally evaluated flag")
}
}

func TestFlagDefinitionsWithTimeoutExceeded(t *testing.T) {

// create buffer to write logs to
var buf bytes.Buffer

server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if strings.HasPrefix(r.URL.Path, "/decide") {
w.Write([]byte(fixture("test-decide-v2.json")))
} else if strings.HasPrefix(r.URL.Path, "/api/feature_flag/local_evaluation") {
time.Sleep(11 * time.Second)
w.Write([]byte(fixture("feature_flag/test-flag-group-properties.json")))
} else if strings.HasPrefix(r.URL.Path, "/batch/") {
// Ignore batch requests
} else {
t.Error("Unknown request made by library")
}
}))
defer server.Close()

client, _ := NewWithConfig("Csyjlnlun3OzyNJAafdlv", Config{
PersonalApiKey: "some very secret key",
Endpoint: server.URL,
FeatureFlagRequestTimeout: 10 * time.Millisecond,
Logger: StdLogger(log.New(&buf, "posthog-test", log.LstdFlags)),
})
defer client.Close()

_, err := client.IsFeatureEnabled(
FeatureFlagPayload{
Key: "enabled-flag",
DistinctId: "-",
},
)
if err != nil {
t.Error("Unexpected error")
}

output := buf.String()
if !strings.Contains(output, "Unable to fetch feature flags") {
t.Error("Expected error fetching flags")
}

if !strings.Contains(output, "context deadline exceeded") {
t.Error("Expected timeout error fetching flags")
}
}
5 changes: 4 additions & 1 deletion featureflags.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func (poller *FeatureFlagsPoller) GetFeatureFlag(flagConfig FeatureFlagPayload)

result, err = poller.getFeatureFlagVariant(featureFlag, flagConfig.Key, flagConfig.DistinctId, flagConfig.Groups, flagConfig.PersonProperties, flagConfig.GroupProperties)
if err != nil {
return nil, nil
return nil, err
}
}

Expand Down Expand Up @@ -897,6 +897,9 @@ func (poller *FeatureFlagsPoller) getFeatureFlagVariants(distinctId string, grou
defer cancel()
if err != nil || res.StatusCode != http.StatusOK {
errorMessage = "Error calling /decide/"
if err != nil {
errorMessage += " - " + err.Error()
}
poller.Errorf(errorMessage)
return nil, errors.New(errorMessage)
}
Expand Down
26 changes: 25 additions & 1 deletion fixtures/feature_flag/test-flag-group-properties.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"count": 1,
"count": 2,
"next": null,
"previous": null,
"flags": [
Expand Down Expand Up @@ -28,6 +28,30 @@
"active": true,
"is_simple_flag": false,
"rollout_percentage": null
},
{
"id": 719,
"name": "",
"key": "simple-flag",
"filters": {
"groups": [
{
"properties": [
{
"key": "region",
"operator": "exact",
"value": ["USA"],
"type": "person"
}
],
"rollout_percentage": 100
}
]
},
"deleted": false,
"active": true,
"is_simple_flag": true,
"rollout_percentage": null
}
],
"group_type_mapping" : {"0": "company", "1": "project"}
Expand Down

0 comments on commit c512d08

Please sign in to comment.