sashapolo commented on code in PR #1709:
URL: https://github.com/apache/ignite-3/pull/1709#discussion_r1124118760
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/internal/InternalSchemaTest.java:
##########
@@ -26,15 +26,15 @@
import org.apache.ignite.internal.configuration.ConfigurationManager;
import org.apache.ignite.internal.schema.configuration.ExtendedTableView;
import org.apache.ignite.internal.schema.configuration.TablesConfiguration;
-import org.apache.ignite.internal.sql.engine.AbstractBasicIntegrationTest;
+import org.apache.ignite.internal.sql.engine.SharedClusterIntegrationTest;
import org.apache.ignite.internal.testframework.IgniteTestUtils;
import org.apache.ignite.sql.IgniteSql;
import org.apache.ignite.sql.ResultSet;
import org.apache.ignite.sql.Session;
import org.junit.jupiter.api.Test;
/** Tests for internal manipulations with schema. */
-public class InternalSchemaTest extends AbstractBasicIntegrationTest {
+public class InternalSchemaTest extends SharedClusterIntegrationTest {
Review Comment:
Not related to this PR, but this test's name doesn't start with `It`
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/TestStartingIgnites.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * 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;
+
+import org.apache.ignite.internal.junit.StopAllIgnitesAfterTests;
+import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * A test that starts some Ignite instances (and cleans them up later if they
are forgotten).
+ */
+// The order is important here.
+@ExtendWith({WorkDirectoryExtension.class, StopAllIgnitesAfterTests.class})
+public abstract class TestStartingIgnites extends BaseIgniteAbstractTest {
Review Comment:
This class' name looks ugly to me. Can we introduce a different one?
`IgniteIntegrationTest`?
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/SharedClusterIntegrationTest.java:
##########
@@ -67,15 +66,13 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.TestInfo;
import org.junit.jupiter.api.TestInstance;
-import org.junit.jupiter.api.extension.ExtendWith;
/**
- * Abstract basic integration test.
+ * Abstract basic integration test that starts a cluster once for all the
tests it runs.
*/
-@ExtendWith(WorkDirectoryExtension.class)
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
-public class AbstractBasicIntegrationTest extends BaseIgniteAbstractTest {
- private static final IgniteLogger LOG =
Loggers.forClass(AbstractBasicIntegrationTest.class);
+public abstract class SharedClusterIntegrationTest extends TestStartingIgnites
{
Review Comment:
I would prefer this class to be called `ClusterPerClassIntegrationTest`
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sqllogic/ItSqlLogicTest.java:
##########
@@ -137,11 +135,10 @@
* @see <a
href="https://www.sqlite.org/sqllogictest/doc/trunk/about.wiki";>Extended format
documentation.</a>
*/
@Tag(value = "sqllogic")
-@ExtendWith({WorkDirectoryExtension.class, SystemPropertiesExtension.class})
Review Comment:
Looks like `SystemPropertiesExtension` is still needed
--
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]
