Add acceptance tests #40
Conversation
carrieedwards
force-pushed
the
cedwards/add-acceptance-tests
branch
2 times, most recently
from
6aaccf7
to
0acacfe
Compare
There was a problem hiding this comment.
The tests are looking good 👍
As well as the tests for single points, let's have a test that ingests a list of points (and use the Prometheus query_range endpoint to check all the points have been ingested), and also one where there are multiple series for the same metric but different labels.
|
On another note, I am going to file a separate PR for changing the write endpoint, since some unit tests will also need to be updated. |
carrieedwards
force-pushed
the
cedwards/add-acceptance-tests
branch
from
d74dc66
to
1950838
Compare
| s.T().Logf("Test ended at %s", time.Now().Format(time.RFC3339Nano)) | ||
| } | ||
|
|
||
| func (s *Suite) printLogs(res *dockertest.Resource) { |
There was a problem hiding this comment.
eventually (not now), is this something that could be broken out into an acceptance test library? This seems to have required a lot of code, and it would be nice if we could reduce the amount of boilerplate required for writing these tests.
There was a problem hiding this comment.
Yeah, I think that would be a good thing to do in the future to limit code duplication
| return container | ||
| } | ||
|
|
||
| func (s *Suite) startInfluxProxy() *dockertest.Resource { |
There was a problem hiding this comment.
continuing from above, would it be possible to move the influx-specific setup into a separate file so that we have a clean division between general acceptance test setup and this-proxy-specific stuff?
acceptance/write_proxy_test.go
Outdated
| Value: 23.5, | ||
| } | ||
|
|
||
| result, _, err := s.api.querierClient.Query(context.Background(), "stat_avg", time.Now()) |
There was a problem hiding this comment.
can you add a query before the record is written to confirm that the result before and after the write is different?
There was a problem hiding this comment.
As in, query prometheus to verify that metrics do not exist prior to the write?
acceptance/write_proxy_test.go
Outdated
| Value: 3, | ||
| } | ||
|
|
||
| result, _, err := s.api.querierClient.Query(context.Background(), "{__name__=~\"measurement_.+\",__proxy_source__=\"influx\"}", time.Now()) |
There was a problem hiding this comment.
same for all of these I think
pkg/influx/api.go
Outdated
|
|
||
| registerer.RegisterRoute("/api/v1/push/influx/write", http.HandlerFunc(a.handleSeriesPush), http.MethodPost) | ||
| registerer.RegisterRoute("/api/v2/write", http.HandlerFunc(a.handleSeriesPush), http.MethodPost) | ||
| registerer.RegisterRoute("/healthz", http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { |
There was a problem hiding this comment.
I think Justin added healthz in a different PR (#51), let's make sure this isn't redundant.
There was a problem hiding this comment.
Good catch, thanks
carrieedwards
force-pushed
the
cedwards/add-acceptance-tests
branch
from
8c5daf2
to
e564482
Compare
This comment has been minimized.
This comment has been minimized.
carrieedwards
force-pushed
the
cedwards/add-acceptance-tests
branch
from
1f7ea2e
to
f2d10a0
Compare
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
carrieedwards
force-pushed
the
cedwards/add-acceptance-tests
branch
from
c7757b6
to
a8adb50
Compare
|
Go coverage report: Click to expand.
Go lint report: No issues found. 😎 |
| // These tests verify that the Influx proxy is able to take in InfluxDB line protocol, | ||
| // correctly parse it and convert it into a timeseries, and write the timeseries to | ||
| // Cortex. Several services are run to execute the tests. The InfluxDB client is used to | ||
| // send line protocol to the proxy. The proxy service accepts the line protocol, parses it, | ||
| // and writes it to the Cortex service. The Prometheus client and API are used to query | ||
| // Prometheus to verify that the line protocol was parsed and converted into the expected | ||
| // timeseries, and that the timeseries was successfully written. | ||
|
|
||
| func TestSuite(t *testing.T) { |
There was a problem hiding this comment.
small tweak so that the documentation generator will pick up this comment. (we don't generate docs automatically, but we could, and this would be a nice comment to see if we do)
| // These tests verify that the Influx proxy is able to take in InfluxDB line protocol, | |
| // correctly parse it and convert it into a timeseries, and write the timeseries to | |
| // Cortex. Several services are run to execute the tests. The InfluxDB client is used to | |
| // send line protocol to the proxy. The proxy service accepts the line protocol, parses it, | |
| // and writes it to the Cortex service. The Prometheus client and API are used to query | |
| // Prometheus to verify that the line protocol was parsed and converted into the expected | |
| // timeseries, and that the timeseries was successfully written. | |
| func TestSuite(t *testing.T) { | |
| // TestSuite verifies that the Influx proxy is able to take in InfluxDB line protocol, | |
| // correctly parses it and converts it into a timeseries, and writes the timeseries to | |
| // Cortex. Several services are run to execute the tests. The InfluxDB client is used to | |
| // send line protocol to the proxy. The proxy service accepts the line protocol, parses it, | |
| // and writes it to the Cortex service. The Prometheus client and API are used to query | |
| // Prometheus to verify that the line protocol was parsed and converted into the expected | |
| // timeseries, and that the timeseries was successfully written. | |
| func TestSuite(t *testing.T) { |
This PR adds acceptance tests for influx2cortex