gnodet commented on code in PR #12242:
URL: https://github.com/apache/maven/pull/12242#discussion_r3407728432
##########
impl/maven-testing/src/main/java/org/apache/maven/api/plugin/testing/MojoExtension.java:
##########
@@ -437,9 +437,9 @@ public void beforeEach(ExtensionContext context) throws
Exception {
} else {
model = new MavenMerger().merge(tmodel, defaultModel, false, null);
}
- tmodel = new DefaultModelPathTranslator(new DefaultPathTranslator())
- .alignToBaseDirectory(tmodel, Paths.get(getBasedir()), null);
- context.getStore(MOJO_EXTENSION).put(Model.class, tmodel);
+ final Model alignedModel = new DefaultModelPathTranslator(new
DefaultPathTranslator())
+ .alignToBaseDirectory(model, Paths.get(getBasedir()), null);
Review Comment:
This is the heart of the fix: `model` is guaranteed non-null here (either
`defaultModel` or the merge result), so the NPE is eliminated. And since
`model` carries the merged defaults, the aligned result now includes build
paths and coordinates from `defaultModel` — which was the other half of the bug.
Nit: the local `model` variable (line 434) is now only used as an
intermediate to compute `alignedModel`. That's fine — keeping it separate is
more readable than inlining the ternary.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
