[GitHub] [accumulo] ctubbsii commented on a change in pull request #2007: Set reuseForks to true for unit tests, false for ITs

[GitBox <gi...@apache.org>]

ctubbsii commented on a change in pull request #2007:
URL: https://github.com/apache/accumulo/pull/2007#discussion_r609975385



##########
File path: pom.xml
##########
@@ -814,7 +812,17 @@
           <artifactId>maven-surefire-plugin</artifactId>
           <configuration>
             <forkCount>${surefire.forkCount}</forkCount>
+            <reuseForks>true</reuseForks>
             <excludedGroups>${surefire.excludedGroups}</excludedGroups>
+            <!-- Tests to exclude due to not working with reuseForks being True -->
+            <excludes>
+              <exclude>**/TableIdTest.java</exclude>
+              <exclude>**/HadoopCredentialProviderTest.java</exclude>
+              <exclude>**/IdleRatioScanPrioritizerTest.java</exclude>
+              <exclude>**/AssignmentWatcherTest.java</exclude>
+              <exclude>**/TestCfCqSlice.java</exclude>
+              <exclude>**/TestCfCqSliceSeekingFilter.java</exclude>
+            </excludes>

Review comment:
       As long as this list is small, this is a reasonable solution. Another way to exclude them here and include them in the failsafe plugin is to simply rename them to match the failsafe naming pattern, but I like your solution better, because it highlights the problematic ones.
   
   It would be good to do subsequent work to figure out why these have problems with reusing a fork, and to modify the test (or the relevant Accumulo code) to make them play nice with fork reuse.

##########
File path: pom.xml
##########
@@ -814,7 +812,17 @@
           <artifactId>maven-surefire-plugin</artifactId>
           <configuration>
             <forkCount>${surefire.forkCount}</forkCount>
+            <reuseForks>true</reuseForks>

Review comment:
       Setting this here makes it impossible for the user to override it with `-DreuseForks=false` on the command line if they need to. Did you consider creating a `surefire.reuseForks` and `failsafe.reuseForks`  and a `reuseForks` profile to set them both, like what I did with `forkCount`? See #1756 




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org