sashapolo commented on code in PR #1709: URL: https://github.com/apache/ignite-3/pull/1709#discussion_r1120222335
########## modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/junit/StopAllIgnitesAfterTests.java: ########## @@ -0,0 +1,93 @@ +/* + * 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.testframework.junit; + +import java.lang.reflect.Method; +import java.util.ServiceLoader; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.ExtensionContext; + +/** + * This extension tries to do it best to stop all Ignite instances that were started in this JVM after a test + * suite finishes running (after-all). + * + * <p>This extension is designed to be + * <a href="https://junit.org/junit5/docs/current/user-guide/#extensions-registration-automatic";>automatically registered</a> + * via META-INF/services/org.junit.jupiter.api.extension.Extension. + * For this to work, system property {@code junit.jupiter.extensions.autodetection.enabled} must be set to {@code true}. + * If the property is set (currently, this is done via Gradle build scripts), it is enough to add this module as a dependency + * to make tests automatically register this extension, like this: + * + * <pre> + * integrationTestImplementation(testFixtures(project(':ignite-core'))) + * </pre> + */ +public class StopAllIgnitesAfterTests implements AfterAllCallback { + private static final String IGNITION_MANAGER_CLASS_NAME = "org.apache.ignite.IgnitionManager"; + + private static final String IGNITION_CLASS_NAME = "org.apache.ignite.Ignition"; + + private static final String IGNITION_IMPL_CLASS_NAME = "org.apache.ignite.internal.app.IgnitionImpl"; + + private static final IgniteLogger LOG = Loggers.forClass(StopAllIgnitesAfterTests.class); + + @Override + public void afterAll(ExtensionContext context) throws Exception { + // Try to stop all Ignite nodes via reflection to make sure that this extension does not break anything + // even in modules where IgnitionManager or IgnitionImpl are not available (so even for unit tests). + + Class<?> ignitionClass = findClassByName(IGNITION_CLASS_NAME); + Class<?> ignitionImplClass = findClassByName(IGNITION_IMPL_CLASS_NAME); + + if (isIgnitionManagerClassAvailable() && ignitionClass != null && ignitionImplClass != null) { + String testInstanceName = context.getTestClass().map(Class::getName).orElse("<unknown>"); Review Comment: Should we use `orElseThrow` instead of `orElse("<unknown>")`? When can the test class be absent? ########## modules/runner/src/main/java/org/apache/ignite/internal/app/IgnitionImpl.java: ########## @@ -138,6 +140,24 @@ public void stop(String nodeName) { }); } + /** + * Stops all Ignite instances started in this JVM. + */ + @TestOnly + public void stopAll() { + List<String> nodeNames = new ArrayList<>(nodes.keySet()); + + if (!nodeNames.isEmpty()) { + LOG.info("Going to stop the following Ignite instances: " + nodeNames); Review Comment: ```suggestion LOG.info("Going to stop Ignite instances: " + nodeNames); ``` ########## modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/junit/StopAllIgnitesAfterTests.java: ########## @@ -0,0 +1,93 @@ +/* + * 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.testframework.junit; + +import java.lang.reflect.Method; +import java.util.ServiceLoader; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.ExtensionContext; + +/** + * This extension tries to do it best to stop all Ignite instances that were started in this JVM after a test + * suite finishes running (after-all). + * + * <p>This extension is designed to be + * <a href="https://junit.org/junit5/docs/current/user-guide/#extensions-registration-automatic";>automatically registered</a> + * via META-INF/services/org.junit.jupiter.api.extension.Extension. + * For this to work, system property {@code junit.jupiter.extensions.autodetection.enabled} must be set to {@code true}. + * If the property is set (currently, this is done via Gradle build scripts), it is enough to add this module as a dependency + * to make tests automatically register this extension, like this: + * + * <pre> + * integrationTestImplementation(testFixtures(project(':ignite-core'))) + * </pre> + */ +public class StopAllIgnitesAfterTests implements AfterAllCallback { + private static final String IGNITION_MANAGER_CLASS_NAME = "org.apache.ignite.IgnitionManager"; + + private static final String IGNITION_CLASS_NAME = "org.apache.ignite.Ignition"; + + private static final String IGNITION_IMPL_CLASS_NAME = "org.apache.ignite.internal.app.IgnitionImpl"; + + private static final IgniteLogger LOG = Loggers.forClass(StopAllIgnitesAfterTests.class); + + @Override + public void afterAll(ExtensionContext context) throws Exception { + // Try to stop all Ignite nodes via reflection to make sure that this extension does not break anything Review Comment: I don't understand the reason for using reflection here. If this extension is only supposed to be used for Ignite instances that were created using the `Ignition` interface, then these tests should already be dependent on the `api` module. Why can't this extension use these classes directly as well? Or at least not hardcode the class names ########## modules/runner/src/main/java/org/apache/ignite/internal/app/IgnitionImpl.java: ########## @@ -138,6 +140,24 @@ public void stop(String nodeName) { }); } + /** + * Stops all Ignite instances started in this JVM. + */ + @TestOnly + public void stopAll() { + List<String> nodeNames = new ArrayList<>(nodes.keySet()); + + if (!nodeNames.isEmpty()) { + LOG.info("Going to stop the following Ignite instances: " + nodeNames); + + for (String nodeName : nodeNames) { + stop(nodeName); + } + + LOG.info("Stopped the following Ignite instances: " + nodeNames); Review Comment: Do we really need this message as well? -- 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]
