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
promql: extend test scripting language to support asserting on expected error message #14038
base: main
Are you sure you want to change the base?
Conversation
promql/test.go
Outdated
@@ -292,6 +292,12 @@ func (t *test) parseEval(lines []string, i int) (int, *evalCmd, error) { | |||
i-- | |||
break | |||
} | |||
|
|||
if cmd.fail && strings.HasPrefix(defLine, "expected_message") { |
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.
how about fail_message
or fail_msg
to be more explicit?
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.
I've changed it to expected_fail_message
, how's that?
Note for a future PR: It would be good to also be able to assert annotations. |
The nhcb branch lets us assert warning annotations, but not yet specific ones. After this is merged, when I fix the conflicts I can look into doing something similar to assert specific annotations. Might be slightly more complicated though because you can only have maximum 1 error but you may have multiple warning annotations. |
Why do we need both an explicit string and a regex, given the former is a subset of the latter? Incidentally "pattern" did not immediately suggest "regex" to me, maybe because I spent too much time using LogQL. |
Many error messages contain characters that are special in regexes (eg. However, an exact match isn't great for every case - sometimes a regex is more convenient or clearer (eg. if only parts of the error message are important).
Fair point, I'll change this. |
Signed-off-by: Charles Korn <[email protected]>
…age` Signed-off-by: Charles Korn <[email protected]>
Signed-off-by: Charles Korn <[email protected]>
Signed-off-by: Charles Korn <[email protected]>
Signed-off-by: Charles Korn <[email protected]>
199ff32
to
78861fe
Compare
I've rebased this to resolve the conflicts with #13999. |
Any chance I could get a review on this? Would be great to get this merged. |
The PR has diverged, with the introduction of regexes, since my initial review.
Could you provide examples? Moreover, we should clarify that we do not guarantee the stability of these messages, which might make these checks challenging to maintain. |
The use case for this is not for |
In that case, I think we can go with exact matching if we want to have such safeguards in our tests and let's see if they're easy to maintain. |
This PR extends the PromQL test scripting language to support asserting that a query fails with a particular error message or an error message matching a regexp (rather than just asserting that the query fails for any reason).