grpc-go
grpc-go copied to clipboard
grpc
Expose the underlying channel from testutils.Channel
testutils.Channel provides a useful timed receive operation for tests.
There are some tests though where access to the underlying channel is required, because it is passed to some other component which receives on the channel. For example: readyCh in this test.
It would be nice to expose this underlying channel for tests that really need it. Something along the lines of timer.C would suffice.
I'm not sure whether I got the right point of issue. In order to expose the underlying channel, Is it enough that modify [ch](ch chan interface{}) into Ch? I think that I may not understand correctly. Maybe I need some tips, thanks.
Yes, rename ch to C and add a comment, something to this effect
// C is the underlying channel on which values sent using the SendXxx() methods are delivered.
// Tests which cannot use ReceiveXxx() for whatever reasons can use C to read the values.
And also try to change the example test which I pointed to in the original description.
Hope this helps.
And also try to change the example test which I pointed to in the original description.
The test should be changed, but it will be easy for me if the instruction and problem about test file is more clear.
Maybe I get the point. because underlying channel is't exposed, there aren't some tests.
https://github.com/grpc/grpc-go/blob/997ce619eb555b6a481e741afa6390ad3cd80d5c/xds/internal/server/listener_wrapper_test.go#L212
change into lw, readyCh, xdsC, lis, cleanup := newListenerWrapper(t)
And I should add some tests about underlying channel in xdsC and lis.
It would be nice to expose this underlying channel for tests that really need it. Something along the lines of timer.C would suffice.
@easwars
I have a question regarding this, changing readyCh is kind of confusing here. readyCh is essentially a <- chan struct{} which is triggered from goodUpdate.Done()
method.
Am I supposed to change Event struct in grpcsync package from
type Event struct {
fired int32
c chan struct{}
o sync.Once
}
to
type Event struct {
fired int32
c testutils.Channel{}
o sync.Once
}
A little guidance would be lovely, thank you!
@Aditya-Sood Yes, this is still relevant, but very low priority. I replied on your other PR about some additional changes that we require on the xDS client and config_selector. That work is higher priority and definitely more interesting than this one.
Sure, the other one with the LRS loads would be higher priority for sure. I'll let you pick which one you want to work on.