Migrate core ITs from maven-verifier to maven-executor (3.10.x) - #446
Open
slachiewicz wants to merge 5 commits into
Open
slachiewicz wants to merge 5 commits into
slachiewicz wants to merge 5 commits into
Conversation
slachiewicz
added a commit
that referenced
this pull request
Sep 21, 2026
…rgument The prior execute() prepended "-l <logFileName>" to the CLI arguments to get Maven's own log-file flag to populate the log. maven-verifier's ForkedLauncher/Embedded3xLauncher never did that: they redirected the actual process stdout/stderr into the log file directly, which is what lets MavenITmng3183LoggingToFileTest (which adds its own "-l maven.log" argument) still see an empty captured stdout. Restored that behavior via ExecutorRequest's stdOut/stdErr, buffered per-stream to avoid the maven-executor pump threads closing a shared stream out from under each other, then written into the log file after execution. Caught by upstream CI on PR #446 (run 35591231728): 876/877 core-it-suite tests passed, this one failed.
slachiewicz
marked this pull request as ready for review
September 21, 2026 11:21
maven-verifier is deprecated (apache/maven-verifier#186); the Verifier is now the in-repo org.apache.maven.it.Verifier on maven-executor, as on master. Artifact paths are computed locally, without the toolbox plugin (apache/maven-executor#44). (cherry picked from commit 8b1c9b0)
These three IT classes landed on maven-3.10.x after the maven-3.9.x branch point and still imported org.apache.maven.shared.verifier.*, so the cherry-picked migration commit did not touch them.
Same fix as the maven-3.9.x branch: maven-verifier kept this overload alongside the Map<String,String> one and MavenITmng0768OfflineModeTest still calls it with a Properties instance.
spotless:apply removed the now-redundant org.apache.maven.it.Verifier / ResourceExtractor imports the sed-based migration left behind (Verifier, VerificationException and ResourceExtractor all live in the tests' own org.apache.maven.it package) and reflowed one line in Verifier.java.
…rgument The prior execute() prepended "-l <logFileName>" to the CLI arguments to get Maven's own log-file flag to populate the log. maven-verifier's ForkedLauncher/Embedded3xLauncher never did that: they redirected the actual process stdout/stderr into the log file directly, which is what lets MavenITmng3183LoggingToFileTest (which adds its own "-l maven.log" argument) still see an empty captured stdout. Restored that behavior via ExecutorRequest's stdOut/stdErr, buffered per-stream to avoid the maven-executor pump threads closing a shared stream out from under each other, then written into the log file after execution. Caught by upstream CI on this PR (run 35591231728): 876/877 core-it-suite tests passed, this one failed.
slachiewicz
force-pushed
the
agent/maven-executor-migration-310
branch
from
October 1, 2026 14:12
e1b0fa5 to
caa0ecc
Compare
slachiewicz
added a commit
that referenced
this pull request
Oct 1, 2026
…rgument The prior execute() prepended "-l <logFileName>" to the CLI arguments to get Maven's own log-file flag to populate the log. maven-verifier's ForkedLauncher/Embedded3xLauncher never did that: they redirected the actual process stdout/stderr into the log file directly, which is what lets MavenITmng3183LoggingToFileTest (which adds its own "-l maven.log" argument) still see an empty captured stdout. Restored that behavior via ExecutorRequest's stdOut/stdErr, buffered per-stream to avoid the maven-executor pump threads closing a shared stream out from under each other, then written into the log file after execution. Caught by upstream CI on PR #446 (run 35591231728): 876/877 core-it-suite tests passed, this one failed.
Comment on lines
+147
to
+150
| @Deprecated | ||
| public Verifier(String basedir, boolean debug) throws VerificationException { | ||
| this(basedir); | ||
| } |
Member
There was a problem hiding this comment.
Why we add deprecated code in new class?
slawekjaranowski
left a comment
slawekjaranowski
left a comment
Member
There was a problem hiding this comment.
What I see .... we deprecated / retire verifier
and now we have a copy like ResourceExtractor.java, Verifier.java in each project?
Maybe we should have a shared code for such in verifier and verifier should use executor ...
Member
|
Yes, this is maybe too early, but cool work! So IMO we need to empower maven-executor first, but this PR is very good work and can serve as example "what we need more in executor". Related: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Forward-port of #445 to maven-3.10.x: maven-verifier is deprecated (apache/maven-verifier#186); the Verifier used by the core ITs is now the in-repo
org.apache.maven.it.Verifier, built on maven-executor, the same way apache/maven's owncore-it-support/maven-it-helperdoes on master. Artifact and local-repository paths are computed locally rather than through a plugin execution, so running the suite does not require downloading a third-party plugin (apache/maven-executor#44).core-it-support/maven-it-sampleandmaven-it-sample-archetypestill referencemaven-verifierin their POMs; both are template/fixture projects outside the reactor, so they were left as-is.Verified: CI (
./mvnw -B clean install -Prun-its,embedded -Dmaven.repo.local=pwd/repo, 18-job matrix: ubuntu/macos/windows × JDK 8/11/17/21/25/26) → Tests run: 877, Failures: 0, Errors: 0, Skipped: 38, all 18 jobs green.