Listener for state change to get rid of timeout in gherkin tests

[ThomasKrieger]

The gherkin tests are flaky because we need to wait for a state change using a timeout:

Awaitility.await().atMost(Duration.ofSeconds(30)).until(() -> {
            ProviderState providerState1 = client.getProviderState();
            return providerState1 == providerState;
        });

We could add an listener on OpenFeatureClient and FeatureProviderStateManager and notify this listener when the state changes.
Than we could use await, anotify insted of using timeouts.

What do you think?

[chrfwow]

I like the idea.
FYI, there is a PR open that would fix some problems with flaky e2e tests. Here @aepfli suggests using a queue to track the events.

[aepfli]

I am in general always in favor for code improvements ;) - the question is, if this is a good topic for a test. Awaitability is something easy to understand and comprehend, especially in a testcase where it is more about an easy glimpse of what is going on, rather to have the nicest code.

To be honest, I am always really really bad when it comes to those concurrency topics. But we should outline, complexity vs. ease of use/understanding especially as this is a testcase.

[ThomasKrieger]

I think I found the problem why the gherkin test is failing:
It is the following line in ProviderSteps:
117 mockProvider.emitProviderReady(details);
118 mockProvider.emitProviderError(details);

Problem is that emit is using a thread to set the state. see EventProvider line 94.
If I test this with VMLens i see the following in the log:
[ThreadPool-2] INFO dev.openfeature.sdk.FeatureProviderStateManager - Provider unknown transitioned from state NOT_READY to state FATAL
[ThreadPool-1] INFO dev.openfeature.sdk.FeatureProviderStateManager - Provider unknown transitioned from state FATAL to state READY

so by using threads and since we do not wait for the change the order can reverse,

There are to potential solutions:
only call mockProvider.emitProviderError(details); without ready, or wait for the result of emitProvider...

[aepfli]

https://github.com/open-feature/java-sdk/pull/1607/files -> is actually adding some more waits to be sure.

[ThomasKrieger]

The change I made is similar I think:
b818057

[aepfli]

Wow, i should definitely start reading the api documentation. this is so clean

[aepfli]

I think we can close this issue, the new solution is nice and neat, and we do not need to investigate further. If you like to investigate further please let me know.