test: Add reproducer of 4622 (negative connection count)#4855
test: Add reproducer of 4622 (negative connection count)#4855mkleczek wants to merge 3 commits intoPostgREST:mainfrom
Conversation
mkleczek
force-pushed
the
test/4622-reproducer
branch
from
670baba to
c7a8cdb
Compare
| spec :: SpecWith (SpecState, Application) | ||
| spec = describe "Tests using Toxiproxy" $ do | ||
| it "Should return 503 on temporary database server unavailability" $ do | ||
| pendingWith "TODO fix" | ||
| SpecState{specAppState, specToxiProxy} <- getState | ||
|
|
||
| -- make sure there are no open connections | ||
| liftIO $ AppState.flushPool specAppState | ||
|
|
||
| liftBaseDiscard (withDisabled specToxiProxy) $ do | ||
| void $ get "/items?id=eq.5" | ||
| `shouldRespondWith` 503 | ||
|
|
||
| void $ get "/items?id=eq.5" | ||
| `shouldRespondWith` 200 | ||
|
|
||
| liftBaseDiscard (withDisabled specToxiProxy) $ do | ||
| void $ get "/items?id=eq.5" | ||
| `shouldRespondWith` 503 | ||
|
|
||
| it "Must not have negative connection count" $ do | ||
| SpecState{specAppState, specToxiProxy, specMetrics=Metric.MetricsState{poolAvailable}} <- getState | ||
|
|
||
| -- make sure there are no open connections | ||
| liftIO $ AppState.flushPool specAppState | ||
|
|
||
| liftBaseDiscard (withDisabled specToxiProxy) $ do | ||
| replicateM_ 5 $ get "/authors_only?id=eq.5" | ||
| `shouldRespondWith` 503 | ||
|
|
||
| -- TODO https://github.com/PostgREST/postgrest/issues/4622 | ||
| -- change to 0 when fixed | ||
| -- HSpec does not support xfail which should be used instead | ||
| liftIO $ getGauge poolAvailable >>= (`shouldBe` (-5::Int)) . round |
There was a problem hiding this comment.
The benefit of this integration is much more clear now.
| @@ -0,0 +1,347 @@ | |||
| {- | |||
There was a problem hiding this comment.
Regarding #4836 (comment), can't we delete the code we don't use?
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: createProxy
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: createToxic
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: deleteProxy
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: deleteToxic
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: getProxies
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: getProxy
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: getToxic
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: getToxics
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: getVersion
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: postPopulate
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: postReset
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: updateToxic
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:331:1: withToxic
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:343:1: withProxy
From https://github.com/PostgREST/postgrest/actions/runs/25049698636/job/73374301118?pr=4855
There was a problem hiding this comment.
Regarding #4836 (comment), can't we delete the code we don't use?
postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: createProxy postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: createToxic postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: deleteProxy postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: deleteToxic postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: getProxies postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: getProxy postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: getToxic postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: getToxics postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: getVersion postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: postPopulate postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: postReset postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:282:1: updateToxic postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:331:1: withToxic postgrest-15-inplace-observability: test/observability/Toxiproxy.hs:343:1: withProxyFrom https://github.com/PostgREST/postgrest/actions/runs/25049698636/job/73374301118?pr=4855
Yeah... I've left it for now as the code is copied as-is from https://github.com/jpittis/toxiproxy-haskell and I was not sure what to do with it.
Happy to remove unused code, the only thing I am hesitant to delete is withToxic - we do not have these kind of tests yet but I expect to have them in the future.
wolfgangwalther
left a comment
wolfgangwalther
left a comment
There was a problem hiding this comment.
Thinking about whether this should be an hspec or IO test, in light of #4860 (comment) and #4836 (review):
It seems to me like this test would not suffer from the "brittle log checking" problem we currently have with IO tests. Instead we'd check the return of the metrics endpoint, correct?
True - if we had |
mkleczek
force-pushed
the
test/4622-reproducer
branch
from
c7a8cdb to
f546667
Compare
This commit introduces toxiproxy in withTools adding the following helpers: * withToxiproxyServer * withToxiproxyProxy * withToxiproxyPgProxy
mkleczek
force-pushed
the
test/4622-reproducer
branch
from
f546667 to
c02231e
Compare
3 participants
DISCLAIMER:
This commit was authored entirely by a human without the assistance of LLMs.
Stacked on top of #4836
The test can be enabled (ie. the assertion changed to a proper one) in #4672 if this PR is merged into
main.