Skip to content
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

xds: Fake control plane test setup code to Rules #9666

Merged
merged 1 commit into from Nov 3, 2022

Conversation

temawi
Copy link
Contributor

@temawi temawi commented Nov 1, 2022

This extracts the startup and shutdown code for the control and data plane server to separate JUnit rules, which allows this logic to be reused in other tests in a simple manner. Also makes the test easier to read with the boiler plate init code removed.

Comment on lines +114 to +116
nameResolverProvider = XdsNameResolverProvider.createForTest(SCHEME,
defaultBootstrapOverride());
NameResolverRegistry.getDefaultRegistry().register(nameResolverProvider);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it better to move to data plane?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I included it here because my thinking was that this rule handles everything needed to make a control plane available to use, not just the server startup. And since registering this test nameserver is part of that, I'd keep it here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see. My thinking was that this belongs to Data Plane while this file is called Control Plane. But thats monior.

This extracts the startup and shutdown code for the control and data
plane server to reparate JUnit rules, which allows this logic to be
resued in other tests in a simple manner. Also makes the test easier to
read with the boiler plate init code removed.
@temawi temawi force-pushed the xds-test branch 2 times, most recently from 82cb3d3 to d6bbcad Compare November 3, 2022 00:14
Copy link
Contributor

@YifeiZhuang YifeiZhuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you for the change, its great!

@temawi temawi merged commit c1d0e14 into grpc:master Nov 3, 2022
@temawi temawi deleted the xds-test branch November 3, 2022 17:49
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants