[DAT-11778] added check for autocommit, unignored tests (#368)

* added check for autocommit, unignored tests
liquibase · Oct 21, 2022 · c7a9508 · c7a9508
1 parent 44d62a8
commit c7a9508
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 75 deletions.
diff --git a/.github/workflows/aws.yml b/.github/workflows/aws.yml
@@ -110,7 +110,7 @@ jobs:
           changeLogFile: "mssql.sql"
           username: "${{secrets.TH_DB_ADMIN}}"
           password: "${{secrets.TH_DB_PASSWD}}"
-          url: ${{ secrets.TH_MSSQLURL }}
+          url: "${{ secrets.TH_MSSQLURL }}"
 
       - uses: liquibase/liquibase-github-action@v7
         if: ${{ matrix.database == 'postgresql' && matrix.version == 'aurora' }}
@@ -134,7 +134,7 @@ jobs:
         if: ${{ matrix.database == 'postgresql' && matrix.version != 'aurora' }}
         env:
           LIQUIBASE_PRO_LICENSE_KEY: ${{ secrets.LICENSE_KEY }}
-        run: mvn -Dtest=LiquibaseHarnessSuiteTest -DconfigFile=/harness-config-cloud.yml -DdbName=${{ matrix.database }} -DdbVersion=${{ matrix.version }} -Dprefix=aws -DdbUsername=${{secrets.TH_DB_ADMIN}} -DdbPassword=${{secrets.TH_DB_PASSWD}} -DdbUrl='${{ secrets[format('TH_PGRESURL_{0}', matrix.version)] }}' -Dmaven.test.failure.ignore=true test
+        run: mvn -Dtest=LiquibaseHarnessSuiteTest -DconfigFile=/harness-config-cloud.yml -DdbName=${{ matrix.database }} -DdbVersion=${{ matrix.version }} -Dprefix=aws -DdbUsername=${{secrets.TH_DB_ADMIN}} -DdbPassword=${{secrets.TH_DB_PASSWD}} -DdbUrl='${{ secrets[format('TH_PGRESURL_{0}', matrix.version)] }}' test
 
       - name: AWS RDS ${{ matrix.database }}-${{ matrix.version }} Test Run
         if: ${{ matrix.database == 'oracle' }}
@@ -152,25 +152,25 @@ jobs:
         if: ${{ matrix.database == 'mysql' && matrix.version == 'aws' }}
         env:
           LIQUIBASE_PRO_LICENSE_KEY: ${{ secrets.LICENSE_KEY }}
-        run: mvn -Dtest=LiquibaseHarnessSuiteTest -DconfigFile=/harness-config-cloud.yml -DdbName=${{ matrix.database }} -DdbVersion=${{ matrix.version }} -DdbUsername=${{secrets.TH_DB_ADMIN}} -DdbPassword=${{secrets.TH_DB_PASSWD}} -DdbUrl='${{ secrets.TH_MYSQLURL_8_0 }}' -Dmaven.test.failure.ignore=true test
+        run: mvn -Dtest=LiquibaseHarnessSuiteTest -DconfigFile=/harness-config-cloud.yml -DdbName=${{ matrix.database }} -DdbVersion=${{ matrix.version }} -DdbUsername=${{secrets.TH_DB_ADMIN}} -DdbPassword=${{secrets.TH_DB_PASSWD}} -DdbUrl='${{ secrets.TH_MYSQLURL_8_0 }}' test
 
       - name: AWS RDS ${{ matrix.database }}-${{ matrix.version }} Test Run
         if: ${{ matrix.database == 'mssql' }}
         env:
           LIQUIBASE_PRO_LICENSE_KEY: ${{ secrets.LICENSE_KEY }}
-        run: mvn -Dtest=LiquibaseHarnessSuiteTest -DconfigFile=/harness-config-cloud.yml -DdbName=${{ matrix.database }} -DdbVersion=${{ matrix.version }} -DdbUsername=${{secrets.TH_DB_ADMIN}} -DdbPassword=${{secrets.TH_DB_PASSWD}} -DdbUrl='${{ secrets.TH_MSSQLURL }}'  test
+        run: mvn -Dtest=LiquibaseHarnessSuiteTest -DconfigFile=/harness-config-cloud.yml -DdbName=${{ matrix.database }} -DdbVersion=${{ matrix.version }} -DdbUsername=${{secrets.TH_DB_ADMIN}} -DdbPassword=${{secrets.TH_DB_PASSWD}} -DdbUrl='${{ secrets.TH_MSSQLURL }}' test
 
       - name: AWS Aurora ${{ matrix.database }} Test Run
         if: ${{ matrix.database == 'mysql' && matrix.version == 'aurora' }}
         env:
           LIQUIBASE_PRO_LICENSE_KEY: ${{ secrets.LICENSE_KEY }}
-        run: mvn -Dtest=LiquibaseHarnessSuiteTest -DconfigFile=/harness-config-cloud.yml -DdbName=${{ matrix.database }} -DdbVersion=${{ matrix.version }} -DdbUsername=${{secrets.TH_DB_ADMIN}} -DdbPassword=${{secrets.TH_DB_PASSWD}} -DdbUrl='${{ secrets.TH_AURORA_MYSQLURL }}' -Dmaven.test.failure.ignore=true test
+        run: mvn -Dtest=LiquibaseHarnessSuiteTest -DconfigFile=/harness-config-cloud.yml -DdbName=${{ matrix.database }} -DdbVersion=${{ matrix.version }} -DdbUsername=${{secrets.TH_DB_ADMIN}} -DdbPassword=${{secrets.TH_DB_PASSWD}} -DdbUrl='${{ secrets.TH_AURORA_MYSQLURL }}' test
 
       - name: AWS Aurora ${{ matrix.database }} Test Run
         if: ${{ matrix.database == 'postgresql' && matrix.version == 'aurora' }}
         env:
           LIQUIBASE_PRO_LICENSE_KEY: ${{ secrets.LICENSE_KEY }}
-        run: mvn -Dtest=LiquibaseHarnessSuiteTest -DconfigFile=/harness-config-cloud.yml -DdbName=${{ matrix.database }} -DdbVersion=13 -Dprefix=aurora -DdbUsername=${{secrets.TH_DB_ADMIN}} -DdbPassword=${{secrets.TH_DB_PASSWD}} -DdbUrl='${{ secrets.TH_AURORA_POSTGRESQLURL }}' -Dmaven.test.failure.ignore=true test
+        run: mvn -Dtest=LiquibaseHarnessSuiteTest -DconfigFile=/harness-config-cloud.yml -DdbName=${{ matrix.database }} -DdbVersion=13 -Dprefix=aurora -DdbUsername=${{secrets.TH_DB_ADMIN}} -DdbPassword=${{secrets.TH_DB_PASSWD}} -DdbUrl='${{ secrets.TH_AURORA_POSTGRESQLURL }}' test
 
       - name: Archive AWS RDS ${{ matrix.database }}-${{ matrix.version }} Test Results
         uses: actions/upload-artifact@v3

diff --git a/src/main/groovy/liquibase/harness/compatibility/basic/BasicCompatibilityTest.groovy b/src/main/groovy/liquibase/harness/compatibility/basic/BasicCompatibilityTest.groovy
@@ -12,6 +12,7 @@ import org.skyscreamer.jsonassert.JSONCompareMode
 import spock.lang.Shared
 import spock.lang.Specification
 import spock.lang.Unroll
+import java.sql.Connection
 import java.sql.DriverManager
 import java.sql.ResultSet
 import java.sql.SQLException
@@ -99,27 +100,26 @@ class BasicCompatibilityTest extends Specification {
 
         then: "execute metadata checking sql, obtain result set, compare it to expected result set"
         JSONArray generatedResultSetArray
+        Connection newConnection
         try {
             ResultSet resultSet
             if (connection.isClosed()) {
-                connection = DriverManager.getConnection(testInput.url, testInput.username, testInput.password)
-                resultSet = connection.createStatement().executeQuery("SELECT * FROM DATABASECHANGELOG")
-                generatedResultSetArray = mapResultSetToJSONArray(resultSet)
-                if (!connection.autoCommit) {
-                    connection.commit()
-                }
+                newConnection = DriverManager.getConnection(testInput.url, testInput.username, testInput.password)
+                resultSet = newConnection.createStatement().executeQuery("SELECT * FROM DATABASECHANGELOG")
             } else {
                 resultSet = ((JdbcConnection) connection).createStatement().executeQuery("SELECT * FROM DATABASECHANGELOG")
-                generatedResultSetArray = mapResultSetToJSONArray(resultSet)
-                if (!connection.autoCommit) {
-                    connection.commit()
-                }
+                connection.autoCommit ?: connection.commit()
             }
+            generatedResultSetArray = mapResultSetToJSONArray(resultSet)
+
             def expectedResultSetArray = new JSONObject(expectedResultSet).getJSONArray(testInput.change)
             assert compareJSONArrays(generatedResultSetArray, expectedResultSetArray, JSONCompareMode.LENIENT)
         } catch (Exception exception) {
             Scope.getCurrentScope().getUI().sendMessage("Error executing metadata checking sql! " + exception.printStackTrace())
             Assert.fail exception.message
+        } finally {
+            newConnection == null ?: newConnection.close()
+
         }
 
         and: "check for actual presence of created object"
@@ -136,7 +136,7 @@ class BasicCompatibilityTest extends Specification {
 
         cleanup: "rollback changes if we ran changeSet"
         if (shouldRunChangeSet) {
-            for (int i = 0; i < changelogList.size(); i++) {
+            for (int i = 0; i < changelogList.size(); i++) {//TODO rethink rollback logic to do it only once
                 argsMap.put("changeLogFile", changelogList.get(i))
                 strategy.performRollback(argsMap)
             }
@@ -154,6 +154,7 @@ class BasicCompatibilityTest extends Specification {
                         Assert.fail()
                     }
                 } catch (ignored) {
+                    (connection.isClosed() || connection.autoCommit) ?: connection.commit()
                     // Assume test object does not exist and 'rollback' was successful. Ignore exception.
                     Scope.getCurrentScope().getUI().sendMessage("Rollback was successful. Removed object was not found.")
                 }

diff --git a/src/main/groovy/liquibase/harness/compatibility/basic/BasicCompatibilityTestHelper.groovy b/src/main/groovy/liquibase/harness/compatibility/basic/BasicCompatibilityTestHelper.groovy
@@ -53,27 +53,16 @@ class BasicCompatibilityTestHelper {
     static ResultSet executeQuery(String pathToSql, TestInput testInput) throws SQLException {
         Connection newConnection
         ResultSet resultSet
-        try {
-            if (testInput.database.connection.isClosed()) {
-                newConnection = DriverManager.getConnection(testInput.url, testInput.username, testInput.password)
-                resultSet = newConnection.createStatement().executeQuery(pathToSql)
-                if (!newConnection.autoCommit) {
-                    newConnection.commit()
-                }
-                return resultSet
-            } else {
-                JdbcConnection connection = (JdbcConnection) testInput.database.connection
-                resultSet = connection.createStatement().executeQuery(pathToSql)
-                if (!testInput.database.connection.autoCommit) {
-                    testInput.database.connection.commit()
-                }
-                return resultSet
-            }
-        } finally {
-            if (newConnection != null) {
-                newConnection.close()
-            }
+        if (testInput.database.connection.isClosed()) {
+            newConnection = DriverManager.getConnection(testInput.url, testInput.username, testInput.password)
+            resultSet = newConnection.createStatement().executeQuery(pathToSql)
+            newConnection.close()
+        } else {
+            JdbcConnection connection = (JdbcConnection) testInput.database.connection
+            resultSet = connection.createStatement().executeQuery(pathToSql)
+            testInput.database.connection.autoCommit ?: testInput.database.connection.commit()
         }
+        return resultSet
     }
 
     @Builder

diff --git a/src/main/groovy/liquibase/harness/data/ChangeDataTests.groovy b/src/main/groovy/liquibase/harness/data/ChangeDataTests.groovy
@@ -1,10 +1,12 @@
 package liquibase.harness.data
 
 import liquibase.Scope
+import liquibase.database.DatabaseFactory
 import liquibase.database.jvm.JdbcConnection
 import liquibase.harness.config.DatabaseUnderTest
 import liquibase.harness.config.TestConfig
 import liquibase.harness.util.rollback.RollbackStrategy
+import liquibase.resource.ClassLoaderResourceAccessor
 import org.json.JSONArray
 import org.json.JSONObject
 import org.junit.Assert
@@ -13,14 +15,18 @@ import org.skyscreamer.jsonassert.JSONCompareMode
 import spock.lang.Shared
 import spock.lang.Specification
 import spock.lang.Unroll
+
 import java.sql.Connection
 import java.sql.DriverManager
 import java.sql.ResultSet
 
-import static liquibase.harness.util.JSONUtils.*
-import static liquibase.harness.util.FileUtils.*
+import static liquibase.harness.data.ChangeDataTestHelper.buildTestInput
+import static liquibase.harness.data.ChangeDataTestHelper.saveAsExpectedSql
+import static liquibase.harness.util.FileUtils.getJSONFileContent
+import static liquibase.harness.util.FileUtils.getSqlFileContent
+import static liquibase.harness.util.JSONUtils.compareJSONArrays
+import static liquibase.harness.util.JSONUtils.mapResultSetToJSONArray
 import static liquibase.harness.util.TestUtils.*
-import static liquibase.harness.data.ChangeDataTestHelper.*
 
 class ChangeDataTests extends Specification {
     @Shared
@@ -63,9 +69,9 @@ class ChangeDataTests extends Specification {
         assert shouldRunChangeSet: "No checkingSql for ${testInput.changeData}!"
 
         and: "check database under test is online"
-        def connection = testInput.database.getConnection()
-        shouldRunChangeSet = connection instanceof JdbcConnection
+        shouldRunChangeSet = testInput.database.getConnection() instanceof JdbcConnection
         assert shouldRunChangeSet: "Database ${testInput.databaseName} ${testInput.version} is offline!"
+        JdbcConnection connection = testInput.database.getConnection() as JdbcConnection
 
         when: "get sql generated for the change set"
         def generatedSql = parseQuery(executeCommandScope("updateSql", argsMap).toString())
@@ -93,26 +99,27 @@ class ChangeDataTests extends Specification {
         ResultSet resultSet
         JSONArray generatedResultSetArray
         try {
-            if (connection.isClosed()) {
+            //For embedded databases, let's create separate connection to run checking SQL
+            if (connection.isClosed()||connection.getDatabaseProductName().equalsIgnoreCase("sqlite")) {
                 newConnection = DriverManager.getConnection(testInput.url, testInput.username, testInput.password)
                 resultSet = newConnection.createStatement().executeQuery(checkingSql)
-                generatedResultSetArray = mapResultSetToJSONArray(resultSet)
-                newConnection.commit()
             } else {
-                resultSet = ((JdbcConnection) connection).createStatement().executeQuery(checkingSql)
-                generatedResultSetArray = mapResultSetToJSONArray(resultSet)
-                connection.commit()
+                connection.close()
+                connection = DatabaseFactory.getInstance().openConnection(testInput.url, testInput.username, testInput.password,
+                        null, new ClassLoaderResourceAccessor()) as JdbcConnection
+                resultSet = connection.createStatement().executeQuery(checkingSql)
+                connection.autoCommit ?: connection.commit()
             }
+            generatedResultSetArray = mapResultSetToJSONArray(resultSet)
+
             def expectedResultSetJSON = new JSONObject(expectedResultSet)
             def expectedResultSetArray = expectedResultSetJSON.getJSONArray(testInput.getChangeData())
             assert compareJSONArrays(generatedResultSetArray, expectedResultSetArray, JSONCompareMode.NON_EXTENSIBLE)
         } catch (Exception exception) {
             Scope.getCurrentScope().getUI().sendMessage("Error executing checking sql! " + exception.printStackTrace())
             Assert.fail exception.message
         } finally {
-            if (newConnection != null) {
-                newConnection.close()
-            }
+            newConnection == null ?: newConnection.close()
         }
 
         and: "if expected sql is not provided save generated sql as expected sql"

diff --git a/src/main/groovy/liquibase/harness/snapshot/SnapshotObjectTestHelper.groovy b/src/main/groovy/liquibase/harness/snapshot/SnapshotObjectTestHelper.groovy
@@ -4,13 +4,13 @@ import groovy.transform.ToString
 import groovy.transform.builder.Builder
 import liquibase.Scope
 import liquibase.database.Database
+import liquibase.database.jvm.JdbcConnection
 import liquibase.harness.config.DatabaseUnderTest
 import liquibase.harness.config.TestConfig
 import liquibase.harness.util.DatabaseConnectionUtil
 import liquibase.harness.util.FileUtils
-import liquibase.statement.SqlStatement
-import liquibase.statement.core.RawSqlStatement
 import org.junit.Assert
+
 import java.sql.Connection
 import java.sql.DriverManager
 import java.util.logging.Logger
@@ -33,15 +33,15 @@ class SnapshotObjectTestHelper {
         Logger.getLogger(this.class.name).warning("Only " + TestConfig.instance.inputFormat
                 + " input files are taken into account for this test run")
         List<TestInput> inputList = new ArrayList<>()
-        for (DatabaseUnderTest databaseUnderTest: new DatabaseConnectionUtil()
+        for (DatabaseUnderTest databaseUnderTest : new DatabaseConnectionUtil()
                 .initializeDatabasesConnection(TestConfig.instance.getFilteredDatabasesUnderTest())) {
             for (def changeLogEntry : FileUtils.resolveInputFilePaths(databaseUnderTest, inputSqlPath, "sql")
                     .entrySet()) {
                 if (!commandLineSnapshotObjectList || commandLineSnapshotObjectList.contains(changeLogEntry.key)) {
                     String pathToCleanupSQL = FileUtils.resolveInputFilePaths(databaseUnderTest,
                             cleanupSqlPath, "sql").get(changeLogEntry.key)
                     String pathToExpectedSnapshot = FileUtils.resolveInputFilePaths(databaseUnderTest,
-                                    expectedSnapshotPath, "json").get(changeLogEntry.key)
+                            expectedSnapshotPath, "json").get(changeLogEntry.key)
                     inputList.add(TestInput.builder()
                             .database(databaseUnderTest)
                             .snapshotObjectName(changeLogEntry.key)
@@ -59,35 +59,29 @@ class SnapshotObjectTestHelper {
         String query = FileUtils.getResourceContent(pathToSql)
         Database database = testInput.database.database
         Connection newConnection
-        if (database.connection.isClosed()) {
-            try {
+        try {
+            if (database.connection.isClosed()) {
                 newConnection = DriverManager.getConnection(testInput.database.url, testInput.database.username,
                         testInput.database.password)
                 newConnection.createStatement().execute(query)
-                newConnection.commit()
-            } catch (Exception exception) {
-                Scope.getCurrentScope().getUI().sendMessage("Failed to execute query! " + query + " " +
-                        exception.printStackTrace())
-                Assert.fail exception.message
-            } finally {
-                if (newConnection != null) {
-                    newConnection.close()
-                }
+            } else {
+                JdbcConnection connection = testInput.database.database.getConnection() as JdbcConnection
+                connection.createStatement().execute(query)
+                connection.autoCommit ?: connection.commit()
             }
-        } else {
-            try {
-                database.execute([new RawSqlStatement(query)] as SqlStatement[], null)
-                database.commit()
-            } catch (Exception exception)  {
-                Scope.getCurrentScope().getUI().sendMessage("Failed to execute query! " + query + " " +
-                        exception.printStackTrace())
-                Assert.fail exception.message
+        } catch (Exception exception) {
+            Scope.getCurrentScope().getUI().sendMessage("Failed to execute query! " + query + " " +
+                    exception.printStackTrace())
+            Assert.fail exception.message
+        } finally {
+            if (newConnection != null) {
+                newConnection.close()
             }
         }
     }
 
     @Builder
-    @ToString(includeNames=true, includeFields=true, includePackage = false)
+    @ToString(includeNames = true, includeFields = true, includePackage = false, excludes ='database')
     static class TestInput {
         DatabaseUnderTest database
         String snapshotObjectName

diff --git a/src/main/resources/liquibase/harness/change/expectedSql/h2/sqlFile.sql b/src/main/resources/liquibase/harness/change/expectedSql/h2/sqlFile.sql
@@ -0,0 +1,4 @@
+CREATE TABLE PUBLIC.sqltest (id INT)
+insert into sqltest (id) values (1)
+insert into sqltest (id) values (2)
+insert into sqltest (id) values (3)