A Review of Unit Testing
To ensure we’re all on the same page, allow me to share the basic anatomy of a well-written unit test:
1) Mock Out +--------------+ +---------------------->+ External | | | Dependency | | +------+-------+ | ^ | | 3) Side Effect | +------+-------+ +--+---+ 2) Call | | | +------------------>+ Subject | | TEST | 4) Return Value | Under | | +<------------------+ Test | +------+ | | +--------------+
So the steps are:
- TEST mocks out all external dependencies of the SUT
- TEST calls the SUT
- SUT creates side effects against the mocked out external dependencies
- SUT returns with, possibly, a return value
In this case, TEST would validate two things:
- The value returned by the SUT; and
- The side effect that the TEST indirectly triggered.
Validation step #1 is easy enough so I don’t need to discuss that further.
Validation step #2 is also simple but an important thing to note is that to validate the side effect, one only needs to see if the SUT called the mocked out External Dependency correctly. And to do that, one simply needs to interrogate the mocked out dependency. For example:
self.assertEqual(mocked_dep.function_name.call_count, expected_call_count) self.assertEqual(mocked_dep.function_name.call_args, cal(...))
If the SUT called it correctly, then we can confidently say that the SUT is implemented as designed.
Unit Testing with the Test Harness
Applying the above knowledge to unit testing with the Test Harness, here is the same diagram updated to the Test Harness context:
1) Mock out via +--------------+ Test Harness | | +--------------------->+ Dependencies | | | | | +--------------+ | | Framework | | +------+-------+ | ^ | | 3) Side Effect | +------+-------+ +---+--+ 2) Call | | | +------------------>+ | | Test | 4) Return Value | Charm | | +<------------------+ | +------+ | | +--------------+
In this case, the mocking out of the framework’s dependencies is taken care of by the test harness which is a big advantage to the test writer since that means less code to write. The concern now is with validating the side effect of our charm. Here is a sample test code to further explain the issue:
@patch('charm.build_juju_unit_status', spec_set=True, autospec=True) def test__config_changed__sets_unit_under_maintenance_until_k8s_pod_is_ready( self, mock_build_juju_unit_status_func, ): # Setup harness = Harness(charm.Charm) harness.begin() mock_juju_unit_states = [ MaintenanceStatus(str(uuid4())), MaintenanceStatus(str(uuid4())), ActiveStatus(str(uuid4())), ] mock_build_juju_unit_status_func.side_effect = mock_juju_unit_states # Exercise harness.update_config() # Assert # How to assert if the charm set framework.model.unit.status # 3 times as per mock_juju_unit_states
Unlike our previous assertion examples above, this test cannot interrogate the framework directly for information about how many times it was set because it’s a real framework instance, not a mock. Likewise, the test cannot go one level further and interrogate the mocked out dependencies because that is beyond its jurisdiction. So at this point, the only assertion we can do is the final state set by the charm:
This is less than ideal, however, because the assertion doesn’t really satisfy the full intent of the test which is to check whether the charm’s
on_config_changed handler reported the correct status 3 times.
A few ideas was shared over in IRC about this, specifically:
Use this patch that adds direct checking of every backend call;
INITIAL THOUGHTS: This is a good start but I’m a little concerned with using a private method directly (
_get_backend_calls()). Perhaps when this is exposed as a public method I’ll be more inclined.
Check after each individual steps;
INITIAL THOUGHTS: This is actually possible in cases where the test makes multiple repeated calls to the SUT. However, in our example above because the SUT will actually not return until an
ActiveStatusobject is received, this is not feasible.
Break apart the two so the one side emits events, and then you can have a second handler that monitors what events are sent;
INITIAL THOUGHTS: This will get the information the test needs. However, this adds an artifact to the charm code that doesn’t really add anything to its overall architecture but instead just exists to serve the test’s needs. That just adds code noise and can potentially make it unmaintainable.
Just assert the final state.
INITIAL THOUGHTS: As mentioned in the previous section, this is not ideal because we wouldn’t be satisfying the objective of the unit test which is to validate that the handler is actually actively checking the status of the underlying pod before returning an
A Proposed Solution
Being that the Harness is meant to be a mocking tool, perhaps it would benefit the charm author if it also provided methods similar to the ones found in
unittest.mock. For instance:
self.assertEqual(harness.framework_set_status.call_count, 3) self.assertEqual(hadness.framework_set_status.call_args, call(...))
In the example above, it was a deliberate choice to interrogate calls to the framework instead of the framework’s dependencies that the harness actually mocks. The reason for this is because charm authors should only be concerned with their charm’s code and should just assume that the framework will behave correctly as long as its methods are called correctly.
This post got longer than expected but I hope that it explains my concerns about the test harness and a solution can come out of this.