You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/02/12 15:44:31 UTC
[GitHub] [maven-integration-testing] gnodet opened a new pull request #98: [MNG-7095] Support for parallel builds when resuming
gnodet opened a new pull request #98:
URL: https://github.com/apache/maven-integration-testing/pull/98
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-integration-testing] MartinKanters commented on pull request #98: [MNG-7095] Support for parallel builds when resuming
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on pull request #98:
URL: https://github.com/apache/maven-integration-testing/pull/98#issuecomment-778413713
Also I don't mind it too much, but perhaps we could avoid `install` now, if MNG-4660 indeed has fixed the issue you described in MNG-7099.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-integration-testing] gnodet commented on pull request #98: [MNG-7095] Support for parallel builds when resuming
Posted by GitBox <gi...@apache.org>.
gnodet commented on pull request #98:
URL: https://github.com/apache/maven-integration-testing/pull/98#issuecomment-778426174
I've fixed the comments and change the tests to rely on the `verify` goal rather than `install`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-integration-testing] MartinKanters commented on a change in pull request #98: [MNG-7095] Support for parallel builds when resuming
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on a change in pull request #98:
URL: https://github.com/apache/maven-integration-testing/pull/98#discussion_r575473333
##########
File path: core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5760ResumeFeatureTest.java
##########
@@ -175,6 +179,112 @@ public void testShouldNotCrashWithoutProject() throws Exception
}
}
+ public void testFailureWithParallelBuild() throws Exception
+ {
+ Verifier verifier = newVerifier( fourModulesTestDir.getAbsolutePath() );
+ verifier.addCliOption( "-T2" );
+ verifier.addCliOption( "-Dmodule-a.delay=1000" );
+ verifier.addCliOption( "-Dmodule-a.fail=true" );
+ verifier.addCliOption( "-Dmodule-c.fail=true" );
+
+ verifier.executeGoal( "org.apache.maven.plugins:maven-clean-plugin:clean" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-a", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-b", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-c", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-d", "1.0", "jar" );
+
+ try
+ {
+ verifier.executeGoal( "install" );
+ fail( "Expected this invocation to fail" );
+ }
+ catch ( final VerificationException ve )
+ {
+ // Expected to fail.
+ verifier.assertArtifactNotPresent( "org.apache.maven.its.mng5760", "module-a", "1.0", "jar" );
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-b", "1.0", "jar" );
+ verifier.assertArtifactNotPresent( "org.apache.maven.its.mng5760", "module-c", "1.0", "jar" );
+ verifier.assertArtifactNotPresent( "org.apache.maven.its.mng5760", "module-d", "1.0", "jar" );
+ }
+ finally
+ {
+ verifier.resetStreams();
+ }
+
+ // Let module-b and module-c fail, if they would have been built...
Review comment:
You are only letting module-b fail. I think the comment is not correct, right? (if so, please also correct the one below)
##########
File path: core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5760ResumeFeatureTest.java
##########
@@ -175,6 +179,112 @@ public void testShouldNotCrashWithoutProject() throws Exception
}
}
+ public void testFailureWithParallelBuild() throws Exception
+ {
+ Verifier verifier = newVerifier( fourModulesTestDir.getAbsolutePath() );
+ verifier.addCliOption( "-T2" );
+ verifier.addCliOption( "-Dmodule-a.delay=1000" );
+ verifier.addCliOption( "-Dmodule-a.fail=true" );
+ verifier.addCliOption( "-Dmodule-c.fail=true" );
+
+ verifier.executeGoal( "org.apache.maven.plugins:maven-clean-plugin:clean" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-a", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-b", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-c", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-d", "1.0", "jar" );
+
+ try
+ {
+ verifier.executeGoal( "install" );
+ fail( "Expected this invocation to fail" );
+ }
+ catch ( final VerificationException ve )
+ {
+ // Expected to fail.
+ verifier.assertArtifactNotPresent( "org.apache.maven.its.mng5760", "module-a", "1.0", "jar" );
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-b", "1.0", "jar" );
+ verifier.assertArtifactNotPresent( "org.apache.maven.its.mng5760", "module-c", "1.0", "jar" );
+ verifier.assertArtifactNotPresent( "org.apache.maven.its.mng5760", "module-d", "1.0", "jar" );
+ }
+ finally
+ {
+ verifier.resetStreams();
+ }
+
+ // Let module-b and module-c fail, if they would have been built...
+ verifier = newVerifier( fourModulesTestDir.getAbsolutePath() );
+ verifier.addCliOption( "-T2" );
+ verifier.addCliOption( "-Dmodule-b.fail=true" );
+ // ... but adding -r should exclude those two from the build because the previous Maven invocation
+ // marked them as successfully built.
+ verifier.addCliOption( "-r" );
+ try
+ {
+ verifier.executeGoal( "install" );
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-a", "1.0", "jar" );
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-b", "1.0", "jar" );
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-c", "1.0", "jar" );
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-d", "1.0", "jar" );
+ }
+ finally
+ {
+ verifier.resetStreams();
+ }
+ }
+
+ public void testFailureAfterSkipWithParallelBuild() throws Exception
+ {
+ Verifier verifier = newVerifier( fourModulesTestDir.getAbsolutePath() );
+ verifier.addCliOption( "-T2" );
+ verifier.addCliOption( "-Dmodule-b.delay=2000" );
+ verifier.addCliOption( "-Dmodule-d.fail=true" );
+
+ verifier.executeGoal( "org.apache.maven.plugins:maven-clean-plugin:clean" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-a", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-b", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-c", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-d", "1.0", "jar" );
+
+ try
+ {
+ verifier.executeGoals( Arrays.asList( "org.apache.maven.plugins:maven-clean-plugin:clean", "install" ) );
Review comment:
Clean is not necessary here as you have cleaned it in line 242
##########
File path: core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5760ResumeFeatureTest.java
##########
@@ -175,6 +179,112 @@ public void testShouldNotCrashWithoutProject() throws Exception
}
}
+ public void testFailureWithParallelBuild() throws Exception
+ {
+ Verifier verifier = newVerifier( fourModulesTestDir.getAbsolutePath() );
+ verifier.addCliOption( "-T2" );
+ verifier.addCliOption( "-Dmodule-a.delay=1000" );
+ verifier.addCliOption( "-Dmodule-a.fail=true" );
+ verifier.addCliOption( "-Dmodule-c.fail=true" );
+
+ verifier.executeGoal( "org.apache.maven.plugins:maven-clean-plugin:clean" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-a", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-b", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-c", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-d", "1.0", "jar" );
+
+ try
+ {
+ verifier.executeGoal( "install" );
+ fail( "Expected this invocation to fail" );
+ }
+ catch ( final VerificationException ve )
+ {
+ // Expected to fail.
+ verifier.assertArtifactNotPresent( "org.apache.maven.its.mng5760", "module-a", "1.0", "jar" );
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-b", "1.0", "jar" );
+ verifier.assertArtifactNotPresent( "org.apache.maven.its.mng5760", "module-c", "1.0", "jar" );
+ verifier.assertArtifactNotPresent( "org.apache.maven.its.mng5760", "module-d", "1.0", "jar" );
+ }
+ finally
+ {
+ verifier.resetStreams();
+ }
+
+ // Let module-b and module-c fail, if they would have been built...
+ verifier = newVerifier( fourModulesTestDir.getAbsolutePath() );
+ verifier.addCliOption( "-T2" );
+ verifier.addCliOption( "-Dmodule-b.fail=true" );
+ // ... but adding -r should exclude those two from the build because the previous Maven invocation
+ // marked them as successfully built.
+ verifier.addCliOption( "-r" );
+ try
+ {
+ verifier.executeGoal( "install" );
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-a", "1.0", "jar" );
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-b", "1.0", "jar" );
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-c", "1.0", "jar" );
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-d", "1.0", "jar" );
+ }
+ finally
+ {
+ verifier.resetStreams();
+ }
+ }
+
+ public void testFailureAfterSkipWithParallelBuild() throws Exception
+ {
+ Verifier verifier = newVerifier( fourModulesTestDir.getAbsolutePath() );
+ verifier.addCliOption( "-T2" );
+ verifier.addCliOption( "-Dmodule-b.delay=2000" );
+ verifier.addCliOption( "-Dmodule-d.fail=true" );
+
+ verifier.executeGoal( "org.apache.maven.plugins:maven-clean-plugin:clean" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-a", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-b", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-c", "1.0", "jar" );
+ verifier.deleteArtifact( "org.apache.maven.its.mng5760", "module-d", "1.0", "jar" );
+
+ try
+ {
+ verifier.executeGoals( Arrays.asList( "org.apache.maven.plugins:maven-clean-plugin:clean", "install" ) );
+ fail( "Expected this invocation to fail" );
+ }
+ catch ( final VerificationException ve )
+ {
+ // Expected to fail.
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-a", "1.0", "jar" );
+ verifier.assertArtifactPresent( "org.apache.maven.its.mng5760", "module-b", "1.0", "jar" );
+ verifier.assertArtifactNotPresent( "org.apache.maven.its.mng5760", "module-c", "1.0", "jar" );
+ verifier.assertArtifactNotPresent( "org.apache.maven.its.mng5760", "module-d", "1.0", "jar" );
+ }
+ finally
+ {
+ verifier.resetStreams();
+ }
+
+ // Let module-b and module-c fail, if they would have been built...
Review comment:
This should be module-a and module-b, right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-integration-testing] MartinKanters commented on pull request #98: [MNG-7095] Support for parallel builds when resuming
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on pull request #98:
URL: https://github.com/apache/maven-integration-testing/pull/98#issuecomment-778436035
> I've fixed the comments and change the tests to rely on the `verify` goal rather than `install`.
Great. They are still running atm, but the results look good: https://github.com/apache/maven/runs/1890081377?check_suite_focus=true
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-integration-testing] MartinKanters commented on pull request #98: [MNG-7095] Support for parallel builds when resuming
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on pull request #98:
URL: https://github.com/apache/maven-integration-testing/pull/98#issuecomment-778442266
> > > I've fixed the comments and change the tests to rely on the `verify` goal rather than `install`.
> >
> >
> > Great. They are still running atm, but the results look good: https://github.com/apache/maven/runs/1890081377?check_suite_focus=true
>
> Is the CI build set up to run the tests against a branch of maven with the same name ?
Yep, it checks whether the contributor who provides the PR has a matching branch on his/hers maven-integration-testing fork.
You can see it in the "Collect environment context variables" step in one of the integration-test tasks:
> Found a branch "MNG-7095" in fork "gnodet/maven-integration-testing", configuring this for the integration tests to be run against.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-integration-testing] gnodet commented on pull request #98: [MNG-7095] Support for parallel builds when resuming
Posted by GitBox <gi...@apache.org>.
gnodet commented on pull request #98:
URL: https://github.com/apache/maven-integration-testing/pull/98#issuecomment-778325826
I'll add another test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-integration-testing] MartinKanters commented on pull request #98: [MNG-7095] Support for parallel builds when resuming
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on pull request #98:
URL: https://github.com/apache/maven-integration-testing/pull/98#issuecomment-778411008
I've rerun the GitHub Actions build on your maven PR, this should take these new ITs into account as well (as both branches are named the same):
https://github.com/apache/maven/actions/runs/561224481
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-integration-testing] MartinKanters commented on pull request #98: [MNG-7095] Support for parallel builds when resuming
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on pull request #98:
URL: https://github.com/apache/maven-integration-testing/pull/98#issuecomment-778420212
> I've rerun the GitHub Actions build on your maven PR, this should take these new ITs into account as well (as both branches are named the same):
> https://github.com/apache/maven/actions/runs/561224481
FYI, the ITs failed because your maven-integration-testing branch is not even with apache:master.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-integration-testing] MartinKanters closed pull request #98: [MNG-7095] Support for parallel builds when resuming
Posted by GitBox <gi...@apache.org>.
MartinKanters closed pull request #98:
URL: https://github.com/apache/maven-integration-testing/pull/98
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-integration-testing] gnodet commented on pull request #98: [MNG-7095] Support for parallel builds when resuming
Posted by GitBox <gi...@apache.org>.
gnodet commented on pull request #98:
URL: https://github.com/apache/maven-integration-testing/pull/98#issuecomment-778440870
> > I've fixed the comments and change the tests to rely on the `verify` goal rather than `install`.
>
> Great. They are still running atm, but the results look good: https://github.com/apache/maven/runs/1890081377?check_suite_focus=true
Is the CI build set up to run the tests against a branch of maven with the same name ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org