From fdb184ec9a5f9f027fea7b9975c25b8644e226fb Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Tue, 4 Feb 2025 10:41:37 +0000 Subject: [PATCH] remove classpath jar functionality --- .../commandline/OptionsParser.java | 8 +++-- .../commandline/OptionsParserTest.java | 30 ++----------------- .../mutationtest/config/ReportOptions.java | 10 ------- .../mutationtest/tooling/EntryPoint.java | 3 +- .../java/org/pitest/process/Java9Process.java | 21 +++++++++++-- .../org/pitest/process/LaunchOptions.java | 19 +----------- .../java/org/pitest/process/ProcessArgs.java | 7 +---- .../maven/MojoToReportOptionsConverter.java | 1 - .../main/java/org/pitest/maven/PitMojo.java | 8 +---- .../MojoToReportOptionsConverterTest.java | 13 ++++---- 10 files changed, 34 insertions(+), 86 deletions(-) diff --git a/pitest-command-line/src/main/java/org/pitest/mutationtest/commandline/OptionsParser.java b/pitest-command-line/src/main/java/org/pitest/mutationtest/commandline/OptionsParser.java index 8e8bec934..fc9595f68 100644 --- a/pitest-command-line/src/main/java/org/pitest/mutationtest/commandline/OptionsParser.java +++ b/pitest-command-line/src/main/java/org/pitest/mutationtest/commandline/OptionsParser.java @@ -148,8 +148,12 @@ public class OptionsParser { private final ArgumentAcceptingOptionSpec exportLineCoverageSpec; private final OptionSpec javaExecutable; private final OptionSpec pluginPropertiesSpec; + + // unused but temporarily retained private final OptionSpec testPluginSpec; private final ArgumentAcceptingOptionSpec includeLaunchClasspathSpec; + + // unused but temporarily retained private final ArgumentAcceptingOptionSpec useClasspathJarSpec; private final OptionSpec projectBaseSpec; private final OptionSpec inputEncoding; @@ -463,8 +467,6 @@ private ParseResult parseCommandLine(final ReportOptions data, data.setIncludeLaunchClasspath(booleanValue(includeLaunchClasspathSpec, userArgs)); - data.setUseClasspathJar(booleanValue(useClasspathJarSpec, userArgs)); - data.setShouldCreateTimestampedReports(booleanValue(timestampedReportsSpec, userArgs)); data.setNumberOfThreads(this.threadsSpec.value(userArgs)); @@ -559,7 +561,7 @@ private void setClassPath(final OptionSet userArgs, final ReportOptions data) { LOG.warning("Unable to read class path file:" + this.classPathFile.value(userArgs).getAbsolutePath() + " - " + ioe.getMessage()); } - data.setUseClasspathJar(true); + } elements.addAll(this.additionalClassPathSpec.values(userArgs)); data.setClassPathElements(elements); diff --git a/pitest-command-line/src/test/java/org/pitest/mutationtest/commandline/OptionsParserTest.java b/pitest-command-line/src/test/java/org/pitest/mutationtest/commandline/OptionsParserTest.java index 2bdc9295b..fdce204a8 100644 --- a/pitest-command-line/src/test/java/org/pitest/mutationtest/commandline/OptionsParserTest.java +++ b/pitest-command-line/src/test/java/org/pitest/mutationtest/commandline/OptionsParserTest.java @@ -350,14 +350,6 @@ public void shouldAcceptFileWithListOfAdditionalClassPathElements() { assertTrue(actual.contains("/etc/bar")); } - @Test - public void alsoSetsUseClasspathJarWhenClasspathFileProvided() { - final ClassLoader classLoader = getClass().getClassLoader(); - final File classPathFile = new File(classLoader.getResource("testClassPathFile.txt").getFile()); - final ReportOptions ro = parseAddingRequiredArgs("--classPathFile", - classPathFile.getAbsolutePath()); - assertThat(ro.useClasspathJar()).isTrue(); - } @Test public void shouldFailWhenNoMutationsSetByDefault() { @@ -621,28 +613,10 @@ public void shouldIncludePluginPropertyValuesWhenMultipleKeys() { assertEquals("2", actual.getFreeFormProperties().getProperty("bar")); } - @Test - public void shouldDefaultToNotUsingAClasspathJar() { - final ReportOptions actual = parseAddingRequiredArgs(); - assertFalse(actual.useClasspathJar()); - } - - @Test - public void shouldUseClasspathJarWhenFlagSet() { - final ReportOptions actual = parseAddingRequiredArgs("--useClasspathJar"); - assertTrue(actual.useClasspathJar()); - } - - @Test - public void shouldUseClasspathJarWhenTrueSupplied() { - final ReportOptions actual = parseAddingRequiredArgs("--useClasspathJar=true"); - assertTrue(actual.useClasspathJar()); - } @Test - public void shouldNotUseClasspathJarWhenFalseSupplied() { - final ReportOptions actual = parseAddingRequiredArgs("--useClasspathJar=false"); - assertFalse(actual.useClasspathJar()); + public void shouldNotErrorWhenLegacyClasspathJarWhenFlagSet() { + assertThatCode(() -> parseAddingRequiredArgs("--useClasspathJar")).doesNotThrowAnyException(); } @Test diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/config/ReportOptions.java b/pitest-entry/src/main/java/org/pitest/mutationtest/config/ReportOptions.java index a4ec7f451..031e05d0c 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/config/ReportOptions.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/config/ReportOptions.java @@ -145,7 +145,6 @@ public class ReportOptions { private String testPlugin = ""; - private boolean useClasspathJar; private Path projectBase; private Charset inputEncoding; @@ -625,14 +624,6 @@ public TestPluginArguments createMinionSettings() { this.getIncludedTestMethods(), this.skipFailingTests()); } - public boolean useClasspathJar() { - return useClasspathJar; - } - - public void setUseClasspathJar(boolean useClasspathJar) { - this.useClasspathJar = useClasspathJar; - } - public Path getProjectBase() { return projectBase; } @@ -729,7 +720,6 @@ public String toString() { .add("excludedRunners=" + excludedRunners) .add("includedTestMethods=" + includedTestMethods) .add("testPlugin='" + testPlugin + "'") - .add("useClasspathJar=" + useClasspathJar) .add("projectBase=" + projectBase) .add("inputEncoding=" + inputEncoding) .add("outputEncoding=" + outputEncoding) diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/EntryPoint.java b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/EntryPoint.java index 75be0707f..7b7d46256 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/EntryPoint.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/EntryPoint.java @@ -109,8 +109,7 @@ public AnalysisResult execute(File baseDir, ReportOptions data, final CoverageOptions coverageOptions = settings.createCoverageOptions(); final LaunchOptions launchOptions = new LaunchOptions(ja, - settings.getJavaExecutable(), createJvmArgs(data), environmentVariables) - .usingClassPathJar(data.useClasspathJar()); + settings.getJavaExecutable(), createJvmArgs(data), environmentVariables); final ProjectClassPaths cps = data.getMutationClassPaths(); diff --git a/pitest-entry/src/main/java/org/pitest/process/Java9Process.java b/pitest-entry/src/main/java/org/pitest/process/Java9Process.java index ff6ff03c9..abbcd5255 100644 --- a/pitest-entry/src/main/java/org/pitest/process/Java9Process.java +++ b/pitest-entry/src/main/java/org/pitest/process/Java9Process.java @@ -26,6 +26,10 @@ public class Java9Process implements WrappingProcess { private final Class minionClass; private JavaProcess process; + private final long pid = ProcessHandle.current().pid(); + + private static int counter = 0; + public Java9Process(int port, ProcessArgs args, Class minionClass) { this.port = port; this.processArgs = args; @@ -67,6 +71,10 @@ public void destroy() { this.process.destroy(); } + public JavaProcess getProcess() { + return this.process; + } + private ProcessBuilder createProcessBuilder(String javaProc, List args, Class mainClass, List programArgs, JavaAgent javaAgent, String classPath) { @@ -116,7 +124,13 @@ private List createLaunchArgs(JavaAgent agentJarLocator, List ar } private Path createArgsFile(List cmd) throws IOException { - Path args = Files.createTempFile("pitest-args", ".args"); + // To avoid conflicts with running analysis, we use the PID as part of the file name + // To prevent conflicts between multiple threads counter is used + // All files should be deleted on process exit, although some garbage may be left + // if the process is killed. Files are however created in the system temp directory + // so should be cleaned up on reboot + String name = "pitest-args-" + pid + "-" + nextCounter(); + Path args = Files.createTempFile(name, ".args"); args.toFile().deleteOnExit(); Files.write(args, cmd); return args; @@ -143,7 +157,8 @@ private static Predicate isJavaAgentParam() { return a -> a.toLowerCase().startsWith("-javaagent"); } - public JavaProcess getProcess() { - return this.process; + private static synchronized int nextCounter() { + return counter++; } + } diff --git a/pitest-entry/src/main/java/org/pitest/process/LaunchOptions.java b/pitest-entry/src/main/java/org/pitest/process/LaunchOptions.java index 170356728..fa1fc8bb1 100644 --- a/pitest-entry/src/main/java/org/pitest/process/LaunchOptions.java +++ b/pitest-entry/src/main/java/org/pitest/process/LaunchOptions.java @@ -25,30 +25,20 @@ public class LaunchOptions { private final List childJVMArgs; private final JavaExecutableLocator javaExecutable; private final Map environmentVariables; - private final boolean usingClassPathJar; public LaunchOptions(JavaAgent javaAgentFinder) { this(javaAgentFinder, new DefaultJavaExecutableLocator(), Collections .emptyList(), new HashMap<>()); } - - public LaunchOptions(JavaAgent javaAgentFinder, - JavaExecutableLocator javaExecutable, - List childJVMArgs, - Map environmentVariables) { - this(javaAgentFinder, javaExecutable, childJVMArgs, environmentVariables, false); - } public LaunchOptions(JavaAgent javaAgentFinder, JavaExecutableLocator javaExecutable, List childJVMArgs, - Map environmentVariables, - boolean usingClassPathJar) { + Map environmentVariables) { this.javaAgentFinder = javaAgentFinder; this.childJVMArgs = childJVMArgs; this.javaExecutable = javaExecutable; this.environmentVariables = environmentVariables; - this.usingClassPathJar = usingClassPathJar; } public JavaAgent getJavaAgentFinder() { @@ -67,11 +57,4 @@ public Map getEnvironmentVariables() { return this.environmentVariables; } - public LaunchOptions usingClassPathJar(boolean useJar) { - return new LaunchOptions(javaAgentFinder, javaExecutable, childJVMArgs, environmentVariables, useJar); - } - - public boolean useClasspathJar() { - return usingClassPathJar; - } } diff --git a/pitest-entry/src/main/java/org/pitest/process/ProcessArgs.java b/pitest-entry/src/main/java/org/pitest/process/ProcessArgs.java index 34aae0570..9bec81152 100644 --- a/pitest-entry/src/main/java/org/pitest/process/ProcessArgs.java +++ b/pitest-entry/src/main/java/org/pitest/process/ProcessArgs.java @@ -35,7 +35,6 @@ public final class ProcessArgs { private File workingDir = null; private String javaExecutable; private Map environmentVariables; - private boolean useClasspathJar = false; private ProcessArgs(final String launchClassPath) { this.launchClassPath = launchClassPath; @@ -92,16 +91,12 @@ public String getJavaExecutable() { return this.javaExecutable; } - public boolean useClasspathJar() { - return useClasspathJar; - } - + public ProcessArgs andLaunchOptions(final LaunchOptions launchOptions) { this.jvmArgs = launchOptions.getChildJVMArgs(); this.javaAgentFinder = launchOptions.getJavaAgentFinder(); this.javaExecutable = launchOptions.getJavaExecutable(); this.environmentVariables = launchOptions.getEnvironmentVariables(); - this.useClasspathJar = launchOptions.useClasspathJar(); return this; } diff --git a/pitest-maven/src/main/java/org/pitest/maven/MojoToReportOptionsConverter.java b/pitest-maven/src/main/java/org/pitest/maven/MojoToReportOptionsConverter.java index a742fb635..1f9c98b8b 100644 --- a/pitest-maven/src/main/java/org/pitest/maven/MojoToReportOptionsConverter.java +++ b/pitest-maven/src/main/java/org/pitest/maven/MojoToReportOptionsConverter.java @@ -209,7 +209,6 @@ private ReportOptions parseReportOptions(final List classPath) { data.setCodePaths(codePaths); } - data.setUseClasspathJar(this.mojo.isUseClasspathJar()); data.setClassPathElements(classPath); data.setFailWhenNoMutations(shouldFailWhenNoMutations()); diff --git a/pitest-maven/src/main/java/org/pitest/maven/PitMojo.java b/pitest-maven/src/main/java/org/pitest/maven/PitMojo.java index c55b3560a..ae41bbd32 100644 --- a/pitest-maven/src/main/java/org/pitest/maven/PitMojo.java +++ b/pitest-maven/src/main/java/org/pitest/maven/PitMojo.java @@ -423,9 +423,7 @@ public final class PitMojo extends AbstractMojo { /** - * Communicate the classpath using a temporary jar with a classpath - * manifest. This allows support of very large classpaths but may cause - * issues with certain libraries. + * Unused since 1.18.0. Temporarily left in place */ @Parameter(property = "useClasspathJar", defaultValue = "false") private boolean useClasspathJar; @@ -824,10 +822,6 @@ public ArrayList getFeatures() { return consolidated; } - public boolean isUseClasspathJar() { - return this.useClasspathJar; - } - public String getVerbosity() { return verbosity; } diff --git a/pitest-maven/src/test/java/org/pitest/maven/MojoToReportOptionsConverterTest.java b/pitest-maven/src/test/java/org/pitest/maven/MojoToReportOptionsConverterTest.java index e9bd66925..5e20c7ac9 100644 --- a/pitest-maven/src/test/java/org/pitest/maven/MojoToReportOptionsConverterTest.java +++ b/pitest-maven/src/test/java/org/pitest/maven/MojoToReportOptionsConverterTest.java @@ -416,14 +416,11 @@ public void testParsesCustomProperties() { assertEquals("bar", actual.getFreeFormProperties().get("bar")); } - public void testDoesNotUseClasspathJarByDefault() { - final ReportOptions actual = parseConfig(""); - assertFalse(actual.useClasspathJar()); - } - - public void testParsesUseClasspathJar() { - final ReportOptions actual = parseConfig("true"); - assertTrue(actual.useClasspathJar()); + + public void testLegacyClasspathJarParamDoesNotCauseError() { + assertThatCode(() -> parseConfig("true")) + .doesNotThrowAnyException(); + } public void testFailsIfObsoleteMaxMutationsParameterUsed() {