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:
[email protected]
