Skip to content

Commit fb7c824

Browse files
adamwclaude
andauthored
Fix flaky Netty request-timeout metrics tests (#5381)
## Problem The test `properly update metrics when a request times out` (in `NettyFutureRequestTimeoutTests`, and its sync counterpart in `NettySyncRequestTimeoutTests`) is flaky on CI, failing intermittently with `1 was not equal to 0` at the `activeRequests.get() shouldBe 0` assertion. It failed on three dependency-update PRs in the week of 2026-07-06 alone (runs for #5379, #5376 and #5373). The root cause is a race in the test, not in production code: the metrics are only decremented when the endpoint's logic completes, which for the Future-based server happens ~1 second *after* the 503 timeout response is received (the `Future` running the logic is not interruptible). The test used a fixed `Thread.sleep(1100)`, leaving only a ~100 ms margin before asserting — not enough on a loaded CI machine. The sync test had an even tighter fixed `Thread.sleep(100)`. ## Fix Replace the fixed sleeps with `eventually { ... }` and a widened patience config (15 s timeout / 150 ms interval) — the same idiom already used for the same reason in `ServerMetricsTest`. This asserts eventual consistency instead of racing the asynchronous metrics update. No production code is changed. ## Verification Ran locally: - `sbt "nettyServer/testOnly sttp.tapir.server.netty.NettyFutureServerTest -- -z \"properly update metrics\""` — passed - `sbt "nettyServerSync3/testOnly sttp.tapir.server.netty.sync.NettySyncServerTest -- -z \"properly update metrics\""` — passed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent c1e7367 commit fb7c824

2 files changed

Lines changed: 28 additions & 8 deletions

File tree

server/netty-server/src/test/scala/sttp/tapir/server/netty/NettyFutureRequestTimeoutTests.scala

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,21 @@ import cats.effect.kernel.Resource
1414
import scala.concurrent.ExecutionContext
1515
import sttp.client4._
1616
import sttp.capabilities.fs2.Fs2Streams
17+
import org.scalatest.concurrent.Eventually
18+
import org.scalatest.concurrent.Eventually.eventually
1719
import org.scalatest.matchers.should.Matchers._
1820
import cats.effect.unsafe.implicits.global
1921
import sttp.model.StatusCode
2022

2123
class NettyFutureRequestTimeoutTests(eventLoopGroup: EventLoopGroup, backend: WebSocketStreamBackend[IO, Fs2Streams[IO]])(implicit
2224
ec: ExecutionContext
2325
) {
26+
// increase the patience for `eventually` for slow CI tests
27+
implicit val patienceConfig: Eventually.PatienceConfig = Eventually.PatienceConfig(
28+
timeout = org.scalatest.time.Span(15, org.scalatest.time.Seconds),
29+
interval = org.scalatest.time.Span(150, org.scalatest.time.Millis)
30+
)
31+
2432
def tests(): List[Test] = List(
2533
Test("properly update metrics when a request times out") {
2634
val e = endpoint.post
@@ -68,10 +76,12 @@ class NettyFutureRequestTimeoutTests(eventLoopGroup: EventLoopGroup, backend: We
6876
basicRequest.post(uri"http://localhost:$port").body("test").send(backend).map { response =>
6977
response.body should matchPattern { case Left(_) => }
7078
response.code shouldBe StatusCode.ServiceUnavailable
71-
// the metrics will only be updated when the endpoint's logic completes, which is 1 second after receiving the timeout response
72-
Thread.sleep(1100)
73-
activeRequests.get() shouldBe 0
74-
totalRequests.get() shouldBe 1
79+
// the metrics will only be updated when the endpoint's logic completes, which is ~1 second
80+
// after receiving the timeout response (and possibly later on a loaded CI machine)
81+
eventually {
82+
activeRequests.get() shouldBe 0
83+
totalRequests.get() shouldBe 1
84+
}
7585
}
7686
}
7787
.unsafeToFuture()

server/netty-server/sync/src/test/scala/sttp/tapir/server/netty/sync/NettySyncRequestTimeoutTests.scala

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package sttp.tapir.server.netty
33
import cats.effect.IO
44
import cats.effect.unsafe.implicits.global
55
import io.netty.channel.EventLoopGroup
6+
import org.scalatest.concurrent.Eventually
7+
import org.scalatest.concurrent.Eventually.eventually
68
import org.scalatest.matchers.should.Matchers.*
79
import ox.*
810
import sttp.capabilities.fs2.Fs2Streams
@@ -23,6 +25,12 @@ import org.slf4j.LoggerFactory
2325
class NettySyncRequestTimeoutTests(eventLoopGroup: EventLoopGroup, backend: WebSocketStreamBackend[IO, Fs2Streams[IO]]):
2426
val logger = LoggerFactory.getLogger(getClass.getName)
2527

28+
// increase the patience for `eventually` for slow CI tests
29+
implicit val patienceConfig: Eventually.PatienceConfig = Eventually.PatienceConfig(
30+
timeout = org.scalatest.time.Span(15, org.scalatest.time.Seconds),
31+
interval = org.scalatest.time.Span(150, org.scalatest.time.Millis)
32+
)
33+
2634
def tests(): List[Test] = List(
2735
Test("properly update metrics when a request times out") {
2836
val e = endpoint.post
@@ -77,10 +85,12 @@ class NettySyncRequestTimeoutTests(eventLoopGroup: EventLoopGroup, backend: WebS
7785
.map: response =>
7886
response.body should matchPattern { case Left(_) => }
7987
response.code shouldBe StatusCode.ServiceUnavailable
80-
// unlike in NettyFutureRequestTimeoutTest, here interruption works properly, and the metrics should be updated quickly
81-
Thread.sleep(100)
82-
activeRequests.get() shouldBe 0
83-
totalRequests.get() shouldBe 1
88+
// unlike in NettyFutureRequestTimeoutTest, here interruption works properly, and the metrics should be updated
89+
// quickly; however, on a loaded CI machine this might still take some time
90+
eventually {
91+
activeRequests.get() shouldBe 0
92+
totalRequests.get() shouldBe 1
93+
}
8494
.unsafeRunSync()
8595
}
8696
)

0 commit comments

Comments
 (0)