m-lab / ndt-server

docker native ndt5 and ndt7 server with prometheus integration
https://www.measurementlab.net/
Apache License 2.0
101 stars 40 forks source link

ndt-server lacks unit tests #51

Open pboothe opened 5 years ago

pboothe commented 5 years ago

We have integration tests, aka "the poor man's unit tests", but that's not good enough. All future PRs should ensure that all new code is unit tested. Over time, we can get code coverage up (and bugs down).

bassosimone commented 5 years ago

Do we aim to (a) have 100% coverage [*] where integration tests cover the common case and unit tests cover the corner cases or (b) to have 100% coverage through unit tests?

[*] Replace with any other sensible number that is considered optimal. Using 100% because I think we have already quite high coverage with regress tests alone.

pboothe commented 5 years ago

We should aim to have good coverage (more than 50%) through unit tests. Integration tests can easily mask problems, and having only integration tests makes it easy for code to become unreadable. Writing testable code means that every module has two users: the code and the test code, which forces interfaces to be cleaner and better-specified.

Basically, we use unit tests as a forcing function for modularity, clarity, and (hopefully) simplicity.

In my smaller projects, I try to aim for 100% coverage, because I imagine that future maintainers would feel ashamed to change 100% coverage to 99% coverage but would not feel nearly as much shame in changing 62% coverage to 61% coverage. It's a social hack, rather than an argument for code correctness - 100% coverage is not appreciably more likely to be bug-free than 99.9% coverage. For larger projects, 100% coverage seems extreme and unlikely.

bassosimone commented 5 years ago

Basically, we use unit tests as a forcing function for modularity, clarity, and (hopefully) simplicity.

👍

pboothe commented 5 years ago

Also, as a developer, when writing new code it is a fun challenge to see if you can simultaneously solve the problem and make coverage 100%. It's a distracting enough challenge that you will naturally tend to avoid making your code too complicated, because it will get in the way of the challenge.

I kind of think that writing code requires me to be dumb (write it the obvious way) and smart (make sure it is correct) simultaneously, and writing tests alongside the code helps me achieve that by offloading the smarts onto the test, which can keep the code dumb. But that idea needs a little more fleshing out.