Skip to content

Conversation

greed42
Copy link
Contributor

@greed42 greed42 commented Apr 2, 2025

Will allow you to use promtool test rules --fuzzy-compare files... to get very slightly less exact floating-point comparisons.

Fixes #5352

@greed42 greed42 requested a review from dgl as a code owner April 2, 2025 15:37
@greed42 greed42 force-pushed the promtool-fuzzy-compare branch from ddc578c to 4bb1fe0 Compare April 2, 2025 15:57
@greed42
Copy link
Contributor Author

greed42 commented Apr 2, 2025

@dgl given you're both the promtool maintainer and suggested this approach in #5352 (comment)...

@dgl
Copy link
Member

dgl commented Apr 2, 2025

I like the concept. Maybe rather than a command line flag it should be configured in the test file. Putting it in the test file makes it more explicit, makes the test self contained and avoids a whole "works on my machine" and "how are you running the tests?" situation with the obvious downside this then becomes a piece of boilerplate.

@dgl
Copy link
Member

dgl commented Apr 2, 2025

Another reason to configure this in the file is while I've only ever seen off-by-one value errors we could also allow a file to override with off-by-two, off-by-x if anyone sees such an issue... (I am not suggesting to implement that initially but it leaves the option open to have this per test file.)

@greed42
Copy link
Contributor Author

greed42 commented Apr 3, 2025

The "works on my machine" argument is compelling enough, I'll re-work with a fuzzy_match file scope bool (barring any better naming suggestions).

@beorn7
Copy link
Member

beorn7 commented Apr 3, 2025

In the promql testing framework, we have been using almost.Equal for a while. That's even more tolerant than the approach here, but maybe that's fine?

@greed42
Copy link
Contributor Author

greed42 commented Apr 3, 2025

Well now I'm tempted to just expose the epsilon of almost.Equal as a configurable and then we never have to worry if people want seriously weak comparisons.

I think I'd rather stick with the "off-by-one" for now as that's also the only case I've run into in my rule tests. This PR should provide an obvious path to adding another option should that be necessary.

greed42 added 2 commits April 3, 2025 11:41
Will allow you to use `promtool test rules --fuzzy-compare files...`
to get very slightly less exact floating-point comparisons.

Fixes prometheus#5352

Signed-off-by: Graham Reed <greed@hypervolt.co.uk>
Instead of a flag, so tests that expect it to be enabled will have it. And will fail to parse on older versions of promtool.

Signed-off-by: Graham Reed <greed@hypervolt.co.uk>
@greed42 greed42 force-pushed the promtool-fuzzy-compare branch from 4bb1fe0 to 94a926c Compare April 3, 2025 10:41
@dgl
Copy link
Member

dgl commented May 1, 2025

In the promql testing framework, we have been using almost.Equal for a while. That's even more tolerant than the approach here, but maybe that's fine?

Given it's more tolerant we could change to it later if we liked. Lets go with this.

@dgl dgl merged commit b6aaea2 into prometheus:main May 1, 2025
27 checks passed
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.

Promtool unit test rules support values as a range
3 participants