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.

Sign up for GitHub

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

Jump to bottom

Modify tests so they assert on expected Tags #65

Closed
npazosmendez opened this issue Aug 9, 2022 · 1 comment · Fixed by go-graphite/carbonapi#702
Closed

Modify tests so they assert on expected Tags #65

npazosmendez opened this issue Aug 9, 2022 · 1 comment · Fixed by go-graphite/carbonapi#702
Labels
enhancement New feature or request

Comments

@npazosmendez
Copy link

npazosmendez commented Aug 9, 2022 •
edited

Our fork updated many functions so they set many Tags that GraphiteWeb sets. This was one PR addressing that #22.

Now this new commit in the upstream go-graphite@0d4ebf3 introduced assertions on MetricData tags in a helper function used across many tests. So this is making a bunch of tests fail for us, because we are not setting the expected tags in the expected instances of "MetricData".

Solving this in the upstream sync PR would have made the PR huge. So I'm creating this issue to solve it separately, and we will be skipping the Tags comparison for now.
@npazosmendez npazosmendez added the enhancement New feature or request label Aug 9, 2022
@npazosmendez npazosmendez changed the title Modify function tests so they assert on expected Tags Modify tests so they assert on expected Tags Aug 9, 2022
@leizor leizor mentioned this issue Aug 9, 2022
Merged
@npazosmendez
Copy link
Author

This issue is basically adding los of .SetTag(..) around lots of tests,

https://github.com/go-graphite/carbonapi/blob/0d4ebf3feb206eef72033baafea1de0f42461d94/expr/functions/absolute/function_test.go#L34

and reverting the skip of tags comparisons:

@leizor leizor mentioned this issue Aug 9, 2022
Merged
@npazosmendez npazosmendez mentioned this issue Sep 7, 2022
Merged
@npazosmendez npazosmendez mentioned this issue Sep 19, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Restore tag assertions in tests and fix the failing ones go-graphite/carbonapi
1 participant
@npazosmendez