ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.32k stars 1.63k forks source link

Unstable test "Open circuit should not effect different route" #1706

Closed raman-m closed 10 months ago

raman-m commented 12 months ago

Expected Behavior / New Feature

The test Ocelot.AcceptanceTests.PollyQoSTests.Open_circuit_should_not_effect_different_route doesn't fail in 100% of running

Actual Behavior / Motivation for New Feature

Currently the test Open_circuit_should_not_effect_different_route fails sometimes, very rarely. Possible reason: unknown.

Steps to Reproduce the Problem

Specifications

raman-m commented 12 months ago

@ggnaegi What about next coding challenge? 😉

ggnaegi commented 12 months ago

@raman-m Well, it's very odd... At some point, we have a timing problem... It's very difficult to manage. First I thought, ok instead of using the local variable _requestCount, let's inject a singleton IRequestCountService like this:

.ConfigureServices(services =>
                {
                    services.AddSingleton<IRequestCountService, RequestCountService>();
                })

and then refactor the method like that:

private void GivenThereIsAPossiblyBrokenServiceRunningOn(string url, string responseBody)
        {
            _serviceHandler.GivenThereIsAServiceRunningOn(url, async context =>
            {
                var requestCountService = context.RequestServices.GetService<IRequestCountService>();
                context.Response.StatusCode = 200;

                if (requestCountService.GetRequestCount() == 1)
                {
                    await Task.Delay(1000);
                    await context.Response.WriteAsync("delayed service");
                    return;
                }

                await context.Response.WriteAsync(responseBody);
            });
        }

It brings a bit more stability, but that's not it yet, failing after 200-300 iterations I think

raman-m commented 11 months ago

200-300 iterations

After what number of iterations we have a fail of the test now?

You are assigned! Okay?

ggnaegi commented 11 months ago

@raman-m I will have a go on this next week (wednesday probably)

raman-m commented 10 months ago

@ggnaegicommented on Oct 13

Ah, never mind! Assigned to you with low priority, because it is not critical for builds.

We will fix it in November if you'll have a time... Also, I'm not going to include this to October's release...