Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

End to End Testing #49

Open
darcys22 opened this issue Jul 23, 2020 · 11 comments
Open

End to End Testing #49

darcys22 opened this issue Jul 23, 2020 · 11 comments

Comments

@darcys22
Copy link
Owner

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
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

Todo items

  • Run the tests a second time using the mysql database instead of just the sqlite memory database. Github actions should be able to handle this. Possible example here
  • Make the tests run in parallel

@darcys22
Copy link
Owner Author

darcys22 commented Feb 4, 2021

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
Copy link
Contributor

davidalpert commented Apr 18, 2021

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

  • the mysql tests are currently disabled (not calling the runEndToEndTest function);
  • the logic around managing the test server and running the tests is split between tests.TestSQLite(...) (invoking cmd.MakeConfig(...) which sets the ConfigFile and DatabaseLocation properties for the given test configuration), components.StartGoDBLedger(...) (starting a server process) and tests/runEndToEndTest(...) (killing processes and deleting files) so it's difficult to see where to inject logic to manage an isolated data dir per test
  • the list of test evaluators and supported databases defines a test matrix that is not clearly articulated in the current test code;

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
Copy link
Contributor

davidalpert commented May 2, 2021

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

  • the concept of evaluations is currently spread over three packages

    1. evaluators this package contains test scenarios
    2. types contains a single Evaluator type
    3. tests contains a list of the types in the evaluators package

    golang leans away from generic package names like type preferring to embed types/structs in packages where they are relevant.

    I plan to collect these three things together into the evaluators package

  • the concept of test cases or test runs is further spread over a few packages

    1. the list of evaluators represents the test cases we have against the godbledger server API
    2. the mysql and sqlite classes represent some sort of test context or test run or db config, a higher-order concept of supported backends, against which all the evaluation test cases can be run

    the current list of types.Evaluator is really a set of scenarios used to validate the transaction.TransactorClient; perhaps they belong in the transaction package?

    perhaps it would make sense to invert the package dependency such that the tests package is where we define our integration test infrastructure and orchestration (spinning up and shutting down the server instance with a configured backend) and then the integration tests live in the package they are testing?

  • I've been thinking also on this concept of a TestScenario where the in-memory config object (which knows where the test data directory is and has the backend database connection details configured for that test run) can be created and bundled with either a single evaluator or a list of evaluators

@darcys22
Copy link
Owner Author

darcys22 commented May 3, 2021

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
Copy link
Contributor

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 added a commit to davidalpert/godbledger that referenced this issue May 8, 2021
use the integration tag for all the integration tests
which should exclude them all when that tag is missing

use a compound constraint to further segregate:
- the mysql integration tests which are not yet working,
  they require additional investment to spin up a mysql
  db out-of-process; and
- the secure connection tests which are failing on my
  machine, likely due to some OS-specific behavior
  around how the GRPC client behaves when connecting
  and validating certs:

    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"
davidalpert added a commit to davidalpert/godbledger that referenced this issue May 8, 2021
use the integration tag for all the integration tests
which should exclude them all when that tag is missing

use a compound constraint to further segregate:

- the mysql integration tests which are not yet working,
  they require additional investment to spin up a mysql
  db out-of-process; and

- the secure connection tests which are failing on my
  machine, likely due to pollution of test context
  between tests and my local environment:

    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"

  after running the integration test suite I see the following
  untracked files:

    Untracked files:
        godbledger/rpc/~/
        godbledger/~/
        tests/~/

  I notice that tests/~/Library/ledgerdb/config.toml contains
  configuration pointing to a data direction outside the source
  folder in my local home folder, which makes me uncertain
  which config is being used during the test

  for now I am segregating the secure tests in order to have
  a clean integration test run while I refactor the way
  test data, directory, and config is managed to improve
  isolation between test cases and the local environment
davidalpert added a commit to davidalpert/godbledger that referenced this issue May 8, 2021
no point in running the mysql integration tests until
they are actually testing the evaluators
@davidalpert
Copy link
Contributor

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 added a commit that referenced this issue May 10, 2021
@darcys22
Copy link
Owner Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants