Merge logic of unit and integration test

Hey guys,

I was reading a couple of unit and integration tests files and most of the tests are exactly the same.
If we exclude the lines for mocking the unit tests, the lines are exactly the same.

Shouldn’t we merge integration and unit tests asserts and find a way to separate the configuration of each case?

The other argument is to have only one set of test of maintainability. Having 2 set of tests is harder to maintain and we don’t always think to update a test in unit and integration files.

I create a small POC on the daemon package (that is not Newable and use a shared defaultContainer variable to access the container instance):

func TestLogs(t *testing.T) {
	// `go test` command accept a `-short` flag that is meant to accelerate test.
	if testing.Short() {
		defaultContainer = new(mocks.Container)
	}

	// Other possible condition, if the interface typed variable is actually a mock, we can cast it and configure the mock
	if mockContainer, ok := defaultContainer.(*mocks.Container); ok {
		fmt.Println("it's a mock")
		r, _ := io.Pipe()
		mockContainer.On("ServiceLogs", mock.Anything).Return(r, nil)
	} else {
		// Require config for integration test
		startForTest()
	}

	// Execute the same logic and asserts
	reader, err := Logs()
	require.Nil(t, err)
	require.NotNil(t, reader)
}

What do you think @core ?

Merging integration tests and unit tests like this is doable. As you said, it’ll make the test maintenance easier and we’ll have the same tests for both unit and integration tests.

As a downside, it’ll complicate the tests because we’ll have more if-else statements to check the test type and for bigger tests it can become an issue. Merging tests like this feels like a monolithic approach to me, easy to do but in a long term can be quite confusing and could get out of control.

I prefer having much stupid tests with less lines in it as possible. An ideal test should be small, should be targeted and shouldn’t be updated too often. But having monolithic tests breaks this rules and introduces complexity in tests itself.

I think what we should do it to avoid duplicaiton of the same test with mock and in integration. So for know we have solution with integration tag and it’s good (same with short), but we should only proivde one test because testing the same thing with/without mock doesn’t brings any value.

Actually, I think we don’t need most of the integration test we have. Let me explain:

Let’s say we have 2 packages: container, service.
container uses an external system: docker.
servicedoesn’t use any external system directly but use container.

In this case, container should have integration tests. This way we can be 100% sure that everything is working. Then, we could also have unit test to make sure if we break stuff we can know really fast. But again, the unit test are be optional because they will never give us 100% trust on test on the package.

Then, service should have only unit test that use a mock of container. As service doesn’t internally really on slow external system, we can run the test really fast (with a mock of container of course).

At the end, I think we don’t ask the right question if we need integration test and/or unit test. I think the right question is: shouldn’t we have only 1 way to test a package?

Whenever it’s integration or unit test, it’s more about fast or slow test. So the -short flag and the t.Skip function could prevent slow test to be executed when we don’t want.
Quote from https://golang.org/pkg/testing:

Tests and benchmarks may be skipped if not applicable with a call to the Skip method of *T and *B:

func TestTimeConsuming(t *testing.T) {
    if testing.Short() {
       t.Skip("skipping test in short mode.")
   }
   ...
}

But again, we could reach the same functionality with the -tags and the header in test files (like integration tag)

@krhubert @ilgooz @Anthony what are you feedbacks?

@Nicolas When you put it this way, I totally agree with you. I don’t see any reason keeping integration tests except for the container package. Let’s validate this idea and make sure it’s right.

For container package, we must have the integration tests. We can/should also keep the unit tests for container package for running tests faster while developing the container package.

I’m not so sure about combining unit tests and integration tests together for container package because it might complicate the tests. If we decide to combine them, I’m also in favor of using -short flag for disabling integration tests instead of enabling integration tests with tags=integration. Short flag makes a bit more sense to use in our case because we’ll not heavily depend on integration testing.

In addition to integration testing of core:

There is always a margin for mistakes with unit testing too. So we cannot never be sure if core works well with docker or other external services as it is suppose to be if we don’t have integration tests in the higher level.

I think we can still have some integration tests but not required to have them in the core itself. We can have integration tests within another languages or even with some bash scripts. We can integration test the core through the gRPC API directly.

We don’t even need to check if a network or container is created with this kind of testing. Lets just create a few scenarios about coupling a few services together and compare the expected task results, event data, service list etc. through the gRPC API.

1 Like

You choose the wrong two packages :slight_smile: What I mean by that, here the container and the service should have integration tests.

The service imports container but… The container is just a wrapper for docker api, and the service is just wrapper for container which is wrapper for docker api. Those two packages are super thight releated with docker and at least one (service) of them shouldn’t.

But in general if you have more SOLID code this should work as you described

Nobody said that we need only one of those type tests. And about 1 way, no we shouldn’t. Tests should be smart, if you can tests something with unit test go with them, if not go with other tests (e2e, functional, integration etc…).

I express my thought the wrong way. I should not say “only 1 way to test a package” but only one test per thing to test (can be integration, unit, etc…). I would like to avoid to test the same feature, with the same path, in differents ways. Example:
In service package:

  • TestIntegrationStartWith2DependenciesIntegration / TestStartWith2Dependencies
  • TestIntegrationStartServiceIntegration / TestStartService.

My suggestion is to test with only one test a feature using the best tool.
Example:

  • For a “happy path” of the service.Start function, it should be done by using the real Docker Engine, so integration test.
  • For a path with error of the same service.Start function, we should use a mock to inject precisely the desired error and test if the return of the function is correct.
2 Likes