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

estats: remove dependency on testing package #7579

Merged
merged 2 commits into from
Sep 3, 2024

Conversation

easwars
Copy link
Contributor

@easwars easwars commented Aug 30, 2024

Addresses #7568

RELEASE NOTES:

  • experimental/stats: remove dependency on testing package

@easwars easwars requested a review from dfawley August 30, 2024 22:40
@easwars easwars added the Type: Internal Cleanup Refactors, etc label Aug 30, 2024
@easwars easwars added this to the 1.67 Release milestone Aug 30, 2024
Copy link

codecov bot commented Aug 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.73%. Comparing base (00514a7) to head (bf52c74).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7579      +/-   ##
==========================================
- Coverage   81.73%   81.73%   -0.01%     
==========================================
  Files         361      361              
  Lines       27809    27808       -1     
==========================================
- Hits        22730    22729       -1     
+ Misses       3869     3868       -1     
- Partials     1210     1211       +1     
Files with missing lines Coverage Δ
experimental/stats/metricregistry.go 94.44% <100.00%> (-0.08%) ⬇️

... and 15 files with indirect coverage changes

func snapshotMetricsRegistryForTesting(t *testing.T) {
// registry. Returns a cleanup function that sets the metrics registry to its
// original state.
func snapshotMetricsRegistryForTesting() func() {
Copy link
Member

Choose a reason for hiding this comment

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

If this isn't exported anyway, then can't we simply move it into an _test.go file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't do that since internal.SnapshotMetricRegistryForTesting is set to this unexported function (in an init function) and the variable in internal package is used by other tests and therefore needs to be set.

@dfawley dfawley assigned easwars and unassigned dfawley Sep 3, 2024
@easwars easwars assigned dfawley and unassigned easwars Sep 3, 2024
@dfawley dfawley assigned easwars and unassigned dfawley Sep 3, 2024
@easwars easwars merged commit 535bdce into grpc:master Sep 3, 2024
14 checks passed
@easwars easwars deleted the testing_dependency branch September 3, 2024 18:22
purnesh42H pushed a commit to purnesh42H/grpc-go that referenced this pull request Sep 11, 2024
purnesh42H pushed a commit to purnesh42H/grpc-go that referenced this pull request Sep 11, 2024
purnesh42H added a commit that referenced this pull request Sep 11, 2024
* estats: remove dependency on testing package (#7579)

* grpc: fix regression by freeing request bufferslice after processing unary (#7571)

---------

Co-authored-by: Easwar Swaminathan <easwars@google.com>
Co-authored-by: Codey Oxley <coxley@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants