blaze icon indicating copy to clipboard operation
blaze copied to clipboard

Redesign tests based on `QueueTestHead` and `onFinalizeWeak`

Open RafalSumislawski opened this issue 3 years ago • 2 comments

Http1ClientStageSuite and ClientTimeoutSuite."Idle timeout on slow POST body" use a combo of QueueTestHead to mock the network side of the connection and request.body.onFinalizeWeak to trigger some action in QueueTestHead. In these tests onFinalizeWeak is meant to detect when the request has been sent. The problem is that finalisation of the request body stream, does not guarantee the request has been sent. This can be clearly seen in the code of org.http4s.blazecore.util.EntityBodyWriter#writeEntityBody where writeEnd is executed after drain:

  def writeEntityBody(p: EntityBody[F]): F[Boolean] = {
    val writeBody: F[Unit] = p.through(writePipe).compile.drain
    val writeBodyEnd: F[Boolean] = fromFutureNoShift(F.delay(writeEnd(Chunk.empty)))
    writeBody *> writeBodyEnd
  }

This issue leads to flaky tests like this one: https://github.com/http4s/http4s/runs/4498891671?check_suite_focus=true#step:13:5129

RafalSumislawski avatar Dec 14 '21 18:12 RafalSumislawski

 ==> X org.http4s.blaze.client.Http1ClientStageSuite.Submit a request line with a query  0.017s munit.ComparisonFailException: /home/runner/work/http4s/http4s/blaze-client/src/test/scala/org/http4s/blaze/client/Http1ClientStageSuite.scala:153 Obtained empty output!
152:      val statusLine = request.split("\r\n").apply(0)
153:      assertEquals(statusLine, "GET " + uri + " HTTP/1.1")
154:      assertEquals(response, "done")
    at munit.Assertions.failComparison(Assertions.scala:297)
    at munit.Assertions.failComparison$(Assertions.scala:287)
    at munit.FunSuite.failComparison(FunSuite.scala:11)
    at munit.Assertions$$anon$1.handle(Assertions.scala:31)
    at munit.internal.difflib.Diffs$.assertNoDiff(Diffs.scala:42)
    at munit.Assertions.$anonfun$assertEquals$1(Assertions.scala:141)
    at get @ fs2.Stream$.$anonfun$interruptWhen$3(Stream.scala:1566)
    at map @ org.http4s.blaze.client.Http1ClientStageSuite.$anonfun$getSubmission$14(Http1ClientStageSuite.scala:124)
    at apply @ org.http4s.blaze.client.Http1ClientStageSuite.$anonfun$getSubmission$12(Http1ClientStageSuite.scala:123)
    at fromFuture @ munit.CatsEffectFunFixtures$ResourceFixture$.$anonfun$apply$8(CatsEffectFunFixtures.scala:59)
    at guarantee @ fs2.internal.CompileScope.$anonfun$interruptWhen$1(CompileScope.scala:384)
    at flatMap @ org.http4s.blaze.client.Http1ClientStageSuite.$anonfun$getSubmission$12(Http1ClientStageSuite.scala:123)
    at apply @ org.http4s.blaze.client.Http1ClientStageSuite.$anonfun$getSubmission$10(Http1ClientStageSuite.scala:122)
    at flatMap @ org.http4s.blaze.client.Http1ClientStageSuite.$anonfun$getSubmission$10(Http1ClientStageSuite.scala:122)
    at void @ org.http4s.blaze.client.PoolManager.$anonfun$createConnection$2(PoolManager.scala:122)

I think this issue may be the root cause of why we have marked so many tests in Http1ClientStageSuite as flaky.

RafalSumislawski avatar Dec 14 '21 18:12 RafalSumislawski

Shouldn't this be moved to http4s/blaze ?

FrancescoSerra avatar Jun 17 '22 22:06 FrancescoSerra