You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Hervé BOUTEMY <he...@free.fr> on 2017/02/18 11:25:05 UTC

Re: maven-surefire git commit: SUREFIRE_SYSPROP_DUPLICATES

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 tibordigana@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 <ti...@lycos.com>
> Authored: Thu Feb 16 18:40:45 2017 +0100
> Committer: Tibor17 <ti...@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:tibordigana@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-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: maven-surefire git commit: SUREFIRE_SYSPROP_DUPLICATES

Posted by Tibor Digana <ti...@apache.org>.
yes, Jira will be created for these branches, no problem, but let's see
what the future logs bring.

On Sat, Feb 18, 2017 at 12:51 PM, Tibor Digana-2 [via Maven] <
ml-node+s40175n5899063h93@n5.nabble.com> wrote:

> The build processes use to take from 45min to 2:21h.
>
> On Sat, Feb 18, 2017 at 12:47 PM, Tibor Digana <[hidden email]
> <http:///user/SendEmail.jtp?type=node&node=5899063&i=0>>
> wrote:
>
> > 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 <[hidden email]
> <http:///user/SendEmail.jtp?type=node&node=5899063&i=1>>
> > 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 [hidden email]
> <http:///user/SendEmail.jtp?type=node&node=5899063&i=2> 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 <[hidden email]
> <http:///user/SendEmail.jtp?type=node&node=5899063&i=3>>
> >> > Authored: Thu Feb 16 18:40:45 2017 +0100
> >> > Committer: Tibor17 <[hidden email]
> <http:///user/SendEmail.jtp?type=node&node=5899063&i=4>>
> >> > 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/e
> >> f5b0f46/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/e
> >> f5b0f46/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:[hidden email]
> <http:///user/SendEmail.jtp?type=node&node=5899063&i=5>">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/e
> >> f5b0f46/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: [hidden email]
> <http:///user/SendEmail.jtp?type=node&node=5899063&i=6>
> >> For additional commands, e-mail: [hidden email]
> <http:///user/SendEmail.jtp?type=node&node=5899063&i=7>
> >>
> >>
> >
> >
> > --
> > Cheers
> > Tibor
> >
>
>
>
> --
> Cheers
> Tibor
>
>
> ------------------------------
> If you reply to this email, your message will be added to the discussion
> below:
> http://maven.40175.n5.nabble.com/Re-maven-surefire-git-
> commit-SUREFIRE-SYSPROP-DUPLICATES-tp5899041p5899063.html
> To start a new topic under Maven Developers, email
> ml-node+s40175n142166h86@n5.nabble.com
> To unsubscribe from Maven Developers, click here
> <http://maven.40175.n5.nabble.com/template/NamlServlet.jtp?macro=unsubscribe_by_code&node=142166&code=dGlib3JkaWdhbmFAYXBhY2hlLm9yZ3wxNDIxNjZ8LTI4OTQ5MjEwMg==>
> .
> NAML
> <http://maven.40175.n5.nabble.com/template/NamlServlet.jtp?macro=macro_viewer&id=instant_html%21nabble%3Aemail.naml&base=nabble.naml.namespaces.BasicNamespace-nabble.view.web.template.NabbleNamespace-nabble.view.web.template.NodeNamespace&breadcrumbs=notify_subscribers%21nabble%3Aemail.naml-instant_emails%21nabble%3Aemail.naml-send_instant_email%21nabble%3Aemail.naml>
>




--
View this message in context: http://maven.40175.n5.nabble.com/Re-maven-surefire-git-commit-SUREFIRE-SYSPROP-DUPLICATES-tp5899041p5899071.html
Sent from the Maven Developers mailing list archive at Nabble.com.

Re: maven-surefire git commit: SUREFIRE_SYSPROP_DUPLICATES

Posted by Tibor Digana <ti...@googlemail.com>.
The build processes use to take from 45min to 2:21h.

On Sat, Feb 18, 2017 at 12:47 PM, Tibor Digana <ti...@googlemail.com>
wrote:

> 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 <he...@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 tibordigana@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 <ti...@lycos.com>
>> > Authored: Thu Feb 16 18:40:45 2017 +0100
>> > Committer: Tibor17 <ti...@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/e
>> f5b0f46/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/e
>> f5b0f46/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:tibordigana@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/e
>> f5b0f46/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-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>>
>
>
> --
> Cheers
> Tibor
>



-- 
Cheers
Tibor

Re: maven-surefire git commit: SUREFIRE_SYSPROP_DUPLICATES

Posted by Tibor Digana <ti...@googlemail.com>.
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 <he...@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 tibordigana@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 <ti...@lycos.com>
> > Authored: Thu Feb 16 18:40:45 2017 +0100
> > Committer: Tibor17 <ti...@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:tibordigana@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-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>


-- 
Cheers
Tibor