I am waiting for Michael's "go on". There are 58 TestNG ITs and this is not nice fix, however it is god but the rootcause is that surefire-integration-tests POM has testng default version 5.7 and that is the roortcause of system prop duplicates. I need to find someone who will update 58 tests after we and Michael says that the build is passed. Let's see what Stephan's build on Mac says. Fixing 58 test can be done later, does not block Maven.
On Sat, Feb 18, 2017 at 12:25 PM, Hervé BOUTEMY <herve.bout...@free.fr> wrote: > IIUC, this one is a good enhancement to integrate, since it makes Surefire > more > reliable (not relying on the way multiple "-Dmyprop=" is handled) > > Then there should just be a Jira issue created, and this fix integrated to > Surefire master without waiting, isn't it? > > Or do you fear that this change can have unexpected impact? > > Regards, > > Hervé > > Le jeudi 16 février 2017, 17:42:07 CET tibordig...@apache.org a écrit : > > Repository: maven-surefire > > Updated Branches: > > refs/heads/SUREFIRE_SYSPROP_DUPLICATES [created] ef5b0f460 > > > > > > SUREFIRE_SYSPROP_DUPLICATES > > > > > > Project: http://git-wip-us.apache.org/repos/asf/maven-surefire/repo > > Commit: > > http://git-wip-us.apache.org/repos/asf/maven-surefire/commit/ef5b0f46 > Tree: > > http://git-wip-us.apache.org/repos/asf/maven-surefire/tree/ef5b0f46 > Diff: > > http://git-wip-us.apache.org/repos/asf/maven-surefire/diff/ef5b0f46 > > > > Branch: refs/heads/SUREFIRE_SYSPROP_DUPLICATES > > Commit: ef5b0f460021ad6f827d75cde38f888f37a54415 > > Parents: 179abbf > > Author: Tibor17 <tibo...@lycos.com> > > Authored: Thu Feb 16 18:40:45 2017 +0100 > > Committer: Tibor17 <tibo...@lycos.com> > > Committed: Thu Feb 16 18:40:45 2017 +0100 > > > > ---------------------------------------------------------------------- > > .../surefire/its/fixture/MavenLauncher.java | 45 > ++++++++++++++++--- > > .../surefire/its/fixture/MavenLauncherTest.java | 47 > ++++++++++++++++++++ > > .../surefire/its/fixture/SurefireLauncher.java | 6 +-- > > 3 files changed, 88 insertions(+), 10 deletions(-) > > ---------------------------------------------------------------------- > > > > > > http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/ > ef5b0f46/surefire > > -integration-tests/src/test/java/org/apache/maven/ > surefire/its/fixture/Maven > > Launcher.java > > ---------------------------------------------------------------------- > diff > > --git > > a/surefire-integration-tests/src/test/java/org/apache/ > maven/surefire/its/fi > > xture/MavenLauncher.java > > b/surefire-integration-tests/src/test/java/org/apache/ > maven/surefire/its/fi > > xture/MavenLauncher.java index 1198fcb..0945068 100755 > > --- > > a/surefire-integration-tests/src/test/java/org/apache/ > maven/surefire/its/fi > > xture/MavenLauncher.java +++ > > b/surefire-integration-tests/src/test/java/org/apache/ > maven/surefire/its/fi > > xture/MavenLauncher.java @@ -19,18 +19,22 @@ package > > org.apache.maven.surefire.its.fixture; * under the License. > > */ > > > > +import org.apache.commons.lang.text.StrSubstitutor; > > +import org.apache.maven.it.VerificationException; > > +import org.apache.maven.it.Verifier; > > +import org.apache.maven.it.util.ResourceExtractor; > > +import org.apache.maven.shared.utils.io.FileUtils; > > + > > import java.io.File; > > import java.io.IOException; > > import java.net.URL; > > import java.util.ArrayList; > > import java.util.HashMap; > > import java.util.List; > > +import java.util.ListIterator; > > import java.util.Map; > > -import org.apache.commons.lang.text.StrSubstitutor; > > -import org.apache.maven.it.VerificationException; > > -import org.apache.maven.it.Verifier; > > -import org.apache.maven.it.util.ResourceExtractor; > > -import org.apache.maven.shared.utils.io.FileUtils; > > + > > +import static java.util.Collections.unmodifiableList; > > > > /** > > * Encapsulate all needed features to start a maven run > > @@ -203,13 +207,13 @@ public class MavenLauncher > > > > public MavenLauncher skipClean() > > { > > - goals.add( "-Dclean.skip=true" ); > > + writeGoal( "-Dclean.skip=true" ); > > return this; > > } > > > > public MavenLauncher addGoal( String goal ) > > { > > - goals.add( goal ); > > + writeGoal( goal ); > > return this; > > } > > > > @@ -223,6 +227,33 @@ public class MavenLauncher > > return conditionalExec( "test" ); > > } > > > > + List<String> getGoals() > > + { > > + return unmodifiableList( goals ); > > + } > > + > > + private void writeGoal( String newGoal ) > > + { > > + if ( newGoal != null && newGoal.startsWith( "-D" ) ) > > + { > > + final String sysPropKey = > > + newGoal.contains( "=" ) ? newGoal.substring( 0, > > newGoal.indexOf( '=' ) ) : newGoal; + > > + final String sysPropStarter = sysPropKey + "="; > > + > > + for ( ListIterator<String> it = goals.listIterator(); > > it.hasNext(); ) + { > > + String goal = it.next(); > > + if ( goal.equals( sysPropKey ) || goal.startsWith( > > sysPropStarter ) ) + { > > + it.set( newGoal ); > > + return; > > + } > > + } > > + } > > + goals.add( newGoal ); > > + } > > + > > private OutputValidator conditionalExec(String goal) > > { > > OutputValidator verify; > > > > http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/ > ef5b0f46/surefire > > -integration-tests/src/test/java/org/apache/maven/ > surefire/its/fixture/Maven > > LauncherTest.java > > ---------------------------------------------------------------------- > diff > > --git > > a/surefire-integration-tests/src/test/java/org/apache/ > maven/surefire/its/fi > > xture/MavenLauncherTest.java > > b/surefire-integration-tests/src/test/java/org/apache/ > maven/surefire/its/fi > > xture/MavenLauncherTest.java new file mode 100644 > > index 0000000..4a638b6 > > --- /dev/null > > +++ > > b/surefire-integration-tests/src/test/java/org/apache/ > maven/surefire/its/fi > > xture/MavenLauncherTest.java @@ -0,0 +1,47 @@ > > +package org.apache.maven.surefire.its.fixture; > > + > > +/* > > + * Licensed to the Apache Software Foundation (ASF) under one > > + * or more contributor license agreements. See the NOTICE file > > + * distributed with this work for additional information > > + * regarding copyright ownership. The ASF licenses this file > > + * to you under the Apache License, Version 2.0 (the > > + * "License"); you may not use this file except in compliance > > + * with the License. You may obtain a copy of the License at > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in writing, > > + * software distributed under the License is distributed on an > > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > > + * KIND, either express or implied. See the License for the > > + * specific language governing permissions and limitations > > + * under the License. > > + */ > > + > > +import org.junit.Test; > > + > > +import static org.hamcrest.CoreMatchers.is; > > +import static org.junit.Assert.assertThat; > > +import static org.hamcrest.CoreMatchers.hasItems; > > + > > +/** > > + * @author <a href="mailto:tibordig...@apache.org";>Tibor Digana > > (tibor17)</a> + * @since 2.19.2 > > + */ > > +public class MavenLauncherTest > > +{ > > + @Test > > + public void shouldNotDuplicateSystemProperties() > > + { > > + MavenLauncher launcher = new MavenLauncher( getClass(), "", "" ) > > + .addGoal( "-DskipTests" ) > > + .addGoal( "-Dx=a" ) > > + .addGoal( "-DskipTests" ) > > + .addGoal( "-Dx=b" ); > > + > > + assertThat( launcher.getGoals(), hasItems( "-Dx=b", > "-DskipTests" ) > > ); + > > + assertThat( launcher.getGoals().size(), is( 2 ) ); > > + } > > +} > > > > http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/ > ef5b0f46/surefire > > -integration-tests/src/test/java/org/apache/maven/ > surefire/its/fixture/Suref > > ireLauncher.java > > ---------------------------------------------------------------------- > diff > > --git > > a/surefire-integration-tests/src/test/java/org/apache/ > maven/surefire/its/fi > > xture/SurefireLauncher.java > > b/surefire-integration-tests/src/test/java/org/apache/ > maven/surefire/its/fi > > xture/SurefireLauncher.java index 23a09b0..1c78680 100755 > > --- > > a/surefire-integration-tests/src/test/java/org/apache/ > maven/surefire/its/fi > > xture/SurefireLauncher.java +++ > > b/surefire-integration-tests/src/test/java/org/apache/ > maven/surefire/its/fi > > xture/SurefireLauncher.java @@ -42,7 +42,7 @@ public class > SurefireLauncher > > > > private final MavenLauncher mavenLauncher; > > > > - private final String testNgVersion = System.getProperty( > > "testng.version" ); + private final String testNgVersion = > > System.getProperty( "testng.version" );//todo > > > > private final String surefireVersion = System.getProperty( > > "surefire.version" ); > > > > @@ -129,14 +129,14 @@ public class SurefireLauncher > > > > if ( this.testNgVersion != null ) > > { > > - goals1.add( "-DtestNgVersion=" + testNgVersion ); > > + goals1.add( "-DtestNgVersion=" + testNgVersion );//todo > > > > ArtifactVersion v = new DefaultArtifactVersion( > testNgVersion > > ); try > > { > > if ( VersionRange.createFromVersionSpec( "(,5.12.1)" > > ).containsVersion( v ) ) { > > - goals1.add( "-DtestNgClassifier=jdk15" ); > > + goals1.add( "-DtestNgClassifier=jdk15" );//todo > > } > > } > > catch ( InvalidVersionSpecificationException e ) > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > For additional commands, e-mail: dev-h...@maven.apache.org > > -- Cheers Tibor
