Convert PromQL tests to new syntax via basic migration mode #16585
Conversation
52b08f1 to
9d9f0bd
Compare
|
FTR: We'll have a detailed look here once #16562 is merged. |
9d9f0bd to
0d7f5af
Compare
|
Now this needs a rebase. As hinted at before, I think the Now that I think a better strategy is to do the conversion with the |
0d7f5af to
f77da02
Compare
Signed-off-by: Kapil Lamba <kapillamba4@gmail.com>
…ests Signed-off-by: Kapil Lamba <kapillamba4@gmail.com>
f77da02 to
0308355
Compare
|
First commit contains changes via the script using basic mode.
The changes in the second commit were minimal, based on my understanding @beorn7 |
There was a problem hiding this comment.
Thank you.
So the one fundamental thing you have missed is that we really need to test for the absence of a warn or info if we test other expressions on the same datasets for the presence of a warn or info.
Imagine the annotation is not triggered by the expression but by some irregularity in the dataset. In this case we wouldn't notice that we also get the annotation for the supposedly annotation-free expressions, plus we would not test that the annotation is added for the right reason.
I added many small comments all over the place so that you get the idea.
| {} 1 | ||
|
|
||
| # The histogram is ignored here so there is no result but it has an info annotation now. | ||
| eval_info instant at 0m min(http_requests_histogram) | ||
| eval instant at 0m min(http_requests_histogram) | ||
| expect info | ||
|
|
||
| eval instant at 0m max by (group) (http_requests) |
There was a problem hiding this comment.
This and the one below should probably also get the no_info treatment.
| @@ -462,13 +489,15 @@ eval instant at 1m quantile without(point)(0.8, data) | |||
| {test="uneven samples"} 2.8 | |||
|
|
|||
| # The histogram is ignored here so the result doesn't change but it has an info annotation now. | |||
| eval_info instant at 1m quantile without(point)(0.8, {__name__=~"data(_histogram)?"}) | |||
| eval instant at 1m quantile without(point)(0.8, {__name__=~"data(_histogram)?"}) | |||
| expect info | |||
There was a problem hiding this comment.
Which means we should add an expect no_info to the test up in line 487 to demonstrate that normally, there is no info annotation.
| @@ -681,22 +710,28 @@ load 5m | |||
| series{label="c"} {{schema:1 sum:15 count:10 buckets:[3 2 5 7 9]}} | |||
|
|
|||
| # The histogram is ignored here so the result doesn't change but it has an info annotation now. | |||
| eval_info instant at 0m stddev(series) | |||
| eval instant at 0m stddev(series) | |||
| expect info | |||
There was a problem hiding this comment.
And again, let's add expect no_info to the previous test in line 708 to demonstrate the difference.
| @@ -534,10 +543,12 @@ eval range from 0 to 24m step 6m left_histograms == right_histograms | |||
| eval range from 0 to 24m step 6m left_histograms == bool right_histograms | |||
| {} 1 0 _ _ _ | |||
|
|
|||
| eval_info range from 0 to 24m step 6m left_histograms == right_floats_for_histograms | |||
| eval range from 0 to 24m step 6m left_histograms == right_floats_for_histograms | |||
| expect info | |||
There was a problem hiding this comment.
So no_info on all the applicable other tests in this section.
Signed-off-by: Kapil Lamba <kapillamba4@gmail.com>
|
Apologies for the delay here—got caught up with work and personal life. I've addressed the comments now, please take a look. Thanks for your patience! |
Continuation of #16562