Unit test using input and output proto files #223
Conversation
Codecov Report
@@ Coverage Diff @@
## master #223 +/- ##
=======================================
Coverage 73.02% 73.02%
=======================================
Files 15 15
Lines 1609 1609
=======================================
Hits 1175 1175
Misses 355 355
Partials 79 79Continue to review full report at Codecov.
|
metrics_proto_api_test.go
Outdated
| // Read input Metrics proto. | ||
| f, err := ioutil.ReadFile("testdata/" + "inMetrics_" + filename + ".txt") | ||
| if err != nil { | ||
| panic("error opening in file " + filename) |
There was a problem hiding this comment.
Consider t.Fatalf instead of panic. Same everywhere else.
There was a problem hiding this comment.
Hope you will not get mad on me :), instead of having the "inMetrics" "outTS" as prefix can we have them as suffix?
inMetrics_ExportMetricsOfAllTypes.txt -> ExportMetricsOfAllTypes_InMetrics.txt
Theses way all the files related to one test will be in the close to each other. Another opinion may be to have:
testdata/export_metrics_of_all_types/[in.txt|out.txt|...]
Personally I prefer more the directory structure because then all files related to that test are in one directory.
|
Also, this is super nice :) |
| double_value: 35.5 | ||
| > | ||
| > | ||
| --- |
There was a problem hiding this comment.
Do you need these separators? I think you can use a metric list instead, something like
metric: <
metric_descriptor: <...>
timeseries: <...>
>
metric: <
metric_descriptor: <...>
timeseries: <...>
>
...Then in test.go:
var metrics []*metricspb.metric
proto.UnmarshallTxt(metrics, file)There was a problem hiding this comment.
Same for metric descriptor requests and time series requests.
There was a problem hiding this comment.
UnmarshalText only accepts Message
func UnmarshalText(s string, pb Message) error {I would have to create wrapper proto like
message MetricList {
repeated opencensus.proto.metric.v1.Metric metric = 1;
}
Let me know if I missed something.
There was a problem hiding this comment.
Got it - I didn't know that, LGTM then.
No problem.
I like the directory approach as well. |
|
@bogdandrutu PTAL. |
No description provided.