Add testutil package #445
Add testutil package #445
Conversation
Signed-off-by: Frederic Branczyk <fbranczyk@gmail.com>
It wasn't needed, as is now proven by the tests Signed-off-by: beorn7 <beorn@soundcloud.com>
`testutil` is more in line with stdlib naming conventions. The package should be below `prometheus` as it only provides utils to test exposition code, not to test HTTP client code. Signed-off-by: beorn7 <beorn@soundcloud.com>
Signed-off-by: beorn7 <beorn@soundcloud.com>
Signed-off-by: beorn7 <beorn@soundcloud.com>
prometheus/testutil/testutil_test.go
Outdated
| @@ -1,3 +1,16 @@ | |||
| // Copyright 2018 he Prometheus Authors | |||
prometheus/testutil/testutil.go
Outdated
| // CollectAndCompare registers the provided Collector with a newly created | ||
| // pedantic Registry. It then does the same as GatherAndCompare, gathering the | ||
| // metrics from the pedantic Registry. | ||
| func CollectAndCompare(c prometheus.Collector, expected io.Reader, metricNames ...string) error { |
There was a problem hiding this comment.
Commit message says "Expected text format is not read from an io.Reader" but I see an io.Reader here. Perhaps the message should be "... is now read ..." ?
There was a problem hiding this comment.
Exactly. I will fix the commit description.
| metrics = filterMetrics(metrics, metricNames) | ||
| } | ||
| var tp expfmt.TextParser | ||
| expectedMetrics, err := tp.TextToMetricFamilies(expected) |
There was a problem hiding this comment.
Why bother parsing the expected text? If the test author is responsible for typing/writing the expected golden values, is it that much more work to match the formatting of the encoder?
Perhaps if the normalizing and sorting in this file is later replaced by calls to some shared package this approach won't seem so complicated. Is that the plan?
There was a problem hiding this comment.
It's legal in the text format to liberally use whitespace. Knowing exactly the formatting of the encoder would be a burden for the programmer writing those tests, and one for which I cannot really see any gain.
Personally, I wouldn't even like if a "normalized" formatting is advertised (but that's a discussion that is happening elsewhere and definitely out of scope of this PR). In any case, there is no goal right now to provide a tool for "normalizing".
prometheus/testutil/testutil.go
Outdated
| break | ||
| } | ||
| } | ||
| if !drop { |
There was a problem hiding this comment.
These loops make more sense to me without any boolean var:
if m.GetName() == name {
filtered = append(filtered, m)
break
}There was a problem hiding this comment.
I think you are right.
NB: This is not my code. So I don't know if there is some non-obvious reason why it was done the way it is.
I'll change it according to your suggestion, and @brancz can chime in if we blundered here.
- Expected text format is now read from an io.Reader. - Metrics are gathered from a Gatherer. - Added a convenience wrapper to collect from a Collector. Signed-off-by: beorn7 <beorn@soundcloud.com>
beorn7
force-pushed
the
beorn7/testing
branch
from
1d2faee to
5ba0993
Compare
This is for types we don't want to export but which are used in different packages within client_golang. Currently, that's only NormalizeMetricFamilies (used in the prometheus package and in the testutil package). More to be added as needed. Signed-off-by: beorn7 <beorn@soundcloud.com>
|
I was about to comment that we should probably move the |
|
That's all mapped out in the initial description of this PR. :o) |
Create an internal package
|
Looks good to me! |
This is derived from #423. I recreated it so that it is pulling from a branch I can push to...
Changes to #423 are based on the review that has already happened. I packaged it nicely in commits with messages, so you can follow along.
I will add more documentation and the single-value retrieval helpers (cf. #230) in a separate PR. Same with the creation of an internal package for the duplicated code. Don't want to put too many things into this one PR.
@grobie @brancz