-
Notifications
You must be signed in to change notification settings - Fork 594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use SystemMaterializer in AkkaSpecWithMaterializer #3075
Conversation
Test FAILed. Pull request validation reportFailed Test SuitesTest result for 'akka-http-core / Pr-validation / ./ executeTests'
|
710ac28
to
f8aaaa0
Compare
I thought let's fix this quickly but it was actually quite a ride due to akka/akka#28691 and other issues with assertAllStagesStopped. |
f8aaaa0
to
7b10178
Compare
Test PASSed. |
* use a proper test setup * enable TLS tests Why was the extra materializer needed? To make sure assertAllStagesStopped doesn't stumble over long-running streams (server binding and pool). Now using assertAllStagesStopped is a bit useless as we force more of the background resources to terminate but still cannot hurt. If assertAllStagesStopped fails with weird errors (or just an ask timeout turns up), it's probably because TlsActor doesn't support assertAllStagesStopped. See akka/akka#28691
7b10178
to
8ce3169
Compare
"Http client connections" must { | ||
val address = Await.result( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initialization has been moved to the bottom.
class ClientCancellationSpec extends AkkaSpecWithMaterializer { | ||
// TODO document why this explicit materializer is needed here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra materializer is not needed any more (see explanation in commit message)
}) | ||
|
||
"support cancellation in simple outgoing connection with TLS" in Utils.assertAllStagesStopped(new TestSetup { | ||
pending |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't manage to fix this test here, since Utils.assertAllStagesStopped
is completely broken with TlsModule in place, it was impossible for now to debug what's going on.
* A client transport that will rewrite the target address to a fixed address. This can be used | ||
* to pretend to connect to akka.example.org which is required to connect to the example server certificate. | ||
*/ | ||
def proxyTransport(realAddress: InetSocketAddress): ClientTransport = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ported the trick we used in alpakka to be to test TLS hostname verification in tests.
Test PASSed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
This reverts commit 15f108c.
Currently fails, pushing that here to make sure I remember to fix it enventually.