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
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()
There was a problem hiding this comment.
I guess that the commend was for this line:
_port = server.engine.resolvedConnectors().first().port
I don't think, that we do really need this, as this is rather specific and mostly needed in tests where we need to launch both server and client (e.g ktor, or in projects like rsocket-kotlin)
|
Please check the code style problems |
whyoleg
force-pushed
the
server-suspend
branch
from
7b68676 to
3c27bb7
Compare
osipxd
approved these changes
Nov 19, 2024
whyoleg
force-pushed
the
server-suspend
branch
from
3c27bb7 to
672ebc6
Compare
osipxd
pushed a commit
that referenced
this pull request
Nov 22, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
e5l
pushed a commit
that referenced
this pull request
Nov 25, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
e5l
pushed a commit
that referenced
this pull request
Nov 26, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
e5l
pushed a commit
that referenced
this pull request
Nov 27, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
e5l
pushed a commit
that referenced
this pull request
Nov 28, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
e5l
pushed a commit
that referenced
this pull request
Nov 29, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
e5l
pushed a commit
that referenced
this pull request
Dec 3, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
e5l
pushed a commit
that referenced
this pull request
Dec 3, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
osipxd
pushed a commit
that referenced
this pull request
Dec 6, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
osipxd
pushed a commit
that referenced
this pull request
Dec 11, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
osipxd
pushed a commit
that referenced
this pull request
Dec 11, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
osipxd
pushed a commit
that referenced
this pull request
Dec 12, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
osipxd
pushed a commit
that referenced
this pull request
Dec 12, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
osipxd
pushed a commit
that referenced
this pull request
Dec 12, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
osipxd
pushed a commit
that referenced
this pull request
Dec 19, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
Stexxe
pushed a commit
that referenced
this pull request
Dec 24, 2024
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
e5l
pushed a commit
that referenced
this pull request
Jan 8, 2025
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
osipxd
pushed a commit
that referenced
this pull request
Jan 8, 2025
…EmbeddedServer.stop (#4481) * Add startSuspend/stopSuspend in EmbeddedServer * Make EngineTestBase work on js/wasmJs
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.