[BUG] testing.TestProvider can't reliably be used when tests invoke separate goroutines

[hairyhenderson]

Observed behavior

Currently the testing.TestProvider makes use of goroutine-local variables to store the current test's name, so that the provider can be scoped to a given test.

This has the unfortunate side-effect that tests that need to launch goroutines may panic with the unable to detect test name; be sure to call UsingFlags in the scope of a test (in T.run)!") message.

Expected Behavior

Ideally, the TestProvider should be able to operate reliably when goroutines are involved

Steps to reproduce

No response

[sahidvelji]

Thanks for reporting this. Would you be able to provide a minimal reproducible example demonstrating this bug?

[hairyhenderson]

sorry for the delay - I wasn't able to reproduce this recently (I lost the code where this was originally happening).

As far as I can remember, I think my issue was that I was calling oftesting.NewTestProvider() for each test, meaning the providers map wasn't being shared.

Instead, using a package-global testProvider is more reliable.

I'm going to close this for now, and if I run into this again I'll re-open

[hairyhenderson]

Alright, finally I ran into this again - the key is the bug is triggered when a goroutine is involved in the test.

Here's code to reproduce the bug:

package ofbug

import (
	"fmt"
	"net/http"
	"net/http/httptest"
	"testing"
	"time"

	"github.com/open-feature/go-sdk/openfeature"
	"github.com/open-feature/go-sdk/openfeature/memprovider"
	oftesting "github.com/open-feature/go-sdk/openfeature/testing"
	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

type ofBugHandler struct {
	handlerDone chan struct{}
}

func (d *ofBugHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	ctx := r.Context()

	if openfeature.NewDefaultClient().Boolean(ctx, "myflag", false, openfeature.TransactionContext(ctx)) {
		fmt.Println("evaluated to true")
	}

	ticker := time.NewTicker(500 * time.Millisecond)
	defer ticker.Stop()
	for {
		select {
		case <-r.Context().Done():
			w.WriteHeader(http.StatusServiceUnavailable)
			close(d.handlerDone)
			return
		case <-ticker.C:
			w.WriteHeader(http.StatusOK)
			close(d.handlerDone)
			return
		}
	}
}

func TestServeHTTP_HandlerDone(t *testing.T) {
	ctx := t.Context()

	testProvider := oftesting.NewTestProvider()
	testProvider.UsingFlags(t, map[string]memprovider.InMemoryFlag{
		"myflag": {
			DefaultVariant: "defaultVariant",
			Variants:       map[string]any{"defaultVariant": true},
		},
	})
	_ = openfeature.SetProviderAndWait(testProvider)

	handlerDone := make(chan struct{})
	d := &ofBugHandler{handlerDone: handlerDone}

	req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/drain", nil)
	w := httptest.NewRecorder()

	// Start the handler in a goroutine for _reasons_
	// This is what triggers the TestProvider bug
	go func() {
		d.ServeHTTP(w, req)
	}()

	timedout := false

	// Wait for handler to complete
	select {
	case <-handlerDone:
		assert.Equal(t, http.StatusOK, w.Code)
	case <-time.After(2 * time.Second):
		t.Log("drain not completed within timeout")
		timedout = true
	}

	assert.Equal(t, http.StatusOK, w.Code)
	require.False(t, timedout)
}

This panics with:

panic: unable to detect test name; be sure to call `UsingFlags` in the scope of a test (in T.run)!

goroutine 24 [running]:
github.com/open-feature/go-sdk/openfeature/testing.TestProvider.getProvider({{}, 0x500000000?})
        github.com/open-feature/[email protected]/openfeature/testing/testprovider.go:78 +0x13c
github.com/open-feature/go-sdk/openfeature/testing.TestProvider.BooleanEvaluation({{}, 0x100f8a3c0?}, {0x100dd09f0, 0x140000ee2d0}, {0x100ce9c48, 0x6}, 0x0, 0x140000a3740)
        github.com/open-feature/[email protected]/openfeature/testing/testprovider.go:47 +0x54
github.com/open-feature/go-sdk/openfeature.(*Client).evaluate(0x140000e22d0, {0x100dd09f0, 0x140000ee2d0}, {0x100ce9c48, 0x6}, 0x0, {0x100d6c160, 0x100d48640}, {{0x0?, 0x0?}, ...}, ...)
        github.com/open-feature/[email protected]/openfeature/client.go:743 +0xbc4
github.com/open-feature/go-sdk/openfeature.(*Client).BooleanValueDetails(0x140000e22d0, {0x100dd09f0, 0x140000ee2d0}, {0x100ce9c48, 0x6}, 0x0, {{0x0?, 0x0?}, 0x0?}, {0x0, ...})
        github.com/open-feature/[email protected]/openfeature/client.go:389 +0x18c
github.com/open-feature/go-sdk/openfeature.(*Client).BooleanValue(0x100f7a1a0?, {0x100dd09f0?, 0x140000ee2d0?}, {0x100ce9c48?, 0x0?}, 0x0, {{0x0?, 0x0?}, 0x0?}, {0x0?, ...})
        github.com/open-feature/[email protected]/openfeature/client.go:296 +0x40
github.com/open-feature/go-sdk/openfeature.(*Client).Boolean(...)
        github.com/open-feature/[email protected]/openfeature/client.go:579
ofbug.(*ofBugHandler).ServeHTTP(0x140000a80e8, {0x100dd0610, 0x1400009ec40}, 0x140000f4780)
        ofbug/bug_test.go:24 +0x190
ofbug.TestServeHTTP_HandlerDone.func1()
        ofbug/bug_test.go:65 +0x2c
created by ofbug.TestServeHTTP_HandlerDone in goroutine 22
        ofbug/bug_test.go:64 +0x2c0
FAIL    ofbug   0.299s
FAIL

[erka]

I can't find the original discussion about this but it is designed to be run in goroutine of the test.

https://github.com/open-feature/go-sdk/blob/40fe27eca801dc7ffb4865e403bc2f407fddc339/openfeature/testing/testprovider.go#L34-L37