quamina
quamina copied to clipboard
timbray
Organization of tests
We have a lot of tests and some of them really aren’t “unit tests” but focused on coverage and concurrency, etc. Speaking as a developer, here are the things I want to do.
- Run unit tests in the directory I'm working in, the equivalent of
go test. - Run concurrency tests across all the directories. This is important since thread-safety is an important and advertised feature.
- Coverage tests, to support our coverage badge
- Linting tests
- Acceptance tests, including all of the above, when you think you’re ready to do a PR
I suppose the main activities would be (1), (2), and (3,4,5)
Notes.
- For (1) we absolutely need to run in <10sec, so that a developer can run them every time they save a tiny change
- (1) should include one benchmark test to catch performance regressions, which can easily creep in even on a minor change. I think
TestCityLotsis a good choice since it explores worst-case performance of the Flattener and Matcher. - The
-raceflag really slows things down, in particular the concurrency tests. I don't think it's really needed in case (1)?
If it were 20 years ago, I think I’d say “we need a Makefile”. Um… I think we need a Makefile?
I haven‘t done any profiling why -race is slowing the tests down so massively. But also without -race unit tests should finish within <10s, agreed.
My pov: there should be no different tests for unit/concurrency/coverage. They‘re just flags to the go tool and should finish within a short time with a simple command, e.g. make unit-test.
The -race detector in Go is useful and does a good job of spotting general concurrency issues, e.g. from contributors new to the code base/new to Go.
(4) would be make lint. (5) would be make tests (combined target).
For running specific tests, IMHO we don‘t need special treatment as every Go developer can just use the go test tool directly, e.g. with -run to narrow down the test targets.
Example: this Makefile is simple and has a make help target for pretty-printing: https://github.com/vmware-samples/vcenter-event-broker-appliance/blob/development/vmware-event-router/Makefile
Haha, you're OK with the idea of a Makefile? I assumed that was too old-fashioned for a modern project. I’ll do a Makefile PR.
On the subject of -race, it's 100% toxic in combination with the coverage flag, but probably isn't needed with that tool. It seriously slows down a couple of the Concurrency-specific tests, but also it's probably really adding value there.
Actually, I'm optimistic that we won't have concurrency problems going forward because the places to update the data structure have been pushed down into a very small number of atomic.Load/Store operations hidden behind update() functions. But I've been wrong before on concurrency stuff.
Haha, you're OK with the idea of a Makefile? I assumed that was too old-fashioned for a modern project. I’ll do a Makefile PR.
Haha, not at all! Makefiles are great and still useful. Once in I can update CI.
On the subject of -race, it's 100% toxic in combination with the coverage flag, but probably isn't needed with that tool. It seriously slows down a couple of the Concurrency-specific tests, but also it's probably really adding value there.
Actually, I'm optimistic that we won't have concurrency problems going forward because the places to update the data structure have been pushed down into a very small number of atomic.Load/Store operations hidden behind update() functions. But I've been wrong before on concurrency stuff.
Famous last words :) I'm ok to drop race detector since you own the code and reviews anyways.
Nope, that's a separate issue. Let's leave this issue open until I get the Makefiles done.