skywire icon indicating copy to clipboard operation
skywire copied to clipboard

Hypervisor tests rely on visor field being empty

Open i-hate-nicknames opened this issue 4 years ago • 0 comments

Currently, hypervisor tests instantiate a hypervisor without a visor: hypervisor_test.go

hypervisor code handles this case in some methods: hypervisor.go

So we end up in a situation when applicaton code exists solely for tests, confusing those who need to maintain it.

I can see the following possible solutions, apart from removing existing tests:

  1. Instantiate a real visor from visor config, keep a test visor config somewhere. This will make test initialization heavy, so probably worth moving to integration testing, like so: https://stackoverflow.com/a/28007631

  2. Introduce visor interface and use it in hypervisor instead of concrete Visor type. Then use a mocking library to pass a mock visor in tests. This is a cleaner solution, but will require rewriting hypervisor code to use visor interface, as well as building visor interface itself (decide what should be part of the interface). The mock can be also implemented directly if needed, like it's done with remote visor connections

  3. Investigate whether hypervisor actually needs a direct access to visor object. If it's possible to narrow down required functionality to Connection interface (which is used for remote visors now), this is the easiest option to go with.

i-hate-nicknames avatar Feb 03 '21 12:02 i-hate-nicknames