gnodet-bot commented on code in PR #12620: URL: https://github.com/apache/maven/pull/12620#discussion_r4065573510
########## its/core-it-suite/src/test/java/org/apache/maven/it/MavenITRememberModelProblemsTest.java: ########## @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.it; + +import java.nio.file.Path; +import java.util.Properties; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Verifies that model problems encountered during reactor discovery remain available from the build session. + */ +public class MavenITRememberModelProblemsTest extends AbstractMavenIntegrationTestCase { + + @Test + public void testModelProblemStateInNativeSession() throws Exception { + Path testDir = extractResources("remember-model-problems"); Review Comment: 🚨 **Compile error: missing `super(versionRange)` call.** `AbstractMavenIntegrationTestCase` has no no-arg constructor — all three of its constructors require a `versionRangeStr` argument (see `its/core-it-support/maven-it-helper/src/main/java/org/apache/maven/it/AbstractMavenIntegrationTestCase.java:61`). Every other IT class that extends it must call `super(...)` from an explicit constructor, e.g.: ```java class MavenITmng8736ConcurrentFileActivationTest extends AbstractMavenIntegrationTestCase { MavenITmng8736ConcurrentFileActivationTest() { super("[4.0.0-alpha-1,)"); } ``` This class has no constructor at all and will fail to compile. Beyond the compile error, the version range is also semantically required: `getModelProblemCollector()` is a new 4.1.0 API, so the constraint should be at minimum `"[4.1.0-alpha-1,)"` to prevent the test from running against older Maven distributions that don't have the method. ```suggestion public class MavenITRememberModelProblemsTest extends AbstractMavenIntegrationTestCase { public MavenITRememberModelProblemsTest() { super("[4.1.0-alpha-1,)"); } ``` ########## impl/maven-testing/src/main/java/org/apache/maven/testing/plugin/stubs/SessionStub.java: ########## @@ -153,6 +155,12 @@ public SessionData getData() { return null; } + @Nonnull + @Override + public ProblemCollector<ModelProblem> getModelProblemCollector() { + return ProblemCollector.empty(); Review Comment: ⚠️ **`ProblemCollector.empty()` throws on `reportProblem()` — wrong stub contract.** `ProblemCollector.empty()` deliberately throws `IllegalStateException` on `reportProblem()` (see `ProblemCollector.java`). `SessionStub` is a plugin-testing stub that downstream plugins wire into their unit tests. Any plugin code path that calls `session.getModelProblemCollector().reportProblem(...)` in a test will explode with a non-obvious exception that has nothing to do with what's being tested. A no-op stub that silently discards problems (rather than throwing) is the correct contract for a test double: ```suggestion return ProblemCollector.create(0); ``` `ProblemCollector.create(0)` accepts reports (increments the counter), stores no problems due to `maxCountLimit=0`, and never throws. It behaves like a real but saturated collector — a much safer default for plugin tests than a collector that actively sabotages them. -- 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]
