[FEATURE] New test-oriented client

[hairyhenderson]

Requirements

Right now it's a bit awkward to write unit tests that depend on different values being injected. This is largely because of the singleton evaluation API - if a new default provider is set with some flag values set a certain way, that may cause a test running concurrently to fail.

One option for getting around this is to use named providers and clients, but that can be awkward, and adds the requirement of tracking arbitrary strings for specific tests.

I propose a new test package (github.com/open-feature/go-sdk/openfeature/test) that would contain a client (conforming to openfeature.IClient) with a directly-attached in-memory provider to make it simpler to write unit tests.

(See related Slack conversation)

[Kavindu-Dodan]

@hairyhenderson thanks for this concern and I went through the slack conversation. To add some detail, we use the same approach in Java [1]. And I agree that testing becomes difficult when there's the involvement of a singleton.

Regarding your proposal "I propose a new test package", I would like to know how this could solve the issue with singleton usage ? From what I see, even if we expose a test package when dealing with Openfeature API (ex: openfeature.SetProviderAndWait(provider)), the API will utilize the singleton in the background for provider and client handling. So the underlying issue of dealing with singleton should still exist.

Also, could you please share your specific testing requirement to understand the problem better ?

[1] - https://github.com/open-feature/java-sdk/blob/v1.7.6/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java#L37-L39

[evankanderson]

I just ran across this today; I'm doing something like:

            inMemFlag := memprovider.NewInMemoryProvider(map[string]memprovider.InMemoryFlag{
                string(flags.IDPResolver): {
                    Key:            string(flags.IDPResolver),
                    DefaultVariant: "Fixed",
                    Variants:       map[string]any{"Fixed": tc.idpFlag},
                },
            })
            assert.NoError(t, openfeature.SetProvider(inMemFlag))
            featureClient := openfeature.NewClient(t.Name())
            t.Cleanup(openfeature.Shutdown)

            // pass featureClient to my application

I'd love to be able to write:

            featureClient := openfeaturetest.NewClient(t, map[string]any{
                string(flags.IDPResolver): tc.idpFlag,  // a bool, in this case
            })
            t.Cleanup(featureClient.Shutdown)

            // pass featureClient to my application

ala the net/http/httptest module which makes it easy to set up an HTTP server for testing. I'd also be okay if openfeaturetest.NewClient simply returned a struct which had a Flags map[string]any that I could use to set the flags on-the-fly. That might actually be nicer:

            featureClient := openfeaturetest.NewClient(t)  // uses t.Name() as the name
            t.Cleanup(featureClient.Shutdown)

            // pass featureClient to my application
            featureClient.Flags[string(flags.IDPResolver)] = tc.idpFlag

[Kavindu-Dodan]

@evankanderson like in your proposal, a mock implementation of IClient should help you with testing code modules that depend on an OpenFeature client. The mock implementation could be as simple as returning mocked values.

For components that rely on provider registration (or need to interact with api, ex:- openfeature.SetProvider(inMemFlag), openfeature.NewClient(t.Name())), I think we lack proper API/interface definition in the SDK. A solution for this could be to expose an interface so that it can be mocked when needed (In Java we expose OpenFeatureAPI.getInstance() returning OpenFeatureAPI instance, which can be mocked for testing needs). Once we define these API/Interfaces, we should be able to see how to expose test components from SDK itself.

[evankanderson]

I actually implemented a little fake here:

https://github.com/stacklok/minder/pull/3155/files#diff-d7ee0c5d18f1dee78d21660300df337c9ec91275e31d20e08b26493fd380c6f5

But it would be great to have a more full-featured fake that permitted something like:

featureClient.SetFlag("idp_resolver", false)
featureClient.SetFlagDetails("other_flag", memprovider.InMemoryFlag{ ... })

And which also supported hooks and handlers, rather than just panicing... 😁

For now, I'm happy to leave the SetProvider and NewClient code un-tested, as it's small -- my main concern is being able to test with and without the flag set in unit tests to ensure that code is covered in both cases.

[Kavindu-Dodan]

@hairyhenderson @evankanderson I opened https://github.com/open-feature/go-sdk/pull/268 to address some of the testing issues you faced. The fix focuses on introducing IOFApi interface which includes most of the OpenFeatuer specification contracts allowing you to mock OpenFeature API.

Further, IOFApi exposes client creation methods that return IClient instances, which allows you to mock client interactions through mock interface implementation.

I haven't come up with a testing client though. However, this is easily doable by having a mock implementation of IClient interface.

[Kavindu-Dodan]

I will close this as we now have test-friendly interfaces exposed from the SDK. This was done through https://github.com/open-feature/go-sdk/pull/268 and released with version 1.12.0