xds/resolver: cleanup tests to use real xDS client 3/n

[easwars]

#resource-agnostic-xdsclient-api

RELEASE NOTES: n/a

[dfawley]

FYI: this seems okay as long as everywhere we read from this channel is resilient - as in, if the expected value isn't read, it reads again. Otherwise, you can run into races if you read from the channel before it's written.

And even then, we can have false positives. E.g.:

1. Random setup thing happens that pushes the expected value.
2. Test condition occurs, which will result in pushing something bad here.
3. Read expected value from channel and pass test.
4. Bad value we were hoping to test is actually pushed.

Forcing pulling every update off the channel effectively ensures the above won't happen.

[dfawley]

It seems both options here have problems, so I'm fine with this change.

[dfawley]

Should this use the same channel as above? The signals are supposed to be mutually exclusive, and at some point we may want to refactor things to make the two conditions happen via the same call.

[easwars]

I switched to Replace() mainly to workaround go-control-plane's behavior when the xDS client NACKs an update. The go-control-plane keeps resending the same update until it is ACKed. In one of our test scenarios where we test that a NACKed update results in the error being propagated to the channel, the test logic only reads the error out of the testClientConn once. But since the go-control-plane keeps sending the same update again and again, the resolver receives the same error from the xDS client and has to push it up to the ClientConn. Without a Replace() here, the test will fail with a leaked goroutine.

Changing this to use the same channel will make things more complicated, because I cannot reliably call Replace() because I won't know whether the item that I'm pulling from the channel is an update or an error.

[dfawley]

So what will we do if we refactor the resolver API to merge ReportError and Update? We can still use a different channel if the Update reflects an error, I suppose?

[easwars]

Yes, that seems doable to me. I think we have this testClientConn implementation in a few other places as well. So, we would have to get to them as well.