gnodet-bot commented on code in PR #13648:
URL: https://github.com/apache/maven/pull/13648#discussion_r4352833367
##########
impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnval/ValidateInvokerTest.java:
##########
@@ -156,6 +159,54 @@ private int run(List<String> poms, Optional<String>
format, Optional<String> mod
@DisplayName("Wiring")
class WiringTests {
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ @ResourceLock(SYSTEM_PROPERTIES)
+ @DisplayName("should use standalone repositories and override the
settings local repository")
+ void shouldUseStandaloneRepositories(boolean configuredRepository)
throws Exception {
Review Comment:
💡 **Test coverage note:** `shouldUseStandaloneRepositories` passes
identically on both the old and new code path — `derivedRepositories()` (now
deleted) re-derived repositories from `session.getSettings().getRepositories()`
with a Central fallback, which is exactly what `ApiRunner.newSession()` already
does internally. The test doesn't distinguish the simplified code from the old
workaround.
Consider either:
- Renaming it to `shouldHaveCentralAsDefaultRepository` (signalling it's a
contract guard, not a regression test for this specific change), or
- Using a mock/spy to verify `createSession()` no longer calls the removed
derivation logic (asserting the simplification directly).
Either way, the test is still a valuable safety net — this is a
naming/intent suggestion.
--
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]
