# Testing Akri's `DiscoveryOperator`
This document walks through the issues that arose in trying to make the `DiscoveryOperator` testable and the resultant, arguable non-optimal, design.

So far in Akri, code that has needed to be mocked has often been wrapped in traits or extended using an extension trait. Examples of this are `KubeInterface` and `DeviceExt` for mocking interactions with the Kubernetes API and udev, respectively. These traits are passed as parameters to functions, so in tests, mocked structs that implement the interfaces are passed and expectations are set on which of the structures functions are called. This mocking is often simplified using the [`mockall`](https://docs.rs/mockall/0.9.1/mockall/) crate, as is the case with `KubeInterface`. While `mockall` can handle `async Traits` it cannot handle `impl Trait` parameters in Trait definitions. `mockall` does not support `impl Trait` parameters likely for two reasons. Firstly, it is a fairly new feature. `impl Trait` and `&impl Trait` are intended to be a more performant replacement for `dyn Trait` and `Box<dyn Trait>`. While the latter use dynamic dispatch and are bound at runtime, the new `impl Trait` uses static dispatch like generics and are bound at compile time. Resultantly, `dyn Trait` causes more runtime overhead¹.

This made mocking the `DiscoveryOperator` in the Agent complex when doing the work to modularize the Agent. A `DiscoveryOperator` is created for each applied Akri Configuration and does the work of finding the appropriate DiscoveryHandlers on which to invoke `Discover`, monitoring the ConnectivityStatus of the DiscoveryHandlers and Instance, and creating new Instances and device plugins for discovered devices. Many of it's functions take in Traits, such as `KubeInterface`, `DevicePluginBuilder`, and `StreamExt`. Enter the dilemma: in order to improve mockability and testability, all `impl Trait`s in the `DiscoveryOperator` had to be replaced with `Box<dyn Trait>`. Furthermore, [`async-trait`](https://github.com/dtolnay/async-trait), a crate that enables async traits which are not supported by the Rust standard library, [throws compilation errors around reconciling parameter lifetimes](https://github.com/rust-lang/rust/issues/63033), which I have not been able to resolve. This prevented me from being able to wrap the `DiscoveryOperator` in a Trait, so instead, I used `mockall_double` to mock the struct, which is supposed to help resolve [namespace issues that arise from Struct mocking](https://docs.rs/mockall/0.9.1/mockall/#mocking-structs).

`mockall_double` basically invokes conditional compilation under testing scenarios, which makes it hard to test the methods of the Struct, since the mock Struct will always be used in testing. The way around this was to put the functions that use the mocked `DiscoveryOperator` in their own `start_discovery` module and import the struct using the `#[double]` macro, which converts `DiscoveryOperator` to `MockDiscoveryOperator` during tests. So, when I wanted to mock the `DiscoveryOperator` when testing `do_discover(discovery_operator: Arc<DiscoveryOperator>...)`, I used the `super::start_discovery::DiscoveryOperator`, which automatically returns a mock `DiscoveryOperator`. When I wanted to test `update_connectivity_status`. a method of `DiscoveryOperator`, I used `super::DiscoveryOperator`. This worked well; however, it caused the Rust linter to throw dead code warnings on all of `DiscoveryOperator`'s methods that were not directly tested. It seems that this is happening because the Agent (in `config_action.rs`) invokes `start_discovery::start_discovery(...)`, using the `DiscoveryOperator` in the scope of the `start_discovery` module, but never uses the `DiscoveryOperator` outside of `start_discovery`. I am not sure how to avoid this and put `#[allow(dead_code)]` over all complaining methods for now.

Finally, getters were made for all of the `DiscoverOperator`'s fields in order to enable mocking.

[1] For more details on `impl Trait` vs `dyn Trait` see [this article](https://joshleeb.com/posts/rust-traits-and-trait-objects/).