Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Added test exporter for use in unit tests. #1185

Merged
merged 7 commits into from
Dec 4, 2019
Merged

Added test exporter for use in unit tests. #1185

merged 7 commits into from
Dec 4, 2019

Conversation

jkohen
Copy link
Contributor

@jkohen jkohen commented Dec 3, 2019

With this exporter one can write unit tests to verify that the instrumentation is working. See the included code example.

With this exporter one can write unit tests to verify that the instrumentation is working. See the included code example.
@jkohen jkohen requested review from rakyll, rghetia and a team as code owners December 3, 2019 21:56
Copy link
Contributor

@rghetia rghetia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor comments.

metric/test/exporter.go Show resolved Hide resolved
metrics.ReadAndExport()
metricValue := getCounter(metrics, myMetric.Name(), newMetricKey("label1"))
fmt.Printf("increased by %d\n", metricValue-metricBase)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not simply display the value of Counter?

Counter value 1
Counter value 3
Counter value 6

why do you need metricBase?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

metricBase is needed to isolate test runs. The metric state is global, so if there are two tests that refer to the same metric, a test that looks for absolute values will fail. I confirmed that by removing metricBase, then creating a copy of the example. The output was:

 % go test -v go.opencensus.io/metric/test
=== RUN   ExampleExporter
--- PASS: ExampleExporter (0.00s)
=== RUN   ExampleExporter_2
--- FAIL: ExampleExporter_2 (0.00s)
got:
increased by 7
increased by 9
increased by 12
want:
increased by 1
increased by 3
increased by 6
FAIL
FAIL    go.opencensus.io/metric/test    0.005s

metric/test/exporter.go Show resolved Hide resolved
@jkohen jkohen requested review from rghetia and removed request for a team December 4, 2019 20:06
Copy link
Contributor Author

@jkohen jkohen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added a new example to cover the metric package. @rghetia what do you think? It seems less fragile than the existing tests that use something like:

	ms := r.Read()
	if got, want := ms[0].TimeSeries[0].Points[0].Value.(int64), int64(3); got != want { ... }

metric/test/exporter.go Show resolved Hide resolved
metric/test/exporter_test.go Outdated Show resolved Hide resolved
@jkohen jkohen requested a review from rghetia December 4, 2019 21:10
@rghetia rghetia merged commit 643eada into census-instrumentation:master Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants