Skip to content
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

testutil: Add ScrapeAndCompare #1043

Merged
merged 4 commits into from
Aug 5, 2022

Conversation

sazary
Copy link
Contributor

@sazary sazary commented May 3, 2022

As discussed in #993 by @bwplotka, it would be nice to have a utility function that scrapes an exporter and compares it to a set of expected values.
I've mostly copied the approach used in kubernetes/kubernetes#108277 and refactored some minor functions in the testutil package.

Signed-off-by: sazary [email protected]

@sazary
Copy link
Contributor Author

sazary commented May 3, 2022

hey @bwplotka let me know what you think about this

if this is okay, could you please approve it's workflows?

thanks

@kakkoyun kakkoyun requested a review from bwplotka May 4, 2022 08:34
prometheus/testutil/testutil.go Outdated Show resolved Hide resolved
prometheus/testutil/testutil.go Show resolved Hide resolved
prometheus/testutil/testutil.go Outdated Show resolved Hide resolved
@sazary
Copy link
Contributor Author

sazary commented May 5, 2022

@dohnto thanks for your review

I applied your first & second suggestions and amended them to my previous commit, 6f41614

About the third one, I did it in a separate commit 86f0b14 and applied it to any occurrence in this file

Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Anything to add @bwplotka ?

@kakkoyun
Copy link
Member

kakkoyun commented Aug 5, 2022

@sazary Is it possible for you to rebase and fix conflicts?

@kakkoyun kakkoyun added this to the v1.13.0 milestone Aug 5, 2022
@kakkoyun
Copy link
Member

kakkoyun commented Aug 5, 2022

@sazary I've fixed the conflicts for you ;) It's good to merge now.

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

@bwplotka bwplotka merged commit 1638da9 into prometheus:main Aug 5, 2022
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.

4 participants