cdsbalancer: test cleanup part 1/N #6546
Conversation
easwars
force-pushed
the
cdsbalancer_test_cleanup_part_1
branch
from
4401e12 to
9b6c48f
Compare
|
Thanks for the review. |
| // Tests a single success scenario where the cds LB policy receives a cluster | ||
| // resource from the management server with LRS enabled. Verifies that the load | ||
| // balancing configuration pushed to the child is as expected. | ||
| func (s) TestClusterUpdate_SuccessWithLRS(t *testing.T) { |
There was a problem hiding this comment.
I'm sure there's a reason, but why is this not merged in with the t test directly above?
There was a problem hiding this comment.
I tried to merge them but it made the other test way more complicated than it had to. The reason is that the lrs config in the resource from the management server simply has a config source which says "self". But this has to be translated into the server config from the bootstrap. This means that the server URI (in the wantConfig) has to depend on the actual address of the management server. This means that I can't statically define wantConfig in the test table in the previous test.
There was a problem hiding this comment.
Hmm ok. I think you could argue you could have a conditional and write an extra field to the static configuration. I.e. if test name {test.wantChildConfig.DiscoveryMechanism.LoardReportingServer = want}. But this isn't too many lines of code so seems fine :).
| t.Fatalf("EmptyCall() failed: %v", err) | ||
| } | ||
|
|
||
| // Retrieve the cds LB policy policy and call ExitIdle() on it. |
This PR is the first in a set of test cleanups for the tests in the cds balancer. This is a pre-requisite for switching the cds balancer to use the generic xdsClient watch API.
Summary of changes:
cdsbalancer_test.goto use a real management server and rely on behavior instead of internal implementation detailse2epackageClusterresource to use the utilities provided by thee2epackageUpdateStateCallbackto themanualresolverRELEASE NOTES: none