From 0316619eceb0405d9c2016be0d15269e4567283f Mon Sep 17 00:00:00 2001 From: henry Date: Fri, 9 Feb 2024 10:51:31 +0000 Subject: [PATCH] replace standard maven properties imported from surefire Surefire argLine parsing now controlled by a flag to allow easy disable if it causes problems. --- .../src/test/java/org/pitest/PitMojoIT.java | 2 ++ .../resources/pit-surefire-excludes/pom.xml | 4 +++- .../test/java/com/example/AnotherTest.java | 6 +++++ .../org/pitest/maven/AbstractPitMojo.java | 13 ++++++++++- .../maven/MojoToReportOptionsConverter.java | 23 +++++++++++++++---- .../main/java/org/pitest/maven/ScmMojo.java | 2 +- .../pitest/maven/SurefireConfigConverter.java | 19 +++++++++------ .../MojoToReportOptionsConverterTest.java | 12 +++++++++- .../maven/SurefireConfigConverterTest.java | 20 ++++++++++++---- 9 files changed, 81 insertions(+), 20 deletions(-) diff --git a/pitest-maven-verification/src/test/java/org/pitest/PitMojoIT.java b/pitest-maven-verification/src/test/java/org/pitest/PitMojoIT.java index b3335a8d3..47db7ecd3 100755 --- a/pitest-maven-verification/src/test/java/org/pitest/PitMojoIT.java +++ b/pitest-maven-verification/src/test/java/org/pitest/PitMojoIT.java @@ -321,6 +321,8 @@ public void shouldCorrectlyHandleOverrides() throws Exception { @Test public void shouldReadExclusionsFromSurefireConfig() throws Exception { + // Note this test also tests the argline parsing concern + File testDir = prepare("/pit-surefire-excludes"); verifier.executeGoal("test"); verifier.executeGoal("org.pitest:pitest-maven:mutationCoverage"); diff --git a/pitest-maven-verification/src/test/resources/pit-surefire-excludes/pom.xml b/pitest-maven-verification/src/test/resources/pit-surefire-excludes/pom.xml index d7fc9121d..e9800efc2 100644 --- a/pitest-maven-verification/src/test/resources/pit-surefire-excludes/pom.xml +++ b/pitest-maven-verification/src/test/resources/pit-surefire-excludes/pom.xml @@ -15,6 +15,8 @@ + isSet + alsoSet @@ -37,7 +39,7 @@ **/FailingTest.java **/BadTest.java - @{argLine} -DnotNeeded=avalue + ${argLine} -DMUST_BE_SET=${a.value} -DMUST_ALSO_BE_SET=@{b.value} diff --git a/pitest-maven-verification/src/test/resources/pit-surefire-excludes/src/test/java/com/example/AnotherTest.java b/pitest-maven-verification/src/test/resources/pit-surefire-excludes/src/test/java/com/example/AnotherTest.java index 7a5871b53..b404d1bf3 100644 --- a/pitest-maven-verification/src/test/resources/pit-surefire-excludes/src/test/java/com/example/AnotherTest.java +++ b/pitest-maven-verification/src/test/resources/pit-surefire-excludes/src/test/java/com/example/AnotherTest.java @@ -18,4 +18,10 @@ public void anotherTest() { assertEquals(1, Covered.someCode(1)); } + @Test + public void dependsOnArgLine() { + assertEquals("isSet", System.getProperty("MUST_BE_SET")); + assertEquals("alsoSet", System.getProperty("MUST_ALSO_BE_SET")); + } + } diff --git a/pitest-maven/src/main/java/org/pitest/maven/AbstractPitMojo.java b/pitest-maven/src/main/java/org/pitest/maven/AbstractPitMojo.java index bff3f7508..89fb20b5f 100644 --- a/pitest-maven/src/main/java/org/pitest/maven/AbstractPitMojo.java +++ b/pitest-maven/src/main/java/org/pitest/maven/AbstractPitMojo.java @@ -319,6 +319,13 @@ public class AbstractPitMojo extends AbstractMojo { @Parameter(defaultValue = "true") private boolean parseSurefireConfig; + /** + * When set will try and set the argLine based on surefire configuration. This + * may not give the desired result in some circumstances + */ + @Parameter(defaultValue = "true") + private boolean parseSurefireArgLine; + /** * honours common skipTests flag in a maven run */ @@ -515,7 +522,7 @@ private void throwErrorIfMoreThanMaximumSurvivors(final MutationStatistics resul protected Optional analyse() throws MojoExecutionException { final ReportOptions data = new MojoToReportOptionsConverter(this, - new SurefireConfigConverter(), this.filter).convert(); + new SurefireConfigConverter(this.isParseSurefireArgLine()), this.filter).convert(); return Optional.ofNullable(this.goalStrategy.execute(detectBaseDir(), data, this.plugins, this.environmentVariables)); } @@ -726,6 +733,10 @@ public boolean isParseSurefireConfig() { return this.parseSurefireConfig; } + public boolean isParseSurefireArgLine() { + return this.parseSurefireArgLine; + } + public boolean skipFailingTests() { return this.skipFailingTests; } 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 1c5d7c69a..1ae0207e0 100644 --- a/pitest-maven/src/main/java/org/pitest/maven/MojoToReportOptionsConverter.java +++ b/pitest-maven/src/main/java/org/pitest/maven/MojoToReportOptionsConverter.java @@ -95,7 +95,8 @@ public ReportOptions convert() { // argline may contain surefire style properties that require expanding if (effective.getArgLine() != null) { - effective.setArgLine(this.replacePropertyExpressions(option.getArgLine())); + log.info("Replacing properties in argLine " + effective.getArgLine()); + effective.setArgLine(this.replacePropertyExpressions(effective.getArgLine())); } return effective; @@ -488,14 +489,26 @@ private Properties createPluginProperties() { */ private String replacePropertyExpressions(String argLine) { for (Enumeration e = mojo.getProject().getProperties().propertyNames(); e.hasMoreElements();) { + String key = e.nextElement().toString(); - String field = "@{" + key + "}"; - if (argLine.contains(field)) { - argLine = argLine.replace(field, mojo.getProject().getProperties().getProperty(key, "")); - } + + // Replace surefire late evaluation syntax properties + argLine = replaceFieldForSymbol('@', key, argLine); + + // Normally properties will already have been expanded by maven, but this is + // bypassed for argLines pulled from surefire, se we must handle them here + argLine = replaceFieldForSymbol('$', key, argLine); } return argLine; } + private String replaceFieldForSymbol(char symbol, String key, String argLine) { + String field = symbol + "{" + key + "}"; + if (argLine.contains(field)) { + return argLine.replace(field, mojo.getProject().getProperties().getProperty(key, "")); + } + return argLine; + } + } diff --git a/pitest-maven/src/main/java/org/pitest/maven/ScmMojo.java b/pitest-maven/src/main/java/org/pitest/maven/ScmMojo.java index 5912dc63b..83d02cea0 100644 --- a/pitest-maven/src/main/java/org/pitest/maven/ScmMojo.java +++ b/pitest-maven/src/main/java/org/pitest/maven/ScmMojo.java @@ -135,7 +135,7 @@ protected Optional analyse() throws MojoExecutionException { logClassNames(); defaultTargetTestsIfNoValueSet(); final ReportOptions data = new MojoToReportOptionsConverter(this, - new SurefireConfigConverter(), getFilter()).convert(); + new SurefireConfigConverter(this.isParseSurefireArgLine()), getFilter()).convert(); data.setFailWhenNoMutations(false); return Optional.ofNullable(this.getGoalStrategy().execute(detectBaseDir(), data, diff --git a/pitest-maven/src/main/java/org/pitest/maven/SurefireConfigConverter.java b/pitest-maven/src/main/java/org/pitest/maven/SurefireConfigConverter.java index 40da3bdb4..6f0886c4e 100644 --- a/pitest-maven/src/main/java/org/pitest/maven/SurefireConfigConverter.java +++ b/pitest-maven/src/main/java/org/pitest/maven/SurefireConfigConverter.java @@ -22,14 +22,22 @@ */ public class SurefireConfigConverter { - public ReportOptions update(ReportOptions option, Xpp3Dom configuration) { + private final boolean parseArgLine; + + public SurefireConfigConverter(boolean parseArgLine) { + this.parseArgLine = parseArgLine; + } + + public ReportOptions update(ReportOptions option, Xpp3Dom configuration) { if (configuration == null) { return option; } convertExcludes(option, configuration); convertGroups(option, configuration); convertTestFailureIgnore(option, configuration); - convertArgLine(option, configuration); + if (parseArgLine) { + convertArgLine(option, configuration); + } return option; } @@ -70,13 +78,10 @@ private void convertExcludes(ReportOptions option, Xpp3Dom configuration) { } private void convertArgLine(ReportOptions option, Xpp3Dom configuration) { - if (option.getArgLine() != null) { - return; - } - Xpp3Dom argLine = configuration.getChild("argLine"); if (argLine != null) { - option.setArgLine(argLine.getValue()); + String existing = option.getArgLine() != null ? option.getArgLine() + " " : ""; + option.setArgLine(existing + argLine.getValue()); } } 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 211895a52..410a81982 100644 --- a/pitest-maven/src/test/java/org/pitest/maven/MojoToReportOptionsConverterTest.java +++ b/pitest-maven/src/test/java/org/pitest/maven/MojoToReportOptionsConverterTest.java @@ -441,7 +441,7 @@ public void testParsesArgline() { assertThat(actual.getArgLine()).isEqualTo("foo"); } - public void testEvaluatesArgLineProperties() { + public void testEvaluatesSureFireLateEvalArgLineProperties() { properties.setProperty("FOO", "fooValue"); properties.setProperty("BAR", "barValue"); properties.setProperty("UNUSED", "unusedValue"); @@ -449,6 +449,16 @@ public void testEvaluatesArgLineProperties() { assertThat(actual.getArgLine()).isEqualTo("fooValue barValue"); } + public void testEvaluatesNormalPropertiesInArgLines() { + properties.setProperty("FOO", "fooValue"); + properties.setProperty("BAR", "barValue"); + properties.setProperty("UNUSED", "unusedValue"); + // these are normally auto resolved by maven, but if we pull + // in an argline from surefire it will not have been escaped. + ReportOptions actual = parseConfig("${FOO} ${BAR}"); + assertThat(actual.getArgLine()).isEqualTo("fooValue barValue"); + } + public void testAutoAddsKotlinSourceDirsWhenPresent() throws IOException { // we're stuck in junit 3 land but can // use junit 4's temporary folder rule programatically diff --git a/pitest-maven/src/test/java/org/pitest/maven/SurefireConfigConverterTest.java b/pitest-maven/src/test/java/org/pitest/maven/SurefireConfigConverterTest.java index 6a10d5b97..58ebeac20 100644 --- a/pitest-maven/src/test/java/org/pitest/maven/SurefireConfigConverterTest.java +++ b/pitest-maven/src/test/java/org/pitest/maven/SurefireConfigConverterTest.java @@ -17,7 +17,7 @@ public class SurefireConfigConverterTest { - SurefireConfigConverter testee = new SurefireConfigConverter(); + SurefireConfigConverter testee = new SurefireConfigConverter(true); ReportOptions options = new ReportOptions(); Xpp3Dom surefireConfig; @@ -166,16 +166,28 @@ public void shouldConvertTestFailureIgnoreWhenAbsent() throws Exception { @Test public void convertsArgline() throws Exception { - this.surefireConfig = makeConfig("-Xmx1024m"); + this.surefireConfig = makeConfig("-Xmx1024m -Dfoo=${BAR} -Dfoo=$@BAR}"); ReportOptions actual = this.testee .update(this.options, this.surefireConfig); - assertThat(actual.getArgLine()).isEqualTo("-Xmx1024m"); + assertThat(actual.getArgLine()).isEqualTo("-Xmx1024m -Dfoo=${BAR} -Dfoo=$@BAR}"); + } + + @Test + public void appendsToExistingArgLine() throws Exception { + this.surefireConfig = makeConfig("-Xmx1024m -Dfoo=${BAR} -Dfoo=$@BAR}"); + this.options.setArgLine("alreadyHere"); + + ReportOptions actual = this.testee + .update(this.options, this.surefireConfig); + + assertThat(actual.getArgLine()).isEqualTo("alreadyHere -Xmx1024m -Dfoo=${BAR} -Dfoo=$@BAR}"); } @Test - public void doesNotConvertArglineWhenAlreadySetInPitestConfig() throws Exception { + public void doesNotConvertArglineWhenFlagNotSet() throws Exception { + this.testee = new SurefireConfigConverter(false); this.surefireConfig = makeConfig("-Xmx1024m"); this.options.setArgLine("-foo");