Sled agent uses a lot of mocks to represent interactions with underlying resources. It uses a library named mockall to help with this interaction. We should use fakes instead.
Background
When the sled agent is provisioning a zone, it "mocks" access to zones. This enables unit tests to verify "hey, you would have created a zone here", but in reality, make no such call.
This is okay-ish for some types of unit tests, where we have a module foo which depends on a interface resource:
foo can be conditionally compiled with cfg(test) to use the mockResourcefoo's tests can then use mockall's API to set expectations about calls to the mockResource
This is one such example of a test written in this style.
Problems
This testing strategy really breaks down when it gets nested. Suppose we have a module baz, which depends on foo and bar.
foo's dependency on mockResource still needs to be captured, so baz also needs to set up "expectation" calls in tests- The same applies to
bar, and any other modules which contains mocks
- This basically leaks implementation details, and makes tests really difficult to write
This is apparent for modules like the storage_manager, where we're interfacing with:
- ZFS provisioning
- Zpool provisioning
- Zone provisioning
and all of them would need to be mocked to actually write tests.
Proposal
We should implement fakes wherever these mocks are being used. This will give us a more flexible interface for testing, and make dependencies on "global-ish resources" much more apparent.
Admittedly, doing so will require providing the interfaces to these modules as up-front objects. I propose doing so with Arc-bound trait objects, rather than generics, to keep things relatively simple. The cost of a single Box + Arc's refcounting should be trivial compared with the cost of "provisioning a filesystem" or "managing a zone".
Sled agent uses a lot of mocks to represent interactions with underlying resources. It uses a library named
mockallto help with this interaction. We should use fakes instead.Background
When the sled agent is provisioning a zone, it "mocks" access to zones. This enables unit tests to verify "hey, you would have created a zone here", but in reality, make no such call.
This is okay-ish for some types of unit tests, where we have a module
foowhich depends on a interfaceresource:foocan be conditionally compiled withcfg(test)to use themockResourcefoo's tests can then usemockall's API to set expectations about calls to themockResourceThis is one such example of a test written in this style.
Problems
This testing strategy really breaks down when it gets nested. Suppose we have a module
baz, which depends onfooandbar.foo's dependency onmockResourcestill needs to be captured, sobazalso needs to set up "expectation" calls in testsbar, and any other modules which contains mocksThis is apparent for modules like the
storage_manager, where we're interfacing with:and all of them would need to be mocked to actually write tests.
Proposal
We should implement fakes wherever these mocks are being used. This will give us a more flexible interface for testing, and make dependencies on "global-ish resources" much more apparent.
Admittedly, doing so will require providing the interfaces to these modules as up-front objects. I propose doing so with
Arc-bound trait objects, rather than generics, to keep things relatively simple. The cost of a singleBox+Arc's refcounting should be trivial compared with the cost of "provisioning a filesystem" or "managing a zone".