Integration tests for database migrations

[Mateusz Kwiatkowski]

Hi dev forum,

on last Tech Committee meeting I briefly talked about integration tests for database migrations. I've only quickly talked about the concept of test that we currently have. For reference here are all implemented tests for now. There is a base class that is responsible for managing testing migration and working together with flyway. Basically what those tests do is migrating db to last migration before one that we want to test, then test need to insert some data that will be tested, then we are executing the migration we want to test and verifying the result. Those checks as well as inserting data before migration are made using simple sql commands, but BaseMigrationIntegrationTest class provides basic methods to make it easier. Test classes for each migration are pretty simple, they need to implement 4 methods:
- getTargetBeforeTestMigration - here we need to specify timestamp of migration right before one that we are going to test, i.e. "20180129143106317",

- insertDataBeforeMigration - this method is executed after migrating db to previously defined migration and here we need to insert data that will be modified by migration,

- getTestMigrationTarget - should return timestamp of migration to be tested,

- verifyDataAfterMigration - this method will be executed after running migration specified in previous method, here we need extract info from db after migration and do some assertions

As I said concept of those tests is pretty simple, only 4 methods to implement, so as you can see current tests are rather short so new test should not require that much work since all foundations for testing migrations is done. I think that other positive thing is that those tests can verify some cases that will not be covered by simply checking if services are starting properly, i.e. this migration needed to be tested because it was failing for some reason and it is pretty complex, so in my opinion there is point of writing those tests for "big" migrations. On the other hand we have migration test job on Jenkins that is migrating last stable version to newest changes so it should cover most problems with migrations which makes those integration tests redundant in some way. Moreover those test have only value at the point we are introducing migration, later on we will not change migration code and we can just simply assume it is working and running tests for old migrations over and over becomes pointless. 

What are your thoughts? Do you think we should have more of those tests or they are not bringing enough value?

Best Regards,

Mateusz 

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

[Sebastian Brudziński]

I will just restate my opinion from when we were initially discussing this. In my mind, we should NOT be covering migrations with integration tests (as in not making it a mandatory requirement).

Database migrations, whether SQL or Java, are written once and should not be modified ever again. Having an automated test brings us no value here, since that test will be verifying a piece of code that never changes and that executes in the same environment with the exact same data. That means, if that test passes once, there's no possible way it ever breaks (unless you modify the migration, what you should not do). Having tests that never fail is not something we want and it makes build take longer (those tests need to be executed each time).

Having said that, I don't think we should remove the base migration testing class that we have or currently written migration tests. The base class may be useful for other developers who are writing new migrations and are looking for a quick way to gain confidence on whether the migration works as expected (especially if it's more complex, and not just adding/dropping a column).

As for existing migration tests, one idea would be to have a separate gradle profile that would only run migration tests. The dev working on a new migration could then still write new tests and run that profile on demand locally.

Pros:

 * The tests are in the codebase so the developer and code reviewer have better confidence the migration does what it's supposed to

 * Those tests are run on demand in a separate profile and therefore don't make ITs take longer

Any thoughts?

Kind regards,

Sebastian.

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAHq-FDMY0fc6dBrP60MAdy3p2sAowL%2BfV3RkPLq78--7UhAACg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--

Sebastian Brudziński
Technical Leader
sbrud...@soldevelo.com