Copilot commented on code in PR #13602: URL: https://github.com/apache/ignite/pull/13602#discussion_r4061841094
########## modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/GridCommonAbstractWrapperTest.java: ########## @@ -0,0 +1,73 @@ +/* + * 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.ignite.internal.processors.query.calcite; + +import org.apache.ignite.internal.util.typedef.internal.U; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.TestInstance; + +import static org.apache.ignite.tools.junit.JUnitTeamcityReporter.escapeForTeamcity; + +/** */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class GridCommonAbstractWrapperTest extends GridCommonAbstractTest { + /** */ + @BeforeEach + void beforeTest(TestInfo testInfo) { + printTestName(testInfo, true); + } + + /** */ + @AfterEach + void afterTest(TestInfo testInfo) { + printTestName(testInfo, false); + } + + /** Get and print the display name of the upcoming test. */ + private void printTestName(TestInfo testInfo, boolean start) { + // Get and print the display name of the upcoming test + String testName = testInfo.getDisplayName(); + String testCls = testInfo.getTestClass().orElse(Object.class).getSimpleName(); + + String testFullName = escapeForTeamcity(testCls + "#" + testName); + + if (start) + U.quietAndInfo(log(), ">>> Starting test: " + testFullName + " <<<"); + else + U.quietAndInfo(log(), ">>> Stopping test: " + testFullName + " <<<"); + } + + /** */ + @BeforeAll + void init() { + beforeFirstTest0(); + } Review Comment: Jupiter does not execute `GridAbstractTest`'s JUnit 4 `@ClassRule`; that rule normally runs the full `beforeFirstTest`/`afterLastTest` scaffolding. This bridge calls only `beforeFirstTest0()` (the shared IP finder), so class-loader setup/restoration, test-resource/thread cleanup, and other class-level environment cleanup are omitted for the migrated tests. Please provide an equivalent complete Jupiter lifecycle bridge rather than only initializing the IP finder. ########## modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/GridCommonAbstractWrapperTest.java: ########## @@ -0,0 +1,73 @@ +/* + * 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.ignite.internal.processors.query.calcite; + +import org.apache.ignite.internal.util.typedef.internal.U; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.TestInstance; + +import static org.apache.ignite.tools.junit.JUnitTeamcityReporter.escapeForTeamcity; + +/** */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class GridCommonAbstractWrapperTest extends GridCommonAbstractTest { + /** */ + @BeforeEach + void beforeTest(TestInfo testInfo) { + printTestName(testInfo, true); + } + + /** */ + @AfterEach + void afterTest(TestInfo testInfo) { + printTestName(testInfo, false); + } + + /** Get and print the display name of the upcoming test. */ + private void printTestName(TestInfo testInfo, boolean start) { + // Get and print the display name of the upcoming test + String testName = testInfo.getDisplayName(); + String testCls = testInfo.getTestClass().orElse(Object.class).getSimpleName(); + + String testFullName = escapeForTeamcity(testCls + "#" + testName); + + if (start) + U.quietAndInfo(log(), ">>> Starting test: " + testFullName + " <<<"); + else + U.quietAndInfo(log(), ">>> Stopping test: " + testFullName + " <<<"); + } + + /** */ + @BeforeAll + void init() { + beforeFirstTest0(); + } + + /** {@inheritDoc} */ + @AfterAll + @Override protected void afterTestsStopped() throws Exception { + stopAllGrids(); + + super.afterTestsStopped(); Review Comment: This `@AfterAll` is replacing GridAbstractTest's JUnit4 ClassRule cleanup, but it only calls the non-forced stopAllGrids() and then afterTestsStopped(). The original afterLastTest() also stops test/helper threads, stops test resources, force-stops the topology, and clears several static/class-loader caches; omitting those steps allows leaked threads, grids, and cached state to affect later Jupiter test classes. Please add a bridge for the full after-last-test cleanup rather than only stopping grids. ########## modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AuthorizationIntegrationTest.java: ########## @@ -87,15 +94,29 @@ public class AuthorizationIntegrationTest extends AbstractSecurityTest { private static final AtomicInteger removeCnt = new AtomicInteger(); /** */ - @Parameterized.Parameter + @Parameter(0) public boolean allowDdl; /** */ - @Parameterized.Parameters(name = "allowDdl = {0}") + @MethodSource("parameters") public static Iterable<Object> parameters() { return Arrays.asList(false, true); } + /** */ + @BeforeAll + static void init() { + beforeFirstTest0(); + } + + /** {@inheritDoc} */ + @AfterAll + @Override protected void afterTestsStopped() throws Exception { + stopAllGrids(); + + super.afterTestsStopped(); Review Comment: This class is still a direct GridCommonAbstractTest subclass, so the JUnit4 ClassRule is not executed by Jupiter. The new `@AfterAll` only stops grids and invokes the lightweight afterTestsStopped() hook; it does not perform the core afterLastTest() thread/resource/cache cleanup or the force-stop safety check. Add the same lifecycle bridge used by the Jupiter wrapper before relying on this callback for class teardown. -- 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]
