MQE: Add tests for various mixed data and edge cases #9281
Conversation
jhesketh
changed the title
|
FYI this test seems expensive (it runs a lot of permutations), but it only takes < 5s on my laptop. We can reduce some of the cases if it grows too much as we add more operations/functions etc. |
We could also look at running tests in parallel as well, if need be. But I think 5s is fine for now. |
|
This uncovered a difference in annotation handling. I looked at making MQE return the same results, but handling it was messy and I think it's easier to fix in Prometheus: prometheus/prometheus#14910 |
jhesketh
force-pushed
the
jhesketh/mqe-aggregations-tests
branch
from
f5de3f4
to
54a52b3
Compare
|
prometheus/prometheus#14910 has merged, but it may be a while before prometheus is vendored back into mimir. Until then this PR temporarily doesn't compare annotations. |
| mimirResults := q.Exec(context.Background()) | ||
|
|
||
| // We currently omit checking the annotations due to a difference between the engines. | ||
| // This can be re-enabled once https://github.com/prometheus/prometheus/pull/14910 is vendored. |
There was a problem hiding this comment.
As mentioned offline - given prometheus/prometheus#14910 has been merged, could we cherry-pick it into mimir-prometheus and then vendor that into Mimir so we don't need these workarounds without needing to do the pending Prometheus 3.0 merge?
There was a problem hiding this comment.
Lets deal with this separately as I think it's a bigger and more complicated task and we have a way forward here for now.
jhesketh
force-pushed
the
jhesketh/mqe-aggregations-tests
branch
from
8012108
to
1b4497a
Compare
What this PR does
Which issue(s) this PR fixes or relates to
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.