paupino / psqlpack

A logical database management system for PostgreSQL enabling incremental database deployment.
Apache License 2.0
8 stars 2 forks source link

Introduction of basic integration testing infrastructure for ensuring correct generation #86

Closed paupino closed 6 years ago

paupino commented 6 years ago

Overview

This PR introduces the concept of an integration testing framework which also includes some minor refactoring to allow for future testing without the reliance of a file system. All in all, the introduced test is relatively useless (i.e. creates a database that doesn't exist with a single table) however does start to cover some of the integration testing points and consequently fixes a couple of bugs.

This is in reference to issue #84.

Details

Refactored operation.rs out of psqlpack and into the CLI.

The original reason for operation.rs in psqlpack was to avoid leaking internal implementation out of the library and instead enforce an opinionated approach to the use of this library. The downside of this was that psqlpack was responsible for all possible uses of this library. Since operation.rs was very tied to the file system (i.e. instead of streams etc) the easiest way forward was to move this into the CLI and retain the operation syntax that we'd already been leveraging. This worked out pretty well as the changes were quite minor - i.e. exposing some internal types and leveraging them from the CLI.

The only "weird" thing from all of this is the error handling change for extract. Rather than pull in error-chain just for this one use, I instead just "reimplement" the equivalent bail macro. I didn't particularly want to bring error-chain over in case we want to migrate to failure in the future.

Exposed internal AST

For integration tests to build a package representation without a DSL I needed to also expose the AST objects. To ensure that these continue to be namespaced, they're exposed under the logical mod ast. This will prevent the documentation becoming convoluted as well as logically separating the common scenario's (i.e. enabling the opinionated use of the library).

Updated to be able to generate columns correctly on apply

One major thing I change is that for new databases we use the same generation path as existing databases within delta.rs. This is an important change as it means that it is easier to diagnose issues and ensure similar generation paths. The downside is that it is slightly unoptimal since we know the database objects don't exist. I think this can be a future optimization (if need be) once we have reached 1.0.

The other change in delta was to terminate connections before database dropping. Because the tests run as postgres I found there was often connection collusion - this helps prevent this and allows us to drop the database easily. This could be specified by a publish profile in the future if it's deemed a problem however dropping the database is pretty extreme already so I'm not sure the added check is necessary.

Update package to be able to load columns from a connection

Previously, this was a big "TODO" in the project. After this PR, it still has a "TODO" about it however it is a lot closer to being usable - columns are now loaded.

Tables are now generated mutably with the columns added afterwards - which ultimately is how the AST is structured. The column SQL is quite complex however in a nutshell:

This ultimately required exposing the parser function parse_sql_type so we ultimately reuse existing lalrpop logic for parsing types.

Other changes