[GitHub] maven-surefire issue #164: SUREFIRE-1383: Split IT569 in to multiple lifecyc...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/164 @Tibor17 - Is this what you were thinking of when you asked to clean up the distinct change sets related to #157? --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 @Tibor17 - any updates? --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 @Tibor17 I see what you're going for, that makes sense. I've update my PR to just include IT contents now, with an improved IT that emulates a scenario where working copy changes are made (i.e. install a multi-module project, modify the contents to add more tests, test the modified contents). If your solution gets my IT PR to pass, I think the solution will cover my bona fide scenarios. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 I don't think that logic would work for my scenario. For example: 1. Create multi-module project with a test runner JAR (a la `org.apache.maven.surefire:surefire-1383:1.0-SNAPSHOT`) 2. Recursively run the `install` goal over the multi-module project 3. Modify the test runner module (`org.apache.maven.surefire:surefire-1383-runner:1.0-SNAPSHOT`) 4. Run the `test` goal over the multi-module project According to your logic, Surefire would prefer the already-installed artifact, effectively ignoring those changes made in step 3. That would put us right back where we started. Using my logic, Surefire would prefer the working copy changes and execute accordingly. Since the existing `DependencyScanner` doesn't support classifiers as written, I don't think my approach is introducing any new limitations there. `DependencyScanner.java:114` ``` String[] groupArtifact = groups.split( ":" ); if ( groupArtifact.length != 2 ) { throw new IllegalArgumentException(...); } ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire pull request #164: SUREFIRE-1383: Split IT569 in to multiple ...
GitHub user owenfarrell opened a pull request: https://github.com/apache/maven-surefire/pull/164 SUREFIRE-1383: Split IT569 in to multiple lifecycles @Tibor17 - Based on [your comment](https://github.com/apache/maven-surefire/pull/157#issuecomment-325616922), is this in line with your expectations? Refactored test passes based on contents of master. You can merge this pull request into a Git repository by running: $ git pull https://github.com/owenfarrell/maven-surefire IT569 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/maven-surefire/pull/164.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 #164 commit 2892ec9778d05f7ecdafe87f0a9714981bdc1237 Author: Owen Farrell <owen.farr...@gmail.com> Date: 2017-08-29T12:54:25Z SUREFIRE-1383: Split IT569 in to multiple lifecycles --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire pull request #157: SUREFIRE-1383 dependenciesToScan Does Not ...
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 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 dependenciesToScan = -DependencyScanner.filter( project.getTestArtifacts(), Arrays.asList( getDependenciesToScan() ) ); -DependencyScanner scanner = new DependencyScanner( dependenciesToScan, getIncludedAndExcludedTests() ); -return scanner.scan(); +List 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 @Tibor17 - IT569 does not **fail** when introducing my changes. But since the test was written as a single lifecycle, it inadvertently uses the code I've introduced in this PR ([see comment above](#issuecomment-312972256)). The only changes to IT569 were to split the single lifecycle in to multiple lifecycles. While there, I tried to clean up the test resources to match current standards/conventions. But those changes to IT569 were to preserve its named intent: run tests from a dependency jar. Looks like two options to me: 1. Leave PR as is with minimally modified IT569 2. Revert IT569 to original state, add a new version of IT569 which executes in multiple lifecycles and delete IT1383 (as it would be entirely redundant of IT569). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 @Tibor17 - thoughts? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 @Tibor17 - Have you had a chance to review this PR? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 And I got busy with other work as well. I think I found a solution that doesn't completely violate the pattern used by integration tests. Long story short, the IT for SUREFIRE-569 has been updated to be completely independent modules. The first module leverages the flatten plugin to avoid any parent references, which then allows the second module to complete successfully. The IT for SUREFIRE-1383 executes within a single process and verifies the updates that I've proposed. Sorry for all the noise on the ticket - I hosed my fork while making the last round of updates. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire pull request #157: SUREFIRE-1383 dependenciesToScan Does Not ...
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 <owen.farr...@gmail.com> 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire pull request #157: SUREFIRE-1383 dependenciesToScan Does Not ...
Github user owenfarrell closed the pull request at: https://github.com/apache/maven-surefire/pull/157 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 Copying the `it-parent` POM over to the output directory as part of [MavenLauncher.simpleExtractResources](https://github.com/apache/maven-surefire/blob/master/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/fixture/MavenLauncher.java#L448) doesn't support resolving `it-parent` as part of a dependency. Would it make sense to open a separate issue/enhancement for installing the `it-parent` artifact in the IT repo? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 I don't follow. When I run an install from any level, the `org.apache.maven.surefire:it-parent:1.0:pom` artifact (surefire-integration-tests/src/test/resources/pom.xml) does not get installed in to the IT repo (surefire-setup-integration-tests/target/it-repo). Am I missing something? In order to execute an IT across multiple lifecycles, that POM needs to be installed it in to the local repository specified by failsafe. It looks to me like every IT that references `it-parent` does so via a relative path (explicitly or by default). They never have to rely on the artifact resolver. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 One workaround for this would be to install the missing `it-parent` POM as part of my first lifecycle (i.e. mid-test). But this seems kind of hacky. ``` surefire-569-testjar ... org.apache.maven.plugins maven-install-plugin install-file install ${project.basedir}/../pom.xml ${project.basedir}/../pom.xml ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 Looking at it a bit deeper, looks like the root cause is that the `it-parent` POM is not installed in the IT repo. The issue isn't the dependency itself. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 I've put together an integration test that can validate the execution of [SUREFIRE-1383](https://issues.apache.org/jira/browse/SUREFIRE-1383). And all is good on that front. While I was poking around the other ITs, I tripped on the IT for [SUREFIRE-569](https://issues.apache.org/jira/browse/SUREFIRE-569). As written, the IT passes just fine. But the changes that I'm proposing make that test case moot - it uses the same packaging structure of 2 projects built within a single session. I updated that test case to better reflect the original ask of scanning **_JAR_** dependencies, but that means splitting the test case execution in to 2 separate lifecycles. When I split the single lifecycle in to 2, I'm running in to a dependency resolution issue. For whatever reason, the second execution is unable to locate artifacts installed in the IT repo as part of the first lifecycle. In looking around at the other ITs, it looks like this is the only test case that runs an install goal and **_is then dependent on the result of that install_**. Thoughts? ``` mvn -Dit.test=**/Surefire569RunTestFromDependencyJarsIT.java,**/Surefire1383ScanSessionDependenciesIT.java clean integration-test` ... [DEBUG] Using local repository at //maven-surefire/surefire-integration-tests/../surefire-setup-integration-tests/target/it-repo ... [ERROR] Failed to execute goal on project surefire-569-module1: Could not resolve dependencies for project org.apache.maven.surefire:surefire-569-module1:jar:1.0: Failed to collect dependencies at org.apache.maven.surefire:surefire-569-testjar:jar:tests:1.0: Failed to read artifact descriptor for org.apache.maven.surefire:surefire-569-testjar:jar:tests:1.0: Failure to find org.apache.maven.surefire:it-parent:pom:1.0 in https://repo.maven.apache.org/maven2 was cached in the local repository, resolution will not be reattempted until the update interval of central has elapsed or updates are forced -> [Help 1] org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal on project surefire-569-module1: Could not resolve dependencies for project org.apache.maven.surefire:surefire-569-module1:jar:1.0: Failed to collect dependencies at org.apache.maven.surefire:surefire-569-testjar:jar:tests:1.0 ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire issue #157: SUREFIRE-1383 dependenciesToScan Does Not Leverag...
Github user owenfarrell commented on the issue: https://github.com/apache/maven-surefire/pull/157 @Tibor17 - done. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven-surefire pull request #157: SUREFIRE-1383 dependenciesToScan Does Not ...
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 <owen.farr...@gmail.com> 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 <owen.farr...@gmail.com> 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org