xds: Properly assign picker.

[temawi]

Fixes a bug where the picker was not getting used in RpcBehaviorLoadBalancer.

[ejona86]

You need to change the RpcBehaviorLoadBalancer constructor to not wrap as well.

[ejona86]

This looks busted. I see the tests would mostly work, but this doesn't instill confidence. One place we wrap a RpcBehaviorHelper with a (mock that claims to be) RpcBehaviorHelper and in the other we pass the mock directly into the LB constructor. Seems this should be Helper (which is a pseudo-interface). If you mock a real object, you should pretty much always use delegatesTo(); but in this case the object being mocked is also under test, so that seems strange.

Note that you can easily have a non-mock RpcBehaviorHelper field if you use mock(Helper.class) instead of @Mock. Otherwise you'd need to construct it in each test (not a big problem because it is only used twice... although I think that shows the other tests aren't that great[1]) or use @Before.

1. "Not getting into it now," but helperWrapsPicker() to me is a perfect example of what you get when you make "the unit" be "a single class" along with white box testing. That test should have failed, but didn't, because it didn't test that RpcBehaviorLoadBalancer actually used RpcBehaviorHelper. To me, it'd be best to combine it with pickerAddsRpcBehaviorMetadata() and confirm that RpcBehaviorLoadBalancer as a whole does the one job it is supposed to do. RpcBehaviorPicker would be a private class and not referenced in the test at all. Without combining it all together, the tests are brittle (as they mirror the implementation 1:1 as it is testing internal constraints and not external constraints) and don't actually test the code does what it should. I could delete the line in RpcBehaviorLoadBalancer that calls setRpcBehavior() and all the tests would pass.

[temawi]

Ok noted, we can look at the general testing approach here. For now I just updated mockHelper to use the Helper class.