KTOR-7359 Implement a suspending version of EmbeddedServer.start and EmbeddedServer.stop #4481
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Use stdlib `use` extension-function * Update type checks
Co-authored-by: Osip Fatkullin <[email protected]>
…torio#4411) * Drop `jvmAndNix` shared source set * Commonize `ktor-network` and `ktor-network-tls` * Support TCP and Unix sockets for wasm-js and js on Node * Move `supportsUnixDomainSockets` to posix and use Platform instead of expect/actual
* Support multiple client engines for JS/WasmJs * CIO client Js/WasmJs support * Enable CIO tests in client tests * Make client engines `data object` to have `toString` * Make ClientLoader work with multiple engines on js/wasmJs and so test CIO engine on nodejs(js+wasmJs)
e5l
approved these changes
Nov 15, 2024
| * That's why we assign port after the server is started in [startServer] | ||
| * Note: this means, that [port] can be used only after calling [createAndStartServer] or [startServer]. | ||
| */ | ||
| private var _port: Int = 0 |
There was a problem hiding this comment.
Please consider extracting a constant for non initialized value and use a different value (other than default port)
| @@ -101,7 +125,8 @@ actual constructor( | |||
| // we start it on the global scope because we don't want it to fail the whole test | |||
| // as far as we have retry loop on call side | |||
| val starting = GlobalScope.async { | |||
| server.start(wait = false) | |||
| server.startSuspend(wait = false) | |||
There was a problem hiding this comment.
Should we make an extension function here like server.listeningPorts()
|
Please check the code style problems |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Subsystem
Server
Motivation
Solution
Only
startSuspendandstopSuspendfunctions were added with minimal bridge for JVM and Native implementations.That was done like this, because in most cases users will anyway call
startandstop, and it's not clear, which engines could supportsuspendstart/stop (e.g CIO can, for sure, but for others it's not that clear).That's why minimally invasive changes were implemented to support wasm-js and js case, where
runBlockingis not available.PR also contains changes to
EngineTestBasefor js and wasm-js as this is the main consumer for those new APIs.Note: probably some commonization is possible for both
EmbeddedServerandEngineTestBase, it should be investigated.