Automatize Manual Testing for Core (True Integration Testing)

Problem

When we do add a new feature or do a small/big refactoring in the core, we always required to do some manual testing. Otherwise it’s really easy to miss newly introduced bugs to the system. Existent unit or integration tests may not powerful enough to help us to catch these bugs. We need to integration test core from outside of the core to be aware of these kind of bugs which is what we do with manual testing by spinning up dev-core and using dev-cli to cross test the behaviours of these two and see if they act as expected.

Bad Adoptions

Currently, we have some integration tests that are actually not integration tests. They seem like they are but not fully. They don’t test the actual integration behaviour. The integration tests we have are running inside the core itself and they don’t directly interact with core through the gRPC server.

Lets bring up a sample to examine:

TestExecute is not a real integration test. Yes, the testing service is really started but it’s tasks never get executed. The actual reason of why the service is started here is because there is a RUNNING status check on services and to not get failed by this, services must be exists on Docker.

We’re only able to test a small portion of task execution process in this test by checking the generated execution id.

Like within this example, all the other integration tests that starts services doesn’t really carry about them. And they can’t too because services are not able to connect to gRPC server to get task execution requests, get events and submit task results because our tests doesn’t run in a container and not in the same network with the containers that are running the services. Also for the same limitation, we don’t have any tests that requires listening results and listening events in grpc/core pkg.

Side note:
Other than previous example, the rest of the integration tests inside core are testing very simple cases like this one does. Which they are not needed as integration tests because we only need to test the application logic. So unit tests are enough for testing all the internal packages except container.

Correct Way of Creating Integration Tests

I first thought creating integration tests by using yaml files for simplicity where we can have table tests for testing various scenarios. But using yaml files for testing can quickly get out of control and limit us because we might need to deploy and start a few services in a row and assert some service ids, input/output datas by executing tasks and so on. To accomplish these kind of stuff with yaml test files, we need to introduce comparison and variable primitives which is too much work and another thing that requires us to maintain. So, this is a no.

We can use Go for integration testing and have a flexibility of a programming language. Still, it’s always better to have table tests as much us possible to not duplicate logic between similar tests and test lots of scenarios at once.

We can place our integration test under a package called integration, inside core’s root. And we can have integration/grpc & integration/cli subfolders for testing gRPC API and CLI separetely.

Please see feature/integration branch to get insights from the first implementation of new integration tests. I think this is a good start, we can add more tests by time. Note that integration tests are living under the integration folder and they should be run by ./run-integration. Also, it’s not required to have integration tests under the core repo because it only interacts with core through network and uses cli binary to test cli.

Side notes:

  • We should be adding tests for each cli command including the ones about daemon.
  • We don’t need to add tests for service gRPC API directly because we use real services and they use this API to interact with core. This way we also test if core is starting services correctly inside containers and giving the right service id as env variable to service’s containers. So having integration tests for core gRPC API is enough.

Proposal

  • Lets get rid of all the integration tests inside the internal packages and create equivalent ones inside new integration folder.

  • Lets have the integration tests in the most higher (gRPC server, cli) level and this way we’ll also eliminate doing manual tests after each change on core. Also inc integration tests coverage by adding more tests by time.

  • Lets only keep the integration tests we currently have for container pkg and do unit testing for the rest of the core as suggested before. By having dedicated integration tests for container, we can easily keep track of bugs in container pkg in the low level, otherwise debugging bugs caught from the higher level integration tests can get time consuming.

Links

About the tests.

We should more focus on writing good unit test and refactor existing code, then create more integration tests which are slow (which will lead to refactoring tests because slow tests are bad).

We can save a lot of time by refactoring the code rather than writing a ton of integration tests.

Moreover, the project is in the early stage to come up with such a solution as it adds more complexity to the project. It may happen (sooner than later) then we will need it, but for now let’s focus on features. Having more features will help us understand the nature of the tests we will need.

The code you pushed to GitHub (cli tests) seem to me like e2e tests (rather than integration). Also testing cli output could be done in commands package with unit tests.

So to sum up - you took an effort and prepared this proposal which is great, but first, we need to focus on simplifying what we have and then the time will come for more integration tests.

Thank you for the feedbacks! @krhubert

Tests are not features that we need to discuss about having them or not, they are a requirement. They are not the goals but the tools that helps us to get more closer to our actual goals. And having them shouldn’t be delayed otherwise it’s a technical depth. Having these tests also helps us for skipping manual tests that we do after each change we make in core. Manual testing and having uncaught bugs are more expensive than spending time while creating tests.

Unit tests are in the bottom of testing pyramid, all testing methodologies has their own advantages. Non of them is a great solution alone but having all of them actually solves the testing problem of a product. For the same reason we cannot only have unit tests for commands package, it is not enough by itself. We have to test the binary to see everything works well together and make sure we cover everything that might be skipped by unit tests.

I don’t understand why do you think that having these integration tests complicates things. I thought they help us to skip manual testing part and we can make sure that our product (gRPC API, cli) works as expected in the end users’ perspective.

I also understand this part: We can save a lot of time by refactoring the code rather than writing a ton of integration tests. What is the link between having tests and refactoring core? They’re totally different subjects.

Also these kind of integration tests are not something that will be updated very often because they’re in the top of testing pyramid and doesn’t test the implementation details that changes a lot. It applies in the most higher level, in other means we mostly only need to update these tests when we have breaking changes.

@core team this discussion needs further feedbacks.

We now have true e2e tests https://github.com/mesg-foundation/engine/tree/dev/e2e