ppalaga commented on code in PR #3731:
URL: https://github.com/apache/camel-quarkus/pull/3731#discussion_r850714484
##########
integration-tests/paho-mqtt5/src/main/java/org/apache/camel/quarkus/component/paho/mqtt5/it/PahoMqtt5Resource.java:
##########
@@ -68,13 +70,17 @@ public class PahoMqtt5Resource {
public String consumePahoMessage(
@PathParam("protocol") String protocol,
@PathParam("queueName") String queueName) {
+ String tmpKeystore = null;
+
if ("ssl".equals(protocol)) {
- setKeyStore(keystore, password);
+ tmpKeystore = setKeyStore(keystore, password);
}
+
String result = consumerTemplate.receiveBody("paho-mqtt5:" + queueName
+ "?brokerUrl=" + brokerUrl(protocol), 5000,
String.class);
- if ("ssl".equals(protocol)) {
- removeKeyStore(keystore);
+
+ if ("ssl".equals(protocol) && tmpKeystore != null) {
+ removeKeyStore(tmpKeystore);
Review Comment:
My understanding of reuseForks is that it controls whether JVMs can be
reused for individual test classes within the same Maven module.
To make sure, I have added
```
System.out.println("=== pid(" + getClass().getSimpleName() + ") " +
ManagementFactory.getRuntimeMXBean().getName());
```
to `Base64Test` and `BeanValidatorTest` and I have run
```
cd integration-tests && mvn clean test -pl
:camel-quarkus-integration-test-base64,:camel-quarkus-integration-test-bean-validator
...
=== pid(Base64Test) 111907@terpistone
...
=== pid(BeanValidatorTest) 112141@terpistone
```
So the two tests from two different Maven modules were run in two different
JVMs.
> it is reasonable to clean the environment properties after testing.
+1 on generally wanting to cleanup after the test. The questions in this
particular case are:
1. Whether the cleanup does what it is supposed to do
2. Whether there might be any unwanted side effects.
3. Whether this is a good practice to be followed by end users
For 1., it depends how we define our intention. Removal of the system props
themselves is effective for sure, but I doubt that by doing that we also remove
the SSL context instantiated somewhere deep in Paho. Should that be our goal? I
don't know, probably not. But if we do not remove the SSL context whose state
relies on the props, should we remove the props (and the key store file) at
all? Maybe better not before the the test app is shut down?
For 2., `javax.net.ssl.*` properties are not owned by us. They are owned by
the JVM. There may exist third party code sensitive to the changes. My
impression from what I have read about `javax.net.ssl.*` in the past is, that
they are intended to be passed to a JVM at startup and that they are not so
much intended to be updated at runtime. Will we break any third party code
relying on `javax.net.ssl.*` properties? Probably not (otherwise we'd see
failures in the test).
For 3., I'd say our end users should not be setting `javax.net.ssl.*`
properties at runtime for reasons named in 2. Removing the key store file and
the props may break something in their app. If we keep the code in the test, I
think we should at least add a comment saying something like "This may break
code relying on `javax.net.ssl.*` properties. Do not do this in production.".
Maybe passing the props from a TestResource to the app when it is starting
and removing the key store file after the app is shut down from
`QuarkusTestResourceLifecycleManager.stop()` would address all these questions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
