gnodet-bot commented on code in PR #13229:
URL: https://github.com/apache/maven/pull/13229#discussion_r4065351100
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java:
##########
@@ -936,6 +936,9 @@ protected void
populateRequestFromSettings(MavenExecutionRequest request, Settin
}
protected int calculateDegreeOfConcurrency(String threadConfiguration) {
+ if ("max".equalsIgnoreCase(threadConfiguration)) {
Review Comment:
⚠️ **Missing test coverage for this code path.**
The PR adds tests for `MavenCli.calculateDegreeOfConcurrency` (compat,
deprecated) but not for `LookupInvoker.calculateDegreeOfConcurrency` — which is
the **production code path** for Maven 4.x. Both implementations are identical,
but a test that covers only the deprecated shim leaves the real implementation
unverified.
Please add a test in `impl/maven-cli/src/test/` that mirrors the
`MavenCliTest.testCalculateDegreeOfConcurrency` cases — at minimum the
`max`/`MAX`/`Max` variants and a smoke check that the error message still
includes `'max'`.
A subclass of `LookupInvoker` can be created for testing (the method is
`protected`), or a test on `MavenInvoker` directly:
```java
//
impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerCalculateThreadsTest.java
class MavenInvokerCalculateThreadsTest {
// expose the protected method via a minimal subclass
static class TestableInvoker extends MavenInvoker {
TestableInvoker() { super(null, null); }
@Override protected int execute(MavenContext ctx) { return 0; }
}
private final TestableInvoker invoker = new TestableInvoker();
@Test
void maxIsAvailableProcessorsMinusOneMinimumOne() {
int cpus = Runtime.getRuntime().availableProcessors();
assertEquals(Math.max(1, cpus - 1),
invoker.calculateDegreeOfConcurrency("max"));
assertEquals(Math.max(1, cpus - 1),
invoker.calculateDegreeOfConcurrency("MAX"));
assertEquals(Math.max(1, cpus - 1),
invoker.calculateDegreeOfConcurrency("Max"));
}
@Test
void invalidValueThrowsWithMaxMentionedInMessage() {
IllegalArgumentException ex = assertThrows(
IllegalArgumentException.class,
() -> invoker.calculateDegreeOfConcurrency("invalid"));
assertThat(ex.getMessage(), containsString("'max'"));
}
}
```
--
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]
