From e9dcb591c82d7206b77f919dc147ed872108a01c Mon Sep 17 00:00:00 2001 From: Aurora Lahtela <24460436+AuroraLS3@users.noreply.github.com> Date: Mon, 18 Apr 2022 15:10:33 +0300 Subject: [PATCH] Made MySQL tests 3 times faster (3.5 minutes vs 1 minute) --- .../com/djrapitops/plan/BungeeSystemTest.java | 58 ++++++------- .../plan/BungeeSystemTestDependencies.java | 48 +++++++++++ .../utilities/mocks/BungeeMockComponent.java | 4 +- .../utilities/mocks/PlanBungeeMocker.java | 2 +- .../database/sql/tables/MetadataTable.java | 83 ------------------- .../commands/RemoveEverythingTransaction.java | 17 ++-- .../database/DBPatchMySQLRegressionTest.java | 15 ++-- .../plan/storage/database/DatabaseTest.java | 9 +- .../plan/storage/database/MySQLTest.java | 19 +---- .../src/test/java/utilities/DBPreparer.java | 37 +-------- Plan/config/checkstyle/checkstyle.xml | 11 ++- 11 files changed, 114 insertions(+), 189 deletions(-) create mode 100644 Plan/bungeecord/src/test/java/com/djrapitops/plan/BungeeSystemTestDependencies.java delete mode 100644 Plan/common/src/main/java/com/djrapitops/plan/storage/database/sql/tables/MetadataTable.java diff --git a/Plan/bungeecord/src/test/java/com/djrapitops/plan/BungeeSystemTest.java b/Plan/bungeecord/src/test/java/com/djrapitops/plan/BungeeSystemTest.java index 1cb48dce0..f1bea5199 100644 --- a/Plan/bungeecord/src/test/java/com/djrapitops/plan/BungeeSystemTest.java +++ b/Plan/bungeecord/src/test/java/com/djrapitops/plan/BungeeSystemTest.java @@ -48,13 +48,13 @@ class BungeeSystemTest { private DBPreparer dbPreparer; @BeforeEach - void prepareSystem(@TempDir Path temp) throws Exception { + void prepareSystem(@TempDir Path temp) { component = new BungeeMockComponent(temp); - dbPreparer = new DBPreparer(component.getPlanSystem(), TEST_PORT_NUMBER); + dbPreparer = new DBPreparer(new BungeeSystemTestDependencies(component.getPlanSystem()), TEST_PORT_NUMBER); } @Test - void bungeeEnables() throws Exception { + void bungeeEnables() { PlanSystem bungeeSystem = component.getPlanSystem(); try { PlanConfig config = bungeeSystem.getConfigSystem().getConfig(); @@ -75,45 +75,41 @@ class BungeeSystemTest { @Test void bungeeDoesNotEnableWithDefaultIP() { - EnableException thrown = assertThrows(EnableException.class, () -> { - PlanSystem bungeeSystem = component.getPlanSystem(); - try { - PlanConfig config = bungeeSystem.getConfigSystem().getConfig(); - config.set(WebserverSettings.PORT, TEST_PORT_NUMBER); - config.set(ProxySettings.IP, "0.0.0.0"); + PlanSystem bungeeSystem = component.getPlanSystem(); + try { + PlanConfig config = bungeeSystem.getConfigSystem().getConfig(); + config.set(WebserverSettings.PORT, TEST_PORT_NUMBER); + config.set(ProxySettings.IP, "0.0.0.0"); - DBSystem dbSystem = bungeeSystem.getDatabaseSystem(); - SQLiteDB db = dbSystem.getSqLiteFactory().usingDefaultFile(); - db.setTransactionExecutorServiceProvider(MoreExecutors::newDirectExecutorService); - dbSystem.setActiveDatabase(db); + DBSystem dbSystem = bungeeSystem.getDatabaseSystem(); + SQLiteDB db = dbSystem.getSqLiteFactory().usingDefaultFile(); + db.setTransactionExecutorServiceProvider(MoreExecutors::newDirectExecutorService); + dbSystem.setActiveDatabase(db); - bungeeSystem.enable(); // Throws EnableException - } finally { - bungeeSystem.disable(); - } - }); + EnableException thrown = assertThrows(EnableException.class, bungeeSystem::enable); + assertEquals("IP setting still 0.0.0.0 - Configure Alternative_IP/IP that connects to the Proxy server.", thrown.getMessage()); + } finally { + bungeeSystem.disable(); + } - assertEquals("IP setting still 0.0.0.0 - Configure Alternative_IP/IP that connects to the Proxy server.", thrown.getMessage()); } @Test void testEnableNoMySQL() { - assertThrows(EnableException.class, () -> { - PlanSystem bungeeSystem = component.getPlanSystem(); - try { - PlanConfig config = bungeeSystem.getConfigSystem().getConfig(); - config.set(WebserverSettings.PORT, TEST_PORT_NUMBER); - config.set(ProxySettings.IP, "8.8.8.8"); + PlanSystem bungeeSystem = component.getPlanSystem(); + try { + PlanConfig config = bungeeSystem.getConfigSystem().getConfig(); + config.set(WebserverSettings.PORT, TEST_PORT_NUMBER); + config.set(ProxySettings.IP, "8.8.8.8"); - bungeeSystem.enable(); // Throws EnableException - } finally { - bungeeSystem.disable(); - } - }); + assertThrows(EnableException.class, bungeeSystem::enable); + } finally { + bungeeSystem.disable(); + } } @Test - void testEnableWithMySQL() throws Exception { + void testEnableWithMySQL() { PlanSystem bungeeSystem = component.getPlanSystem(); try { PlanConfig config = bungeeSystem.getConfigSystem().getConfig(); diff --git a/Plan/bungeecord/src/test/java/com/djrapitops/plan/BungeeSystemTestDependencies.java b/Plan/bungeecord/src/test/java/com/djrapitops/plan/BungeeSystemTestDependencies.java new file mode 100644 index 000000000..07eed5014 --- /dev/null +++ b/Plan/bungeecord/src/test/java/com/djrapitops/plan/BungeeSystemTestDependencies.java @@ -0,0 +1,48 @@ +/* + * This file is part of Player Analytics (Plan). + * + * Plan is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License v3 as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Plan is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Plan. If not, see . + */ +package com.djrapitops.plan; + +import com.djrapitops.plan.settings.config.PlanConfig; +import com.djrapitops.plan.storage.database.DBSystem; +import utilities.DBPreparer; + +public class BungeeSystemTestDependencies implements DBPreparer.Dependencies { + + private final PlanSystem system; + + public BungeeSystemTestDependencies(PlanSystem system) { + this.system = system; + } + + @Override + public PlanConfig config() { + return system.getConfigSystem().getConfig(); + } + + @Override + public DBSystem dbSystem() { + return system.getDatabaseSystem(); + } + + @Override + public void enable() { + } + + @Override + public void disable() { + } +} diff --git a/Plan/bungeecord/src/test/java/utilities/mocks/BungeeMockComponent.java b/Plan/bungeecord/src/test/java/utilities/mocks/BungeeMockComponent.java index 4e0040e20..73690a839 100644 --- a/Plan/bungeecord/src/test/java/utilities/mocks/BungeeMockComponent.java +++ b/Plan/bungeecord/src/test/java/utilities/mocks/BungeeMockComponent.java @@ -41,7 +41,7 @@ public class BungeeMockComponent { SQLDB.setDownloadDriver(false); } - public PlanBungee getPlanMock() throws Exception { + public PlanBungee getPlanMock() { if (planMock == null) { planMock = PlanBungeeMocker.setUp() .withDataFolder(tempDir.toFile()) @@ -53,7 +53,7 @@ public class BungeeMockComponent { return planMock; } - public PlanSystem getPlanSystem() throws Exception { + public PlanSystem getPlanSystem() { if (component == null) { PlanBungee planMock = getPlanMock(); component = DaggerPlanBungeeComponent.builder() diff --git a/Plan/bungeecord/src/test/java/utilities/mocks/PlanBungeeMocker.java b/Plan/bungeecord/src/test/java/utilities/mocks/PlanBungeeMocker.java index 37fff4719..b2ed8a7d9 100644 --- a/Plan/bungeecord/src/test/java/utilities/mocks/PlanBungeeMocker.java +++ b/Plan/bungeecord/src/test/java/utilities/mocks/PlanBungeeMocker.java @@ -67,7 +67,7 @@ public class PlanBungeeMocker extends Mocker { return this; } - PlanBungeeMocker withResourceFetchingFromJar() throws Exception { + PlanBungeeMocker withResourceFetchingFromJar() { return this; } diff --git a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/sql/tables/MetadataTable.java b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/sql/tables/MetadataTable.java deleted file mode 100644 index 40c13ea4b..000000000 --- a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/sql/tables/MetadataTable.java +++ /dev/null @@ -1,83 +0,0 @@ -/* - * This file is part of Player Analytics (Plan). - * - * Plan is free software: you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License v3 as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * Plan is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with Plan. If not, see . - */ -package com.djrapitops.plan.storage.database.sql.tables; - -import com.djrapitops.plan.storage.database.DBType; -import com.djrapitops.plan.storage.database.queries.Query; -import com.djrapitops.plan.storage.database.queries.QueryStatement; -import com.djrapitops.plan.storage.database.sql.building.CreateTableBuilder; -import com.djrapitops.plan.storage.database.sql.building.Sql; -import com.djrapitops.plan.storage.database.transactions.ExecStatement; -import com.djrapitops.plan.storage.database.transactions.Executable; - -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.sql.SQLException; - -import static com.djrapitops.plan.storage.database.sql.building.Sql.*; - -public class MetadataTable { - - public static final String TABLE_NAME = "plan_database_metadata"; - - public static final String KEY = "id"; - public static final String VALUE = "uuid"; - public static final String INSERT_STATEMENT = "INSERT INTO " + TABLE_NAME + " (" + - KEY + ',' + - VALUE + - ") VALUES (?, ?)"; - - public static final String UPDATE_STATEMENT = "UPDATE " + TABLE_NAME + " SET " + VALUE + "=?" + - WHERE + KEY + "=?"; - - public static final String SELECT_VALUE_OF_KEY = SELECT + VALUE + FROM + TABLE_NAME + WHERE + KEY + "=?"; - - private MetadataTable() { - /* Static information class */ - } - - public static Executable insertValue(String key, String value) { - return new ExecStatement(MetadataTable.INSERT_STATEMENT) { - @Override - public void prepare(PreparedStatement statement) throws SQLException { - statement.setString(1, key); - statement.setString(2, value); - } - }; - } - - public static Query getValueOrNull(String key) { - return new QueryStatement(MetadataTable.SELECT_VALUE_OF_KEY) { - @Override - public void prepare(PreparedStatement statement) throws SQLException { - statement.setString(1, key); - } - - @Override - public String processResults(ResultSet set) throws SQLException { - return set.next() ? set.getString(MetadataTable.VALUE) : null; - } - }; - } - - public static String createTableSQL(DBType dbType) { - return CreateTableBuilder.create(TABLE_NAME, dbType) - .column(KEY, Sql.varchar(36)).notNull() - .column(VALUE, Sql.varchar(75)).notNull() - .toString(); - } -} diff --git a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/transactions/commands/RemoveEverythingTransaction.java b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/transactions/commands/RemoveEverythingTransaction.java index 4fe81d757..cef9f3786 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/transactions/commands/RemoveEverythingTransaction.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/transactions/commands/RemoveEverythingTransaction.java @@ -17,21 +17,22 @@ package com.djrapitops.plan.storage.database.transactions.commands; import com.djrapitops.plan.storage.database.sql.tables.*; -import com.djrapitops.plan.storage.database.transactions.ThrowawayTransaction; - -import static com.djrapitops.plan.storage.database.sql.building.Sql.DELETE_FROM; +import com.djrapitops.plan.storage.database.transactions.patches.Patch; /** * Transaction that removes everything from the database. * * @author AuroraLS3 */ -public class RemoveEverythingTransaction extends ThrowawayTransaction { +public class RemoveEverythingTransaction extends Patch { @Override - protected void performOperations() { - // Delete statements are run in a specific order as some tables have foreign keys, - // or had at some point in the past. + public boolean hasBeenApplied() { + return false; + } + + @Override + protected void applyPatch() { clearTable(SettingsTable.TABLE_NAME); clearTable(GeoInfoTable.TABLE_NAME); clearTable(NicknamesTable.TABLE_NAME); @@ -59,6 +60,6 @@ public class RemoveEverythingTransaction extends ThrowawayTransaction { } private void clearTable(String tableName) { - execute(DELETE_FROM + tableName); + execute("DELETE FROM " + tableName); } } \ No newline at end of file diff --git a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/DBPatchMySQLRegressionTest.java b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/DBPatchMySQLRegressionTest.java index 58b4fa874..a23347841 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/DBPatchMySQLRegressionTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/DBPatchMySQLRegressionTest.java @@ -16,7 +16,6 @@ */ package com.djrapitops.plan.storage.database; -import com.djrapitops.plan.PlanSystem; import com.djrapitops.plan.delivery.domain.container.PlayerContainer; import com.djrapitops.plan.delivery.domain.keys.PlayerKeys; import com.djrapitops.plan.storage.database.queries.containers.ContainerFetchQueries; @@ -64,16 +63,19 @@ class DBPatchMySQLRegressionTest extends DBPatchRegressionTest { private final String transferTable = "CREATE TABLE IF NOT EXISTS plan_transfer (sender_server_id integer NOT NULL, expiry_date bigint NOT NULL DEFAULT 0, type varchar(100) NOT NULL, extra_variables varchar(255) DEFAULT '', content_64 MEDIUMTEXT, part bigint NOT NULL DEFAULT 0, FOREIGN KEY(sender_server_id) REFERENCES plan_servers(id))"; private MySQLDB underTest; + private static DBPreparer dbPreparer; @BeforeAll static void ensureMySQLAvailable(@TempDir Path tempDir) { assumeTrue(System.getenv(CIProperties.MYSQL_DATABASE) != null); - component = new PluginMockComponent(tempDir); + dbPreparer = new DBPreparer(DaggerDatabaseTestComponent.builder() + .bindTemporaryDirectory(tempDir) + .build(), TEST_PORT_NUMBER); } @AfterAll - static void closeSystem() throws Exception { - if (component != null) component.getPlanSystem().disable(); + static void closeSystem() { + if (dbPreparer != null) dbPreparer.tearDown(); } private void dropAllTables() { @@ -89,9 +91,8 @@ class DBPatchMySQLRegressionTest extends DBPatchRegressionTest { } @BeforeEach - void setUpDBWithOldSchema() throws Exception { - PlanSystem system = component.getPlanSystem(); - Optional db = new DBPreparer(system, TEST_PORT_NUMBER).prepareMySQL(); + void setUpDBWithOldSchema() { + Optional db = dbPreparer.prepareMySQL(); assumeTrue(db.isPresent()); underTest = (MySQLDB) db.get(); diff --git a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/DatabaseTest.java b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/DatabaseTest.java index 5e8d15658..05aecf85b 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/DatabaseTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/DatabaseTest.java @@ -37,6 +37,7 @@ import com.djrapitops.plan.storage.database.queries.objects.playertable.NetworkT import com.djrapitops.plan.storage.database.queries.objects.playertable.ServerTablePlayersQuery; import com.djrapitops.plan.storage.database.sql.building.Sql; import com.djrapitops.plan.storage.database.sql.tables.UserInfoTable; +import com.djrapitops.plan.storage.database.sql.tables.UsersTable; import com.djrapitops.plan.storage.database.transactions.StoreConfigTransaction; import com.djrapitops.plan.storage.database.transactions.Transaction; import com.djrapitops.plan.storage.database.transactions.commands.RemovePlayerTransaction; @@ -56,8 +57,7 @@ import java.sql.SQLException; import java.util.*; import java.util.concurrent.TimeUnit; -import static com.djrapitops.plan.storage.database.sql.building.Sql.SELECT; -import static com.djrapitops.plan.storage.database.sql.building.Sql.WHERE; +import static com.djrapitops.plan.storage.database.sql.building.Sql.*; import static org.junit.jupiter.api.Assertions.*; /** @@ -255,7 +255,7 @@ public interface DatabaseTest extends DatabaseTestPreparer { } @Test - @Disabled + @Disabled("Flaky test, fails every evening.") default void sqlDateParsingSanitySQLDoesNotApplyTimezone() { Database db = db(); @@ -292,7 +292,8 @@ public interface DatabaseTest extends DatabaseTestPreparer { , new Transaction() { @Override protected void performOperations() { - execute("UPDATE " + UserInfoTable.TABLE_NAME + " SET " + UserInfoTable.REGISTERED + "=0" + WHERE + UserInfoTable.USER_ID + "=1"); + execute("UPDATE " + UserInfoTable.TABLE_NAME + " SET " + UserInfoTable.REGISTERED + "=0" + + WHERE + UserInfoTable.USER_ID + "=(" + SELECT + "MAX(" + UsersTable.ID + ")" + FROM + UsersTable.TABLE_NAME + ")"); } } ); diff --git a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/MySQLTest.java b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/MySQLTest.java index 0cb424e52..0657decec 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/MySQLTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/MySQLTest.java @@ -29,7 +29,6 @@ import com.djrapitops.plan.storage.database.queries.filter.QueryFilters; import com.djrapitops.plan.storage.database.transactions.StoreServerInformationTransaction; import com.djrapitops.plan.storage.database.transactions.commands.RemoveEverythingTransaction; import com.djrapitops.plan.storage.database.transactions.init.CreateTablesTransaction; -import com.djrapitops.plan.storage.database.transactions.patches.Patch; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; @@ -78,27 +77,12 @@ class MySQLTest implements DatabaseTest, QueriesTestAggregate { Optional mysql = preparer.prepareMySQL(); Assumptions.assumeTrue(mysql.isPresent()); database = mysql.get(); + database.executeTransaction(new CreateTablesTransaction()); } @BeforeEach void setUp() { TestErrorLogger.throwErrors(true); - db().executeTransaction(new Patch() { - @Override - public boolean hasBeenApplied() { - return false; - } - - @Override - public void applyPatch() { - dropTable("plan_world_times"); - dropTable("plan_kills"); - dropTable("plan_sessions"); - dropTable("plan_worlds"); - dropTable("plan_users"); - } - }); - db().executeTransaction(new CreateTablesTransaction()); db().executeTransaction(new RemoveEverythingTransaction()); db().executeTransaction(new StoreServerInformationTransaction(new Server(serverUUID(), TestConstants.SERVER_NAME, "", TestConstants.VERSION))); @@ -107,6 +91,7 @@ class MySQLTest implements DatabaseTest, QueriesTestAggregate { @AfterAll static void disableSystem() { + preparer.prepareMySQL().ifPresent(Database::close); if (database != null) database.close(); preparer.tearDown(); } diff --git a/Plan/common/src/test/java/utilities/DBPreparer.java b/Plan/common/src/test/java/utilities/DBPreparer.java index 0c9334943..90466c2f3 100644 --- a/Plan/common/src/test/java/utilities/DBPreparer.java +++ b/Plan/common/src/test/java/utilities/DBPreparer.java @@ -16,7 +16,6 @@ */ package utilities; -import com.djrapitops.plan.PlanSystem; import com.djrapitops.plan.SubSystem; import com.djrapitops.plan.settings.config.PlanConfig; import com.djrapitops.plan.settings.config.paths.DatabaseSettings; @@ -35,10 +34,6 @@ public class DBPreparer { private final Dependencies dependencies; private final int testPortNumber; - public DBPreparer(PlanSystem system, int testPortNumber) { - this(new PlanSystemAsDependencies(system), testPortNumber); - } - public DBPreparer(Dependencies dependencies, int testPortNumber) { this.dependencies = dependencies; this.testPortNumber = testPortNumber; @@ -91,6 +86,9 @@ public class DBPreparer { mysql.executeTransaction(new Transaction() { @Override protected void performOperations() { + execute("SET GLOBAL innodb_file_per_table=0"); + execute("SET GLOBAL innodb_fast_shutdown=2"); + execute("DROP DATABASE " + formattedDatabase); execute("CREATE DATABASE " + formattedDatabase); execute("USE " + formattedDatabase); @@ -110,33 +108,4 @@ public class DBPreparer { DBSystem dbSystem(); } - - @Deprecated - static class PlanSystemAsDependencies implements Dependencies { - private final PlanSystem system; - - PlanSystemAsDependencies(PlanSystem system) { - this.system = system; - } - - @Override - public void enable() { - system.enable(); - } - - @Override - public void disable() { - system.disable(); - } - - @Override - public PlanConfig config() { - return system.getConfigSystem().getConfig(); - } - - @Override - public DBSystem dbSystem() { - return system.getDatabaseSystem(); - } - } } diff --git a/Plan/config/checkstyle/checkstyle.xml b/Plan/config/checkstyle/checkstyle.xml index 0f4117fbd..2a65283bd 100644 --- a/Plan/config/checkstyle/checkstyle.xml +++ b/Plan/config/checkstyle/checkstyle.xml @@ -10,6 +10,13 @@ + + + + + + @@ -69,8 +76,8 @@ - - + +