Try to close reused sockets in TestDynamoDbService #185
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
mmollaverdi
commented
Jun 3, 2024
| @@ -32,6 +32,10 @@ import java.net.InetSocketAddress | |||
| import java.net.ServerSocket | |||
| import java.net.Socket | |||
|
|
|||
| fun pickRandomPort(): Int { | |||
There was a problem hiding this comment.
Bringing this back because it was a public method I had unintentionally deleted in the previous PR.
mmollaverdi
commented
Jun 3, 2024
| } | ||
|
|
||
| private val runningServers = ConcurrentHashMap.newKeySet<String>() | ||
| private val log = getLogger<TestDynamoDbService>() | ||
|
|
||
| @JvmStatic | ||
| fun create(serverFactory: TestDynamoDbServer.Factory<*>, tables: List<TestTable>, port: Int? = null): TestDynamoDbService { | ||
| val portHolder = port?.let { PortHolder(it) } ?: defaultPort(serverFactory.toString()) | ||
| fun create( |
mmollaverdi
commented
Jun 3, 2024
| val socket = allocateRandomPort() | ||
| releasePort = { socket.close() } | ||
| socket.localPort | ||
| val socket = allocatedSockets.getOrPut(key) { allocateRandomPort() } |
There was a problem hiding this comment.
Previously releasePort was a no-op if the key already existed in the map. Now it'll return a function which closes the socket (if not already closed). So if a test didn't call TestDynamoDbServer.startUp (which is where the passed in releasePort function is called before starting a ddb server), then another test reusing that port will close it prior to allocating it to ddb.
kyeotic
approved these changes
Jun 3, 2024
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.
Follow-up from #184 and #182
The previous implementation fails with a port already bound error, if a first test creates a test dynamodb server, but doesn't start and then a second test tries to start the server, because the socket allocated in the first test and reused in the second test doesn't get closed (due to first test not calling
startUp).This PR makes it so that we close the socket if it's not closed even when it's reused across tests.