-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
xdsclient: improve authority watchers test #5700
Conversation
cc51ec3
to
85ef8b8
Compare
85ef8b8
to
593edc3
Compare
Ping @dfawley |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I'm a bit concerned about the classification of these as "end to end tests", since they don't actually test gRPC servers or clients, or anything we expose externally to the user for that matter. These are really system level tests for the xdsclient package, right? (And as such, probably belong under the xdsclient directory somewhere instead, as they were before.)
These are e2e tests from the point of view of the xds client, since it uses only exported APIs (and creates a new client and tests the scenarios being tested). The main reason I want these tests in a separate package is because our previous tests lived in the same package and ended up relying too much on internal state and being over-specified. I could have put them in the same directory, but in an |
That's fine, but I do think a package named |
Fair point. I can change that once the last test PR (in this round) is in. |
Improve authority watchers test by moving to an e2e style test which verifies functionality without relying on any internal state.
This PR is similar to how the LDS watchers test was cleaned up in #5506.
#resource-agnostic-xdsclient-api
RELEASE NOTES: n/a