gnodet-bot commented on code in PR #29365:
URL: https://github.com/apache/camel/pull/29365#discussion_r4498855739
##########
components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpConcurrencyIT.java:
##########
@@ -39,6 +40,7 @@
* Integration tests for MINA SFTP concurrency and thread safety.
*/
Review Comment:
💡 **Missing isolation rationale:** `MinaSftpConfigurationValidationIT`
documents why it needs an isolated JVM (global `PathUtils` state that can't be
reset). Consider adding a brief Javadoc block here too explaining what shared
state or port-binding issue makes `MinaSftpConcurrencyIT` require isolation —
it helps future contributors understand why this test can't share a JVM.
##########
components/camel-mina-sftp/pom.xml:
##########
@@ -135,11 +135,29 @@
<artifactId>maven-failsafe-plugin</artifactId>
<executions>
<execution>
+ <id>standard-integration-tests</id>
<goals>
<goal>integration-test</goal>
<goal>verify</goal>
</goals>
<configuration>
+ <excludedGroups>isolated</excludedGroups>
+ <forkCount>1</forkCount>
+ <reuseForks>true</reuseForks>
+ </configuration>
+ </execution>
Review Comment:
💡 **Tag name diverges from project convention:** `camel-http` uses
`"require-isolated-jvm"` for the same pattern (tests that must run in a fresh
JVM). Using `"isolated"` here creates a second tag name for the same concept,
making it harder to query all isolated-JVM tests across the project with a
single `-Dgroups` selector. Consider aligning on `require-isolated-jvm`.
##########
components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/sftp/SftpChangedReadLockIT.java:
##########
@@ -34,6 +35,7 @@
import static org.apache.camel.test.junit6.TestSupport.createDirectory;
import static org.junit.jupiter.api.Assertions.assertEquals;
+@Tag("isolated")
Review Comment:
💡 **Missing isolation rationale:** Same note as `MinaSftpConcurrencyIT` — a
brief comment or Javadoc explaining why `SftpChangedReadLockIT` needs a fresh
JVM would make this more maintainable.
--
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]
