-
Notifications
You must be signed in to change notification settings - Fork 151
Bug 1694715: Add testing against schema for the rate metric type #1529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| ..Default::default() | ||
| }, | ||
| ], | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chutten pointed out in Matrix that maybe we should document how to make these in the developer docs. First, to confirm that this is correct, though...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks correct to me.
chutten
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seem to be a lot of changes not related to the rate type in this local schema copy. Where are they from?
|
|
||
| let rate_metric: RateMetric = RateMetric::new(CommonMetricData { | ||
| name: "rate".into(), | ||
| category: "test".into(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put the category first so I can read it in the same order ( test.rate ) that it appears in the payload?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing.
The vendored copy of the schema hasn't been updated in a while. Something to add to the developer docs... |
93dae34 to
61598dd
Compare
| ..Default::default() | ||
| }, | ||
| ], | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks correct to me.
We should block this on mozilla-services/mozilla-pipeline-schemas#661, since this pulls in the new schema from there.
This just adds rate metric type data to the existing RLB schema validation unit test to make sure the output matches the schema.
No
CHANGELOGentry since this is purely testing...