tink
tink copied to clipboard
Rethink external runtime dependencies in unit tests
The unit test code here creates docker containers for use in unit tests. This feels inappropriate for unit tests. Also the way it's written, all tests that call this function will get their own postgres container. As it currently stands, i have seen up to 4 containers running at a single time. This also feels inappropriate.
I would argue that unit tests should not require a container runtime to be installed. They should strive to only require Go.
I think that for functional or integration testing spinning up containers is acceptable. Maybe this code can be isolated to only be used for functional/integration testing.
Curious what others think?
I remember writing those tests and I had similar concern. However, it was helpful testing the event system I wrote for Tink, and so it was acceptable at that point. But as I look at it now, I find it confusing and overcomplicated.
I believe we should rather define better abstractions and as you rightly said this code can be isolate for integration tests.
I think that for functional or integration testing spinning up containers is acceptable. Maybe this code can be isolated to only be used for functional/integration testing.
Yes.
Unpopular opinion (possibly): Unit tests should also, ideally, run in 10s of ms (sub 100ms) and containers typically take more than .5s to launch.
Closed by https://github.com/tinkerbell/tink/pull/654