Skip to content

Add options to test for skipping warn and info#15713

Closed
SungJin1212 wants to merge 1 commit intoprometheus:mainfrom
SungJin1212:Add-test-option-for-skipping-warn-and-info
Closed

Add options to test for skipping warn and info#15713
SungJin1212 wants to merge 1 commit intoprometheus:mainfrom
SungJin1212:Add-test-option-for-skipping-warn-and-info

Conversation

@SungJin1212
Copy link
Contributor

This PR adds two test options, skipEvalInfo and skipEvalWarn. If true, they skip eval_info and eval_warn respectively.
It allows another engine to do an acceptance test without verifying warn and info.

@SungJin1212 SungJin1212 force-pushed the Add-test-option-for-skipping-warn-and-info branch 2 times, most recently from ed32afb to 9c15e3e Compare December 27, 2024 06:59
@SungJin1212 SungJin1212 reopened this Dec 30, 2024
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212 SungJin1212 force-pushed the Add-test-option-for-skipping-warn-and-info branch from 9c15e3e to 1b71888 Compare December 30, 2024 06:16
@beorn7
Copy link
Member

beorn7 commented Jan 8, 2025

It's quite disruptive to add options to everything, given that Prometheus itself doesn't need it.

Or to be precise: There are plans to make the eval_... commands better suited to test various annotation scenarios. I would strongly prefer to add more commands (or change the existing ones) so that tests can express things like "I don't care about annotations" or "I want both info and warn" or even "I want this very specific annotation".

@beorn7
Copy link
Member

beorn7 commented Jan 8, 2025

See the discussion starting here. This would get rid of eval_info and eval_warn altogether, replacing them by some other parametrization to specify expected infos and warnings, and that should also help your use case here. I'll try to file a proper issue for it later today.

In any case (as also discussed there), the short-term plan is to make eval_ordered ignore annotations. Maybe that's something you could piggyback on until we have the "proper" solution in place.

@beorn7
Copy link
Member

beorn7 commented Jan 8, 2025

See #15794 for the "proper" solution.

@beorn7
Copy link
Member

beorn7 commented Jan 8, 2025

#15795 will at least allow eval_ordered to circumvent annotation checks. (It doesn't work under all circumstances, but maybe it already helps.)

@beorn7
Copy link
Member

beorn7 commented Jan 16, 2025

I think #15794 looks very promising by now. Let's get #15794 done ASAP, which should also solve the problem this PR tries to address. In turn, let's close this to not create the additional complexity in the code that we would then remove again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants