Improve available port discovery in tests #3555
Conversation
| private static String getCallerClassName() { | ||
| return StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE) | ||
| .walk(s -> s.map(StackWalker.StackFrame::getClassName) | ||
| .filter(className -> !className.equals(AvailablePortFinder.class.getName())) |
There was a problem hiding this comment.
Shouldn't we make AvailablePortFinder class final, to better match the assumption made on the above line?
There was a problem hiding this comment.
Shouldn't we make AvailablePortFinder class final
The class declaration is:
public final class AvailablePortFinder {
}
There was a problem hiding this comment.
Filtering on className should be enough then.
There was a problem hiding this comment.
The class declaration is:
public final class AvailablePortFinder { }
I am blind. Sorry for the noise.
...ts-support/test-support/src/main/java/org/apache/camel/quarkus/test/AvailablePortFinder.java
Show resolved
Hide resolved
| LOGGER.info("getNextAvailable() -> {}", port); | ||
| return port; | ||
| String callerClassName = getCallerClassName(); | ||
| String value = RESERVED_PORTS.putIfAbsent(port, callerClassName); |
There was a problem hiding this comment.
Is this making some assumptions about whether this class can concurrently exist in multiple JVMs or multiple class loaders within a single maven invocation? IIRC surefire is forking a new JVM for each Maven module. So if building with mvnd, the AvailablePortFinder class (and thus also separate RESERVED_PORTS instances) may exist in several concurrent JVMs.
There was a problem hiding this comment.
Can you elaborate a bit more? It's not entirely clear to me why it matters in this context.
All it's doing is trying to track which class reserved a specific port. Arguably we don't need to do that. It just provides some nice logging output and a way to clear out stale entries from the RESERVED_PORTS map.
There was a problem hiding this comment.
Ok - after re-reading. It's about separate RESERVED_PORTS instances.
In which case, yes, I suppose the map contents would be different between concurrent JVMs or class loaders.
There was a problem hiding this comment.
Thanks for explaining. If it is just for logging, indeed, multiple RESERVED_PORTS instances in concurrent per-module JVMs do not matter.
No description provided.