darcys22 / godbledger

Accounting Software with GRPC endpoints and SQL Backends
https://www.godbledger.com/
GNU General Public License v3.0
490 stars 58 forks source link

End to End Testing #49

Open darcys22 opened 4 years ago

darcys22 commented 4 years ago

I've stated compiling the files for there to be end to end testing of the system. Ideally before every change there should be an instance spun up with an appropriate database backend, a bunch of journals sent to the system via GRPC and the output checked.

This has been started in the /tests directory. Its passing right now but is essentially commented out

darcys22 commented 4 years ago

Partially implemented with #83

This only fires up an in memory sqlite database for godbledger. And also only a single test. To implement a test for the MySQL backend and load up with lots and lots of testing being sent to the server

darcys22 commented 4 years ago

This is ongoing, currently in the tests directory there is a go test that fires up an instance of GoDBLedger locally and then sends off a single test transaction to the instance. If there are any issues it fails.

The end goal is to get an end-to-end test suite where many concurrent instances are launched and many tests are fired in parallel. Defining these tests in a common format within a json file would allow for easy defining of tests. An example has been created in tests/testdata/BasicTests/add1.json

darcys22 commented 3 years ago

Made bulk updates to the integration tests, will now run multiple godbledger servers (3) and then runs a test in each one. These are not done in parallel currently and not sure how to make these do so yet.

In the /tests/ folder there is an /tests/evaluators folder containing a single file for each test. To add more to this one should add a new file with the test, then add to the array in end to end tests.

darcys22 commented 3 years ago

Todo items

darcys22 commented 3 years ago

e2e tests appear to be picking up configuration from my local installation (~/Library/ledger/config.toml) which is unexpected.

the tests would be safer (i.e. more isolated and predictable) if they don't depend on or interact with any installation we may have running in the same environment.

typically the way I have seen this achieved is to construct the tests in such a way that either (a) each test takes place in it's own sandboxed environment; or (b) a single sandboxed env is created for the test suite and reused (i.e. cleaned or reset before each individual test).

the current e2e tests are doing some isolation (using a different port for each godbledger server process) but are not fully isolated (without passing the datadir or config file args they are picking up my default config file)

that's the first thing I would look at adjusting, to complete the isolation of the test runs.

second thing that I notice is that before we get to running tests in parallel, the e2e suite seems to start up 6 servers on 6 ports and then run 6 tests; I am not sure how this will scale so we may want to adjust how these are run such that each test runs in it's own complete space (data dir, config file, with mysql a unique temp data base or data file) and perhaps is executed as a batch with a configurable number of parallel tests. in this way new tests would queue up behind that bottleneck without consuming more and more ports/memory

davidalpert commented 3 years ago

took another look at these integration tests. couple of observations:

I recommend that we update the components.StartGoDBLedger such that it manages the full lifecycle of the server component for a given test. the runEndToEndTest harness really only needs a few inputs (connection details to the test node, test data dir, test server host and port, test log file, etc). the signature of components.StartGoDBLedger could be modified something like this:

    func RunWithTestServer(t *testing.T, dbType string, index int, doWithServer func(*testing.T, string, *cmd.LedgerConfig)error) {
      // clean and create data dir for dbType and test index
      // invoke `cmd.MakeConfig` to build a configuration for the test data dir, dbType, and test index
      // update the config as needed for the test database (e.g. to point to a mysql database)
      // start the server and wait for it to be listening
      // invoke `doWithServer`
      // stop the server (e.g. kill the process)
      // if test passed {
      //   clean up test directory (including log file and any database files)
      // }
    }

The MySQL tests are interesting. The example using Github Actions referenced above (How to use a MySQL database on GitHub Actions) looks like it tells Github Actions to spin up a single MySQL database for an entire test run, which would not provide the same isolation between test evaluators as using a SQLite in-memory DB per test.

Perhaps the MySQL tests could be updated to spin up a MySQL database inside docker (when running locally) and tagged differently (e.g. with mysql) such that they only run locally and not on Github in Actions. In that way, the MySQL tests could still be run by anyone doing development who has docker installed to provide cross-compatibility feedback, while just the SQLite backend is tested in Github Actions?

davidalpert commented 3 years ago

as I've experimented with these changes a couple more observations:

darcys22 commented 3 years ago

Yeah the evaluator type could definitely be refined. The TransactorClient and transaction package is generated by GRPC though so its not really something we can add to. Its more that the test uses it as a client to talk to the server we spin up.

I'm like the idea of having separate packages for the integration tests because I wasn't able to create build tags that separates mysql from sqlite when they are all in a single test package. Potentially the database modules would be good for this because each database is essentially drawing from our integration test cases (Evaluators) and running them against their backend.

The config object is ugly as is right now, there is the so much repeated code. It passes flags to the server and also passes a config file which seems redundant. If there were separate packages this would probably be a single object passed to the tests package so it knows what to run, and that could be generated in the separate module.

davidalpert commented 3 years ago

I did some reading on using build tags to separate tests with different constraints.

The dev branch currently has the following build tag in the tests/mysql_test.go file:

// +build mysql

which does get ignored unless the go test --tags argument includes mysql

the tags appear to have conventions where commas are treated like AND and spaces are treated like OR so I think we could also use:

// +build integration,mysql

which would then run when the tags looked like this:

go test --tags integration,mysql

and not when they looked like this

go test --tags integration

so it looks like that part is actually working right now.

I'm running into a different issue on my machine:

secure_connection_test.go:130: Node Version request failed: rpc error: code = Unavailable desc = connection error: desc = "transport: authentication handshake failed: x509: certificate is valid for 127.0.0.1, not 0.0.0.0"

so I'm going to add a build tag there for the time being which separates each type of integration test

davidalpert commented 3 years ago

thinking about the sqlite and mysql tests this morning, I'm struck by the orchestration differences between the in-memory sqlite pattern and the out-of-process mysql database.

it seems that each backend may require its own approach to both setup and teardown.

for a sqlite in-memory db setup can look like:

  1. create a unique data dir
  2. build a config object pointing to that data dir
  3. start the server with an in-memory sqlite config pointing to that data dir

and teardown can look like:

  1. stop the server
  2. remove the unique data dir

but for a mysql database, let's say we spin up mysql in docker, are we spinning up and tearing down a new mysql database for each evaluation? maybe that makes sense, or maybe it makes sense to spin up a single mysql database container and run a set of tests against that backend, running db queries after each to clean them up?

where this takes me is that it may not make sense to force both backend orchestrations into a single common abstraction.

darcys22 commented 3 years ago

I'm thinking that there should be a common interface that can be implemented in both mysql and sqlite. It can maintain a pool of available servers that tests can be run using. Then we can queue our evaluators and it will grab a server from the pool if available, run the evaluator, cleanup and return the server to the pool. This way we can specify somewhere how many parallel servers we want to be able to support.

so the interface would have functions like:

init()
add_server()
queue_evaluator() 
run_tests()

and it is up to each instance on how the add_server (also how to cleanup and return the server to the pool after a test) works. That way we have a place for each database to define its own lifecycle. For the mysql one it might mean we up a new database container and point the godbledger instance at that container, closing it down could just be a drop tables, create new database and restart godbledger