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

Reorganize metric integration tests #674

Merged
merged 17 commits into from
Jul 19, 2023

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Jul 17, 2023

My motivation for this is to be able to link to individual fixtures for each metric type (gauge, sum, non-monotonic sum, etc) for the standard and GMP exporter in documentation.

This is probably easiest to review commit-by-commit.

  • makes a function for the GMP config
  • simplifies naming of fixture files (e.g. removes most instances of "metrics" and "basic" from names)
  • adds GMP tests for simple scenarios.
  • Enables the normalization feature gate for GMP tests. This demonstrates correct name translation in fixtures for the GMP exporter.
  • Change units of metrics to UCUM so the translation is tested.
  • Adds basic resource attributes to simple scenarios.
  • Disables service metric labels for fixtures
  • Change names of metrics and labels to include .s so that it tests sanitization.

@dashpole dashpole requested a review from a team as a code owner July 17, 2023 19:03
@dashpole dashpole requested a review from damemi July 17, 2023 19:03
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #674 (d0d9275) into main (7d82f66) will increase coverage by 0.48%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #674      +/-   ##
==========================================
+ Coverage   69.05%   69.54%   +0.48%     
==========================================
  Files          36       36              
  Lines        4699     4699              
==========================================
+ Hits         3245     3268      +23     
+ Misses       1302     1278      -24     
- Partials      152      153       +1     
Impacted Files Coverage Δ
...tor/integrationtest/testcases/testcases_metrics.go 100.00% <100.00%> (+14.47%) ⬆️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@dashpole
Copy link
Contributor Author

rebased

Copy link
Member

@damemi damemi left a comment

Choose a reason for hiding this comment

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

nothing blocking, but some suggestions.

it would also be nice if we added a comment on each test explaining what are the critical things to look for in the input/output fixtures. some are pretty self-explanatory but as they get more complex it gets harder to verify the expected fixtures

@dashpole
Copy link
Contributor Author

clarified all of the test case names to make expectations clearer.

@dashpole dashpole merged commit f8434b1 into GoogleCloudPlatform:main Jul 19, 2023
25 checks passed
@dashpole dashpole deleted the split_tests branch July 19, 2023 14:41
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