netbird icon indicating copy to clipboard operation
netbird copied to clipboard

[Contributions] Unit / Component / Integration tests with ability to run them localy without sideeffects

Open skillcoder opened this issue 1 year ago • 0 comments

Problem description I'm always frustrated when I try to run tests locally. Currently, tests do not have any tags (unit/component/integration). Additionally, there is a lack of any Makefile/Taskfile. Instead of make test, I need to use go test -exec sudo ./... and always specify all parameters like -v -race -timeout -failfast -coverprofile -covermode. Without sudo, tests always fail because current tests also invoke integration tests which are very invasive and change system state. Also, tests leave the system in a modified state, for example, keeping /etc/resolv.conf changed and leaving backups. Even integration tests should not change local system files. For example, many network interfaces are created and not removed. Probably there are also other factors making the system very dirty. It's like a bomb for contributor local systems. Nobody expects to have such invasive system changes by running go test.

Proposed solution I propose splitting all tests into three following categories:

  • Unit: Normal unit tests without any side effects. For filesystem interactions, use temp dirs or ephemeral FS. For all dependencies, use mocks.
  • Component: Run client/server/mgmt as a real application but from the code. For external dependencies, use mocks.
  • Integration: Could be a component or unit in terms of scope but requires real external dependencies (real kernel syscalls, or interact with real FS, or communicate with real service). It could be run only in Docker or on a dedicated VM, and may require root privileges to run.

Each test should have one of three build tags (unit, component, integration) to be able to distinguish each type of test. The Makefile or Taskfile is retired to simplify the development process for all contributors. Additionally, I propose putting each type of test in separate files with the following naming convention: - *_test.go - unit tests - *_component_test.go - component tests - *_integration_test.go - integration tests

Also it is REQUIRED to change https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#test-suite by adding WARNING regarding integration tests, about the system changes.

Considered alternatives

  • Provide an alternative CI environment where contributors could automatically run tests.
  • Provide a comprehensive guide on how to automatically create a local CI environment using some VM solution (or at least provide VM images).

Additional context Could provide on request

Additional Considerations

  • Since netbird working with root privileges - need to create a list of all system changes which netbird client do and which information it send to netbird management server. For example for me it was very big surprise to see such PR which collect process running on my server. What else I don't know about netbird client do without my permissions?

skillcoder avatar Apr 07 '24 11:04 skillcoder