Description
After #1143, the function testutil.GatherAndCompare
:
client_golang/prometheus/testutil/testutil.go
Lines 197 to 203 in 1bae6c1
client_golang/prometheus/testutil/testutil.go
Lines 205 to 222 in 1bae6c1
client_golang/prometheus/testutil/testutil.go
Lines 236 to 245 in 1bae6c1
became error-prone, as we filter the expected
metrics in compareMetricFamilies
as well now, if we make a mistake in one of the arguments expected
or metricNames
, the function will no longer return an error.
If e.g. we make a mistake in the name of the metric (provide a metric that is not exported because of a typo or re-using a variable containing the wrong metric name), the function will return nil
.
Or e.g. if we provide an expected
with the wrong metrics to a faulty code, we could never be aware of that.
I think expected
lost its safeguard/source of truth utility.
I expect the user/tester to have full control on expected
, I don't see why one couldn't provide the exact series they expect.
Plus having "dynamic" expected
makes the tests hard to grasp and debug. No one needs tests with too many logic/intelligence that needs to be tested as well :) Way beneficial when they are more explicit even if this result in a little bit more code.
Looking at the code that was justifying the change:
it's still not making use of the change yet, but I think they can easily split their expected
and pass it as a map or sth to their test function...
Discovered this while adding a test for kubernetes etcd metrics: kubernetes/kubernetes#120827. I copied/pasted a test function and the test was passing because I forgot to change the metric name.