From 0feb8715ac1ad01ad58ca69a26cb050e928eeb35 Mon Sep 17 00:00:00 2001
From: Piotr Findeisen <piotr.findeisen@gmail.com>
Date: Mon, 22 Feb 2021 21:35:04 +0100
Subject: [PATCH] Merge TestRedisDistributed with TestRedisIntegrationSmokeTest

Merge `TestRedisDistributed` into `TestRedisIntegrationSmokeTest`
utilizing `BaseConnectorTest` base class.
---
 ...buted.java => TestRedisConnectorTest.java} | 60 +++++++++++--
 .../redis/TestRedisIntegrationSmokeTest.java  | 48 -----------
 .../AbstractTestDistributedQueries.java       | 85 +++++++++++++++----
 3 files changed, 124 insertions(+), 69 deletions(-)
 rename plugin/trino-redis/src/test/java/io/trino/plugin/redis/{TestRedisDistributed.java => TestRedisConnectorTest.java} (51%)
 delete mode 100644 plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisIntegrationSmokeTest.java

diff --git a/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisDistributed.java b/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisConnectorTest.java
similarity index 51%
rename from plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisDistributed.java
rename to plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisConnectorTest.java
index cc7e428d8611..af7c98e11c8e 100644
--- a/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisDistributed.java
+++ b/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisConnectorTest.java
@@ -15,15 +15,15 @@
 
 import com.google.common.collect.ImmutableMap;
 import io.trino.plugin.redis.util.RedisServer;
-import io.trino.testing.AbstractTestQueries;
+import io.trino.testing.BaseConnectorTest;
 import io.trino.testing.QueryRunner;
 import io.trino.tpch.TpchTable;
 import org.testng.annotations.AfterClass;
-import org.testng.annotations.Test;
 
-@Test
-public class TestRedisDistributed
-        extends AbstractTestQueries
+import static io.trino.plugin.redis.RedisQueryRunner.createRedisQueryRunner;
+
+public class TestRedisConnectorTest
+        extends BaseConnectorTest
 {
     private RedisServer redisServer;
 
@@ -32,7 +32,7 @@ protected QueryRunner createQueryRunner()
             throws Exception
     {
         redisServer = new RedisServer();
-        return RedisQueryRunner.createRedisQueryRunner(redisServer, ImmutableMap.of(), "string", TpchTable.getTables());
+        return createRedisQueryRunner(redisServer, ImmutableMap.of(), "string", TpchTable.getTables());
     }
 
     @AfterClass(alwaysRun = true)
@@ -40,4 +40,52 @@ public void destroy()
     {
         redisServer.close();
     }
+
+    @Override
+    protected boolean supportsCreateSchema()
+    {
+        return false;
+    }
+
+    @Override
+    protected boolean supportsCreateTable()
+    {
+        return false;
+    }
+
+    @Override
+    protected boolean supportsInsert()
+    {
+        return false;
+    }
+
+    @Override
+    protected boolean supportsDelete()
+    {
+        return false;
+    }
+
+    @Override
+    protected boolean supportsViews()
+    {
+        return false;
+    }
+
+    @Override
+    protected boolean supportsArrays()
+    {
+        return false;
+    }
+
+    @Override
+    protected boolean supportsCommentOnTable()
+    {
+        return false;
+    }
+
+    @Override
+    protected boolean supportsCommentOnColumn()
+    {
+        return false;
+    }
 }
diff --git a/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisIntegrationSmokeTest.java b/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisIntegrationSmokeTest.java
deleted file mode 100644
index a01c412b3dff..000000000000
--- a/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisIntegrationSmokeTest.java
+++ /dev/null
@@ -1,48 +0,0 @@
-/*
- * Licensed 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 io.trino.plugin.redis;
-
-import io.trino.plugin.redis.util.RedisServer;
-import io.trino.testing.AbstractTestIntegrationSmokeTest;
-import io.trino.testing.QueryRunner;
-import org.testng.annotations.AfterClass;
-import org.testng.annotations.Test;
-
-import static io.trino.plugin.redis.RedisQueryRunner.createRedisQueryRunner;
-import static io.trino.tpch.TpchTable.CUSTOMER;
-import static io.trino.tpch.TpchTable.NATION;
-import static io.trino.tpch.TpchTable.ORDERS;
-import static io.trino.tpch.TpchTable.REGION;
-
-@Test
-public class TestRedisIntegrationSmokeTest
-        // TODO extend BaseConnectorTest
-        extends AbstractTestIntegrationSmokeTest
-{
-    private RedisServer redisServer;
-
-    @Override
-    protected QueryRunner createQueryRunner()
-            throws Exception
-    {
-        redisServer = new RedisServer();
-        return createRedisQueryRunner(redisServer, "string", CUSTOMER, NATION, ORDERS, REGION);
-    }
-
-    @AfterClass(alwaysRun = true)
-    public void destroy()
-    {
-        redisServer.close();
-    }
-}
diff --git a/testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java b/testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java
index 65988844e3df..7fb6e2511830 100644
--- a/testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java
+++ b/testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java
@@ -73,6 +73,21 @@
 public abstract class AbstractTestDistributedQueries
         extends AbstractTestQueries
 {
+    protected boolean supportsCreateSchema()
+    {
+        return true;
+    }
+
+    protected boolean supportsCreateTable()
+    {
+        return true;
+    }
+
+    protected boolean supportsInsert()
+    {
+        return true;
+    }
+
     protected boolean supportsDelete()
     {
         return true;
@@ -161,6 +176,11 @@ public void testResetSession()
     public void testCreateTable()
     {
         String tableName = "test_create_" + randomTableSuffix();
+        if (!supportsCreateTable()) {
+            assertQueryFails("CREATE TABLE " + tableName + " (a bigint, b double, c varchar)", "This connector does not support creating tables");
+            return;
+        }
+
         assertUpdate("CREATE TABLE " + tableName + " (a bigint, b double, c varchar)");
         assertTrue(getQueryRunner().tableExists(getSession(), tableName));
         assertTableColumnNames(tableName, "a", "b", "c");
@@ -206,6 +226,10 @@ public void testCreateTable()
     public void testCreateTableAsSelect()
     {
         String tableName = "test_ctas" + randomTableSuffix();
+        if (!supportsCreateTable()) {
+            assertQueryFails("CREATE TABLE IF NOT EXISTS " + tableName + " AS SELECT name, regionkey FROM nation", "This connector does not support creating tables with data");
+            return;
+        }
         assertUpdate("CREATE TABLE IF NOT EXISTS " + tableName + " AS SELECT name, regionkey FROM nation", "SELECT count(*) FROM nation");
         assertTableColumnNames(tableName, "name", "regionkey");
         assertUpdate("DROP TABLE " + tableName);
@@ -296,6 +320,8 @@ protected void assertCreateTableAsSelect(Session session, @Language("SQL") Strin
     @Test
     public void testRenameTable()
     {
+        skipTestUnless(supportsCreateTable());
+
         String tableName = "test_rename_" + randomTableSuffix();
         assertUpdate("CREATE TABLE " + tableName + " AS SELECT 123 x", 1);
 
@@ -326,14 +352,12 @@ public void testRenameTable()
     @Test
     public void testCommentTable()
     {
-        String tableName = "test_comment_" + randomTableSuffix();
         if (!supportsCommentOnTable()) {
-            assertUpdate("CREATE TABLE " + tableName + "(a integer)");
-            assertQueryFails("COMMENT ON TABLE " + tableName + " IS 'new comment'", "This connector does not support setting table comments");
-            assertUpdate("DROP TABLE " + tableName);
+            assertQueryFails("COMMENT ON TABLE nation IS 'new comment'", "This connector does not support setting table comments");
             return;
         }
 
+        String tableName = "test_comment_" + randomTableSuffix();
         assertUpdate("CREATE TABLE " + tableName + "(a integer)");
 
         // comment set
@@ -377,14 +401,12 @@ private String getTableComment(String tableName)
     @Test
     public void testCommentColumn()
     {
-        String tableName = "test_comment_column_" + randomTableSuffix();
         if (!supportsCommentOnColumn()) {
-            assertUpdate("CREATE TABLE " + tableName + "(a integer)");
-            assertQueryFails("COMMENT ON COLUMN " + tableName + ".a IS 'new comment'", "This connector does not support setting column comments");
-            assertUpdate("DROP TABLE " + tableName);
+            assertQueryFails("COMMENT ON COLUMN nation.nationkey IS 'new comment'", "This connector does not support setting column comments");
             return;
         }
 
+        String tableName = "test_comment_column_" + randomTableSuffix();
         assertUpdate("CREATE TABLE " + tableName + "(a integer)");
 
         // comment set
@@ -427,6 +449,8 @@ private String getColumnComment(String tableName, String columnName)
     @Test
     public void testRenameColumn()
     {
+        skipTestUnless(supportsCreateTable());
+
         String tableName = "test_rename_column_" + randomTableSuffix();
         assertUpdate("CREATE TABLE " + tableName + " AS SELECT 'some value' x", 1);
 
@@ -458,6 +482,8 @@ public void testRenameColumn()
     @Test
     public void testDropColumn()
     {
+        skipTestUnless(supportsCreateTable());
+
         String tableName = "test_drop_column_" + randomTableSuffix();
         assertUpdate("CREATE TABLE " + tableName + " AS SELECT 123 x, 456 y, 111 a", 1);
 
@@ -480,6 +506,8 @@ public void testDropColumn()
     @Test
     public void testAddColumn()
     {
+        skipTestUnless(supportsCreateTable());
+
         String tableName = "test_add_column_" + randomTableSuffix();
         assertUpdate("CREATE TABLE " + tableName + " AS SELECT CAST('first' AS varchar) x", 1);
 
@@ -516,6 +544,11 @@ public void testAddColumn()
     @Test
     public void testInsert()
     {
+        if (!supportsInsert()) {
+            assertQueryFails("INSERT INTO nation(nationkey) VALUES (42)", "This connector does not support inserts");
+            return;
+        }
+
         String query = "SELECT phone, custkey, acctbal FROM customer";
 
         String tableName = "test_insert_" + randomTableSuffix();
@@ -556,6 +589,8 @@ public void testInsert()
     @Test
     public void testInsertUnicode()
     {
+        skipTestUnless(supportsInsert());
+
         String tableName = "test_insert_unicode_" + randomTableSuffix();
 
         assertUpdate("CREATE TABLE " + tableName + "(test varchar)");
@@ -586,6 +621,8 @@ public void testInsertUnicode()
     @Test
     public void testInsertArray()
     {
+        skipTestUnless(supportsInsert());
+
         String tableName = "test_insert_array_" + randomTableSuffix();
         if (!supportsArrays()) {
             assertThatThrownBy(() -> query("CREATE TABLE " + tableName + " (a array(bigint))"))
@@ -606,18 +643,15 @@ public void testInsertArray()
     @Test
     public void testDelete()
     {
-        // delete half the table, then delete the rest
-        String tableName = "test_delete_" + randomTableSuffix();
-
         if (!supportsDelete()) {
-            assertUpdate("CREATE TABLE " + tableName + " AS SELECT * FROM orders WITH NO DATA", 0);
-            assertQueryFails("DELETE FROM " + tableName, "This connector does not support deletes");
-            assertUpdate("DROP TABLE " + tableName);
+            assertQueryFails("DELETE FROM nation", "This connector does not support deletes");
             return;
         }
 
+        String tableName = "test_delete_" + randomTableSuffix();
         assertUpdate("CREATE TABLE " + tableName + " AS SELECT * FROM orders", "SELECT count(*) FROM orders");
 
+        // delete half the table, then delete the rest
         assertUpdate("DELETE FROM " + tableName + " WHERE orderkey % 2 = 0", "SELECT count(*) FROM orders WHERE orderkey % 2 = 0");
         assertQuery("SELECT * FROM " + tableName, "SELECT * FROM orders WHERE orderkey % 2 <> 0");
 
@@ -958,6 +992,8 @@ public void testShowCreateView()
     @Test
     public void testQueryLoggingCount()
     {
+        skipTestUnless(supportsCreateTable());
+
         QueryManager queryManager = getDistributedQueryRunner().getCoordinator().getQueryManager();
         executeExclusively(() -> {
             assertEventually(
@@ -1013,9 +1049,12 @@ public void testShowSchemasFromOther()
         assertTrue(result.getOnlyColumnAsSet().containsAll(ImmutableSet.of(INFORMATION_SCHEMA, "tiny", "sf1")));
     }
 
+    // TODO move to to engine-only
     @Test
     public void testSymbolAliasing()
     {
+        skipTestUnless(supportsCreateTable());
+
         String tableName = "test_symbol_aliasing" + randomTableSuffix();
         assertUpdate("CREATE TABLE " + tableName + " AS SELECT 1 foo_1, 2 foo_2_4", 1);
         assertQuery("SELECT foo_1, foo_2_4 FROM " + tableName, "SELECT 1, 2");
@@ -1026,6 +1065,9 @@ public void testSymbolAliasing()
     @Flaky(issue = "https://github.com/trinodb/trino/issues/5172", match = "AssertionError: expected \\[.*\\] but found \\[.*\\]")
     public void testWrittenStats()
     {
+        skipTestUnless(supportsCreateTable());
+        skipTestUnless(supportsInsert());
+
         String tableName = "test_written_stats_" + randomTableSuffix();
         String sql = "CREATE TABLE " + tableName + " AS SELECT * FROM nation";
         ResultWithQueryId<MaterializedResult> resultResultWithQueryId = getDistributedQueryRunner().executeWithQueryId(getSession(), sql);
@@ -1050,6 +1092,10 @@ public void testWrittenStats()
     public void testCreateSchema()
     {
         String schemaName = "test_schema_create_" + randomTableSuffix();
+        if (!supportsCreateSchema()) {
+            assertQueryFails("CREATE SCHEMA " + schemaName, "This connector does not support creating schemas");
+            return;
+        }
         assertThat(computeActual("SHOW SCHEMAS").getOnlyColumnAsSet()).doesNotContain(schemaName);
         assertUpdate("CREATE SCHEMA " + schemaName);
         assertThat(computeActual("SHOW SCHEMAS").getOnlyColumnAsSet()).contains(schemaName);
@@ -1061,6 +1107,8 @@ public void testCreateSchema()
     @Test
     public void testInsertForDefaultColumn()
     {
+        skipTestUnless(supportsInsert());
+
         try (TestTable testTable = createTableWithDefaultColumns()) {
             assertUpdate(format("INSERT INTO %s (col_required, col_required2) VALUES (1, 10)", testTable.getName()), 1);
             assertUpdate(format("INSERT INTO %s VALUES (2, 3, 4, 5, 6)", testTable.getName()), 1);
@@ -1071,11 +1119,16 @@ public void testInsertForDefaultColumn()
         }
     }
 
-    protected abstract TestTable createTableWithDefaultColumns();
+    protected TestTable createTableWithDefaultColumns()
+    {
+        throw new UnsupportedOperationException();
+    }
 
     @Test(dataProvider = "testColumnNameDataProvider")
     public void testColumnName(String columnName)
     {
+        skipTestUnless(supportsCreateTable());
+
         if (!requiresDelimiting(columnName)) {
             testColumnName(columnName, false);
         }
@@ -1175,6 +1228,8 @@ protected String dataMappingTableName(String trinoTypeName)
     @Test(dataProvider = "testDataMappingSmokeTestDataProvider")
     public void testDataMappingSmokeTest(DataMappingTestSetup dataMappingTestSetup)
     {
+        skipTestUnless(supportsCreateTable());
+
         String trinoTypeName = dataMappingTestSetup.getTrinoTypeName();
         String sampleValueLiteral = dataMappingTestSetup.getSampleValueLiteral();
         String highValueLiteral = dataMappingTestSetup.getHighValueLiteral();