-
Notifications
You must be signed in to change notification settings - Fork 566
Description
New feature, improvement proposal
Hello,
i would like to propose adding a plugin property that allows users to force a specific checksum for looking up the statistics file here:
Lines 1827 to 1829 in 68aefe8
| private File getStatisticsFile(String configurationHash) { | |
| return new File(getBasedir(), ".surefire-" + configurationHash); | |
| } |
my use case is the following:
a single module that has hundreds of test classes, some of which take orders of magnitude longer than others.
when we want to use forkCount higher than 1 to parallelize the execution of tests we run into the problem that very long tests could be grouped into the same bucket, thus leading to a long overall test duration, not taking full advantage of the parallelism.
grouping of tests should be based on their expected runtime, to create balanced buckets.
for this we already have runOrder=balanced:
https://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#runOrder
the problem here is though that the checksum for the statistics file depends on many input variables like plugin version, system properties, env variables etc:
Lines 2576 to 2651 in 68aefe8
| private String getConfigChecksum() { | |
| ChecksumCalculator checksum = new ChecksumCalculator(); | |
| checksum.add(getPluginName()); | |
| checksum.add(isSkipTests()); | |
| checksum.add(isSkipExec()); | |
| checksum.add(isSkip()); | |
| checksum.add(getTestClassesDirectory()); | |
| checksum.add(getMainBuildPath()); | |
| checksum.add(getClasspathDependencyExcludes()); | |
| checksum.add(getClasspathDependencyScopeExclude()); | |
| checksum.add(getAdditionalClasspathElements()); | |
| checksum.add(getReportsDirectory()); | |
| checksum.add(getProjectBuildDirectory()); | |
| checksum.add(getTestSourceDirectory()); | |
| checksum.add(getTest()); | |
| checksum.add(getIncludes()); | |
| checksum.add(getSkipAfterFailureCount()); | |
| checksum.add(getShutdown()); | |
| checksum.add(getExcludes()); | |
| checksum.add(getLocalRepositoryPath()); | |
| checksum.add(getSystemProperties()); | |
| checksum.add(getSystemPropertyVariables()); | |
| checksum.add(getSystemPropertiesFile()); | |
| checksum.add(getProperties()); | |
| checksum.add(isPrintSummary()); | |
| checksum.add(getReportFormat()); | |
| checksum.add(getReportNameSuffix()); | |
| checksum.add(isUseFile()); | |
| checksum.add(isRedirectTestOutputToFile()); | |
| checksum.add(getForkCount()); | |
| checksum.add(isReuseForks()); | |
| checksum.add(getJvm()); | |
| checksum.add(getArgLine()); | |
| checksum.add(getDebugForkedProcess()); | |
| checksum.add(getForkedProcessTimeoutInSeconds()); | |
| checksum.add(getParallelTestsTimeoutInSeconds()); | |
| checksum.add(getParallelTestsTimeoutForcedInSeconds()); | |
| checksum.add(getEnvironmentVariables()); | |
| checksum.add(getExcludedEnvironmentVariables()); | |
| checksum.add(getWorkingDirectory()); | |
| checksum.add(isChildDelegation()); | |
| checksum.add(getGroups()); | |
| checksum.add(getExcludedGroups()); | |
| checksum.add(getIncludeJUnit5Engines()); | |
| checksum.add(getExcludeJUnit5Engines()); | |
| checksum.add(getSuiteXmlFiles()); | |
| checksum.add(getJunitArtifact()); | |
| checksum.add(getTestNGArtifactName()); | |
| checksum.add(getThreadCount()); | |
| checksum.add(getThreadCountSuites()); | |
| checksum.add(getThreadCountClasses()); | |
| checksum.add(getThreadCountMethods()); | |
| checksum.add(getPerCoreThreadCount()); | |
| checksum.add(getUseUnlimitedThreads()); | |
| checksum.add(getParallel()); | |
| checksum.add(isParallelOptimized()); | |
| checksum.add(isTrimStackTrace()); | |
| checksum.add(disableXmlReport); | |
| checksum.add(isUseSystemClassLoader()); | |
| checksum.add(isUseManifestOnlyJar()); | |
| checksum.add(getEncoding()); | |
| checksum.add(isEnableAssertions()); | |
| checksum.add(getObjectFactory()); | |
| checksum.add(getFailIfNoTests()); | |
| checksum.add(getRunOrder()); | |
| checksum.add(getDependenciesToScan()); | |
| checksum.add(getForkedProcessExitTimeoutInSeconds()); | |
| checksum.add(getRerunFailingTestsCount()); | |
| checksum.add(getTempDir()); | |
| checksum.add(useModulePath()); | |
| checksum.add(getEnableProcessChecker()); | |
| checksum.add(isEnableOutErrElements()); | |
| checksum.add(isEnablePropertiesElement()); | |
| addPluginSpecificChecksumItems(checksum); | |
| return checksum.getSha1(); | |
| } |
for our CI system for example we pass the current build number as a system property into the surefire test execution as tests have to use specific build images via testcontainers.
as a result, we are unable to use the statistics file for runOrder=balanced as we get a new checksum for each invocation.
if we could override the checksum to a fixed value with the new property we could make sure to generate balanced buckets by using the statistic files from the last successful build. in our case the duration of a test class hardly ever depends on configuration and just about what the test class is doing.
the currently baked in assumption seems to be that any change to the surefire configuration (i.e. system property) will invalidate all previous test runtime results, but i would argue that for most test classes this is not true.
if such an "escape hatch" property would be accepted, i could work on providing a PR.
thank you!