Looking for 4.0 code review: Action/ActionLogic and testing

Liquibase Development
1

Looking for a quick code review/feedback on how the tests for 4.0 will work.

https://github.com/liquibase/liquibase/pull/340 is the pull request for most of the Action changes I've done with 4.0 so far, but it is getting a bit overwhelming already. Below is a more packaged few areas I'd be interested in feedback on.

The major refactoring for 4.0 (and what I’ve started on) is a simplifying and consolidating all the database (and other external interactions) down to:

  • - Action classes that describe a particular thing you want done (like AddPrimaryKeyAction or SnapshotDatabaseObjectsAction).
  • - ActionLogic classes take the Action class and transform it into something that can be executed. These classes are prioritized so you can have one implementation for mysql and another for oracle and a 3rd parth-specific implementation for oracle in an extension

These classes take the place of the Change, Statement, Generator, and Snapshot classes in 3.x.

Action Examples:

AddAutoIncrementAction: https://github.com/liquibase/liquibase/blob/4.0_action/liquibase-core/src/main/java/liquibase/action/core/AddAutoIncrementAction.java

SnapshotDatabaseObjectsAction: https://github.com/liquibase/liquibase/blob/4.0_action/liquibase-core/src/main/java/liquibase/action/core/SnapshotDatabaseObjectsAction.java

Notice that these are pure data holder objects, and so I purposely did not use get/set methods but instead just public fields. It does implement liquibase.ExtensibleObject which gives you a get(fieldName) and set(fieldName, value) interface as well as the ability for extensions to add additional properties without subclassing.

ActionLogic Examples:

AddAutoIncrementLogic: https://github.com/liquibase/liquibase/blob/4.0_action/liquibase-core/src/main/java/liquibase/actionlogic/core/AddAutoIncrementLogic.java

AddAutoIncrementLogicMysql: https://github.com/liquibase/liquibase/blob/4.0_action/liquibase-mysql/src/main/java/liquibase/actionlogic/core/mysql/AddAutoIncrementLogicMysql.java

SnapshotTablesLogicJdbc: https://github.com/liquibase/liquibase/blob/4.0_action/liquibase-core/src/main/java/liquibase/actionlogic/core/SnapshotTablesLogicJdbc.java

SnapshotTablesLogicOffline: https://github.com/liquibase/liquibase/blob/4.0_action/liquibase-core/src/main/java/liquibase/actionlogic/core/SnapshotTablesLogicOffline.java

Notice both AddAutoIncrementLogic and AddAutoIncrementLogicMysql handle AddAutoIncrementAction classes, but which is picked depends on the database type.

Similarly, both SnapshotTalbesLogicJdbc and SnapshotTablesLogicOffline both can handle SnapshotDatabaseObjectsAction but which is used depends on the connection on the database as well as the type of object in SnapshotDatabaseObjectsAction

You also see in AddAutoIncrementLogic*, how I am no longer just bulding raw Strings, but instead a smarter StringClauses class I created which lets me build up a standard SQL statement in the base class, and then replace, alter and/or move pieces much easier in database-specific Logic subclasses.

Unit Testing

So now that I have everything using a common Action/ActionLogic pattern, I need to test them. There is unit-test style tests that I do such as:

- AddAutoIncrementLogicTest https://github.com/liquibase/liquibase/blob/4.0_action/liquibase-core/src/test/groovy/liquibase/actionlogic/core/AddAutoIncrementLogicTest.groovy

- SnapshotTablesLogicJdbcTest https://github.com/liquibase/liquibase/blob/4.0_action/liquibase-core/src/test/groovy/liquibase/actionlogic/core/SnapshotTablesLogicJdbcTest.groovy

- SnapshotTablesLogicOfflineTest https://github.com/liquibase/liquibase/blob/4.0_action/liquibase-core/src/test/groovy/liquibase/actionlogic/core/SnapshotTablesLogicOfflineTest.groovy

- SnapshotColumnsLogicJdbcTest https://github.com/liquibase/liquibase/blob/4.0_action/liquibase-core/src/test/groovy/liquibase/actionlogic/core/SnapshotColumnsLogicJdbcTest.groovy

These tests work well for easily checking my standard “checkStatus()” method which does no database interaction (AddAutoIncrementLogicTest) or even test methods passing in example resultSets I’ve seen (SnapshotColumnsLogicJdbcTest). I’m using spock for these tests because spock is awesome.

Integration Testing

The meat of the logic in the Action/ActionLogic classes, however, is whether the SQL executed against the database by each ActionLogic is correct and does what it is supposed to do. And, we can’t really know if it works like it is supposed to without running it against the database. This is where TestMD comes in.

TestMD lets me write tests like this:

Looking at AddAutoIncrementActionTest.“Can apply standard settings to column name” as an example, you can see it is still using spock as the test framework because it is still awesome, but there are a couple things I’m doing differently:

  1. I’m using more of a generative testing pattern, where the “where” block isn’t just a static table, but instead calls a method which will build up all permutations of the items in each list passed to CollectionUtil.permutations. Within the test itself, the assertions are not hard-coded assertions but instead it looks at the permutation values and then checks what should be right based on those values.
  2. The checking of the permutation uses TestMD which lets me say “this permutation is defined by these three attributes, and what is ran against the database is this SQL plan”. When the test is ran successfully, it creates an “accepted.md” file which lists the SQL plan ran for each permutation. For future times it is ran, if the SQL has changed the test is re-ran to verify it still does what it expects. If the SQL has not changed, it is not re-validated.

Example accepted.md files:

which are nicely formatted by github as:

My hope for TestMD is that it gets us the “test everything against the live database” advantage from integration testing, but since without the sloooow performance of integration testing. When I run AddAutoIncrementActionTest against Mysql the first time, the 65 test iterations take 33s to run. Once I have ran run them once, I (or any other contributer) I just have to run “all tests” like normal and they take just milliseconds to run—unless the generated SQL changed. If the generated SQL is ever changed, TestMD will re-run the verification for just the changed permutations. If I (or a committer) doesn’t have the correct environment set up to test the permutation, it will be marked as “unverified” in the accepted.md file and when I see the pull request I’ll see it is unverified, I can look at the SQL changes to make sure they look right and/or run them with the correct environment to ensure they are correct.

My hope is that the combination of generative permutations + testmd will give much, much better test coverage than we’ve ever had before while allowing us to always run all tests and therefore catch all expected and unexpected changes in SQL.

So far I think that plan is panning out because I am definitely finding a lot of parameter permutations I’ve never really handled before just in the couple *ActionTests I’ve done so far. The biggest problem has been now needing to actually handle all those error cases.

Let me know what you think

Nathan