[GitHub] maven-surefire pull request #157: SUREFIRE-1383 dependenciesToScan Does Not ...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] maven-surefire pull request #157: SUREFIRE-1383 dependenciesToScan Does Not ...

hboutemy-2
GitHub user owenfarrell opened a pull request:

    https://github.com/apache/maven-surefire/pull/157

    SUREFIRE-1383 dependenciesToScan Does Not Leverage Classpath Elements

    Added logic to prefer output directories of projects built as part of the current session.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/owenfarrell/maven-surefire master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/maven-surefire/pull/157.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #157
   
----
commit 12bda800118b25397adbe15a3b21547aa1df2aeb
Author: Owen Farrell <[hidden email]>
Date:   2017-06-29T21:12:34Z

    SUREFIRE-1383: Added logic to use build output directories for session projects in favor of installed artifacts

commit 524bb05d2aad37ef90458408607085456e1f774a
Author: Owen Farrell <[hidden email]>
Date:   2017-06-29T21:14:38Z

    SUREFIRE-1383: Added logic to use build output directories for session projects in favor of installed artifacts

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven-surefire pull request #157: SUREFIRE-1383 dependenciesToScan Does Not ...

hboutemy-2
Github user Tibor17 commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/157#discussion_r125169703
 
    --- Diff: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java ---
    @@ -847,12 +847,33 @@ private DefaultScanResult scanDependencies()
             {
                 try
                 {
    +                DefaultScanResult scanResult = new DefaultScanResult( Collections.EMPTY_LIST );
    +                
    +                List<String> dependenciesToScanList = new ArrayList( Arrays.asList( getDependenciesToScan() ) );
    +                TestListResolver includedAndExcludedTests = getIncludedAndExcludedTests();
    +
    +                for ( MavenProject mavenProject : session.getSortedProjects() )
    +                {
    +                    String groupArtifactId = new StringBuilder( mavenProject.getGroupId() ).append( ":" )
    +                            .append( mavenProject.getArtifactId() ).toString();
    +                    if ( dependenciesToScanList.removeAll( Collections.singleton( groupArtifactId ) ) )
    +                    {
    +                        File outputDirectoryFile = new File( mavenProject.getBuild().getOutputDirectory() );
    +                        DirectoryScanner scanner =
    +                                new DirectoryScanner( outputDirectoryFile, includedAndExcludedTests );
    +                        scanResult = scanResult.append( scanner.scan() );
    +                    }
    +                    
    +                }
    +
                     // @TODO noinspection unchecked, check MavenProject 3.x for Generics in surefire:3.0
                     @SuppressWarnings( "unchecked" )
    -                List<File> dependenciesToScan =
    -                    DependencyScanner.filter( project.getTestArtifacts(), Arrays.asList( getDependenciesToScan() ) );
    -                DependencyScanner scanner = new DependencyScanner( dependenciesToScan, getIncludedAndExcludedTests() );
    -                return scanner.scan();
    +                List<File> dependenciesToScanFileList =
    --- End diff --
   
    `dependenciesToScanFileList` already contains plural. Pls rename `dependenciesToScanFileList` to `dependenciesToScanFile`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...

hboutemy-2
In reply to this post by hboutemy-2
Github user Tibor17 commented on the issue:

    https://github.com/apache/maven-surefire/pull/157
 
    Would you add integration test?
    See the module `surefire-integration-tests` and `src/test/java/org/apache/maven/surefire/its/jiras`.
    It's easy. Pickup some existing test, e.g. `Surefire34SecurityManagerIT`, and just inherit `SurefireJUnit4IntegrationTestCase`and see the folder `surefire-34-securityManager` in `test/resources`.
    Your IT will have the same principle. The point is to start `surefire-34-securityManager/pom.xml` as embedded or forked maven process from `Surefire34SecurityManagerIT`. I guess you want to test that multi-module Maven project will have own classes propagated from child modules one one dedicated module where Surefire has tests.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven-surefire pull request #157: SUREFIRE-1383 dependenciesToScan Does Not ...

hboutemy-2
In reply to this post by hboutemy-2
GitHub user owenfarrell reopened a pull request:

    https://github.com/apache/maven-surefire/pull/157

    SUREFIRE-1383 dependenciesToScan Does Not Leverage Classpath Elements

    Added logic to prefer output directories of projects built as part of the current session.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/owenfarrell/maven-surefire master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/maven-surefire/pull/157.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #157
   
----
commit ee3b5be037c70d2a4667d786d4c487cf89ebfdfa
Author: Owen Farrell <[hidden email]>
Date:   2017-07-26T02:30:52Z

    SUREFIRE-1383: Added logic to use build output directories for session projects in favor of installed artifacts

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...

hboutemy-2
In reply to this post by hboutemy-2
Github user Tibor17 commented on the issue:

    https://github.com/apache/maven-surefire/pull/157
 
    @JoelNGeorge
    @owenfarrell
    Pls let me run the build and next steps.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven-surefire pull request #157: SUREFIRE-1383 dependenciesToScan Does Not ...

hboutemy-2
In reply to this post by hboutemy-2
Github user Tibor17 commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/157#discussion_r135765515
 
    --- Diff: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java ---
    @@ -847,12 +847,31 @@ private DefaultScanResult scanDependencies()
             {
                 try
                 {
    +                DefaultScanResult scanResult = new DefaultScanResult( Collections.EMPTY_LIST );
    +                
    +                List<String> dependenciesToScan = new ArrayList();
    +                Collections.addAll( dependenciesToScan, getDependenciesToScan() );
    +                TestListResolver includedAndExcludedTests = getIncludedAndExcludedTests();
    +
    +                for ( MavenProject mavenProject : session.getSortedProjects() )
    +                {
    +                    String groupArtifactId = mavenProject.getGroupId() + ":" + mavenProject.getArtifactId();
    +                    if ( dependenciesToScan.removeAll( Collections.singleton( groupArtifactId ) ) )
    +                    {
    +                        File outputDirectoryFile = new File( mavenProject.getBuild().getOutputDirectory() );
    +                        DirectoryScanner scanner =
    +                                new DirectoryScanner( outputDirectoryFile, includedAndExcludedTests );
    +                        scanResult = scanResult.append( scanner.scan() );
    +                    }
    +                }
    +
                     // @TODO noinspection unchecked, check MavenProject 3.x for Generics in surefire:3.0
                     @SuppressWarnings( "unchecked" )
    -                List<File> dependenciesToScan =
    -                    DependencyScanner.filter( project.getTestArtifacts(), Arrays.asList( getDependenciesToScan() ) );
    -                DependencyScanner scanner = new DependencyScanner( dependenciesToScan, getIncludedAndExcludedTests() );
    -                return scanner.scan();
    +                List<File> dependenciesToScanFile =
    --- End diff --
   
    Pls align to single line after using static import:
    `import static java.lang.Thread.currentThread;
    import static org.apache.maven.plugin.surefire.util.DependencyScanner.filter;
    import static java.util.Collections.singletonMap;`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven-surefire pull request #157: SUREFIRE-1383 dependenciesToScan Does Not ...

hboutemy-2
In reply to this post by hboutemy-2
Github user Tibor17 commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/157#discussion_r135766258
 
    --- Diff: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java ---
    @@ -847,12 +847,31 @@ private DefaultScanResult scanDependencies()
             {
                 try
                 {
    +                DefaultScanResult scanResult = new DefaultScanResult( Collections.EMPTY_LIST );
    +                
    +                List<String> dependenciesToScan = new ArrayList();
    +                Collections.addAll( dependenciesToScan, getDependenciesToScan() );
    +                TestListResolver includedAndExcludedTests = getIncludedAndExcludedTests();
    +
    +                for ( MavenProject mavenProject : session.getSortedProjects() )
    +                {
    +                    String groupArtifactId = mavenProject.getGroupId() + ":" + mavenProject.getArtifactId();
    +                    if ( dependenciesToScan.removeAll( Collections.singleton( groupArtifactId ) ) )
    +                    {
    +                        File outputDirectoryFile = new File( mavenProject.getBuild().getOutputDirectory() );
    +                        DirectoryScanner scanner =
    +                                new DirectoryScanner( outputDirectoryFile, includedAndExcludedTests );
    +                        scanResult = scanResult.append( scanner.scan() );
    +                    }
    +                }
    +
                     // @TODO noinspection unchecked, check MavenProject 3.x for Generics in surefire:3.0
                     @SuppressWarnings( "unchecked" )
    -                List<File> dependenciesToScan =
    -                    DependencyScanner.filter( project.getTestArtifacts(), Arrays.asList( getDependenciesToScan() ) );
    -                DependencyScanner scanner = new DependencyScanner( dependenciesToScan, getIncludedAndExcludedTests() );
    -                return scanner.scan();
    +                List<File> dependenciesToScanFile =
    +                        DependencyScanner.filter( project.getTestArtifacts(), dependenciesToScan );
    --- End diff --
   
    Here some artifacts may have classifier `tests`. This means your loop may appear below test artifacts and only those project artifacts (gid:aid:*) should be excluded which appear in `getTestArtifacts()` and scans appended creating one aggregated scan result.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven-surefire pull request #157: SUREFIRE-1383 dependenciesToScan Does Not ...

hboutemy-2
In reply to this post by hboutemy-2
Github user owenfarrell commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/157#discussion_r135778722
 
    --- Diff: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java ---
    @@ -847,12 +847,31 @@ private DefaultScanResult scanDependencies()
             {
                 try
                 {
    +                DefaultScanResult scanResult = new DefaultScanResult( Collections.EMPTY_LIST );
    +                
    +                List<String> dependenciesToScan = new ArrayList();
    +                Collections.addAll( dependenciesToScan, getDependenciesToScan() );
    +                TestListResolver includedAndExcludedTests = getIncludedAndExcludedTests();
    +
    +                for ( MavenProject mavenProject : session.getSortedProjects() )
    +                {
    +                    String groupArtifactId = mavenProject.getGroupId() + ":" + mavenProject.getArtifactId();
    +                    if ( dependenciesToScan.removeAll( Collections.singleton( groupArtifactId ) ) )
    +                    {
    +                        File outputDirectoryFile = new File( mavenProject.getBuild().getOutputDirectory() );
    +                        DirectoryScanner scanner =
    +                                new DirectoryScanner( outputDirectoryFile, includedAndExcludedTests );
    +                        scanResult = scanResult.append( scanner.scan() );
    +                    }
    +                }
    +
                     // @TODO noinspection unchecked, check MavenProject 3.x for Generics in surefire:3.0
                     @SuppressWarnings( "unchecked" )
    -                List<File> dependenciesToScan =
    -                    DependencyScanner.filter( project.getTestArtifacts(), Arrays.asList( getDependenciesToScan() ) );
    -                DependencyScanner scanner = new DependencyScanner( dependenciesToScan, getIncludedAndExcludedTests() );
    -                return scanner.scan();
    +                List<File> dependenciesToScanFile =
    +                        DependencyScanner.filter( project.getTestArtifacts(), dependenciesToScan );
    --- End diff --
   
    Locally staged updates to reflect your generics comments. But I don't follow what you're asking for here.
   
    [Per the documentation](https://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#dependenciesToScan), the dependenciesToScan property doesn't support classifiers - just group ID and artifact ID. So I didn't add any logic around classifier resolution. Is that what you were expecting?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...

hboutemy-2
In reply to this post by hboutemy-2
Github user Tibor17 commented on the issue:

    https://github.com/apache/maven-surefire/pull/157
 
    @owenfarrell
    I will try by myself. I will let you know with new branch and we can discuss it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...

hboutemy-2
In reply to this post by hboutemy-2
Github user Tibor17 commented on the issue:

    https://github.com/apache/maven-surefire/pull/157
 
    @owenfarrell
    Yeah, I have released 2.20.1 and I had to postpone this issue for 2.21.1.
    OOM was fixed in 2.20.1 and users could not wait longer. This included Java9 support.
    Java 9 is out and I want to release Surefire 2.21.0.Jigsaw in next few days which has higher priority.
    And then Surefire 2.21.1 is planed for your and other pull request fixed as well.
    Then 2.21.2 will fix a blocker issue, and then we will concentrate on 3.0.0.M1 and JUnit5.
    So this is my plan.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]