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

multiple/test: use stub balancer instead of defining wrapped balancers #6514

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

easwars
Copy link
Contributor

@easwars easwars commented Aug 8, 2023

This PR cleans up a bunch of tests which used to define custom balancers which wrapped an underlying balancer and added some functionality on top of it. Instead of defining the balancers outside of the test, this PR uses the support in the stub package to define them inline. This leads to shorter and more readable code.

The pattern followed is as follows:

  • Whenever a custom test balancer needs to wraps another existing policy, we use the Init method to build the underlying policy and store it back in stub.BalancerData.Data, thereby getting access to the underlying balancer whenever needed from other methods.
  • Whenever a custom test balancer needs to override any methods in the balancer.ClientConn passed to the underlying balancer, a new struct testCCWrapper which contains only the overridden functionality is defined. This keeps the changes local and easier to follow.

Also the clustermanager tests were creating a wrapped round_robin policy which could ignore balancer attributes in addresses. The round_robin LB policy was updated a while ago to use the addressMap. So, we don't need this anymore in these tests and can use the vanilla round_robin policy.

RELEASE NOTES: none

@easwars easwars requested a review from dfawley August 8, 2023 20:48
@easwars easwars added this to the 1.58 Release milestone Aug 8, 2023
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

Now we just need an easy way to stub out ClientConns.......

@dfawley dfawley assigned easwars and unassigned dfawley Aug 8, 2023
@easwars easwars merged commit 67a8e73 into grpc:master Aug 9, 2023
1 check passed
easwars added a commit that referenced this pull request Aug 10, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants