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/federation: e2e tests #5103

Merged
merged 4 commits into from
Jan 7, 2022
Merged

xds/federation: e2e tests #5103

merged 4 commits into from
Jan 7, 2022

Conversation

menghanl
Copy link
Contributor

@menghanl menghanl commented Jan 5, 2022

Also moved func BuildResourceName to testutils to be shared

RELEASE NOTES: N/A

move func build resource name to testutils

[xds_fed_e2e_tests] 111

[xds_fed_e2e_tests] magic!
@menghanl menghanl requested a review from easwars January 5, 2022 00:01
@menghanl menghanl added this to the 1.44 Release milestone Jan 5, 2022
"google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version"
)

func BuildResourceName(typ xdsresource.ResourceType, auth, id string, ctxParams map[string]string) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a comment to make vet happy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 50 to 54
defer func() func() {
old := envconfig.XDSFederation
envconfig.XDSFederation = true
return func() { envconfig.XDSFederation = old }
}()()
Copy link
Contributor

Choose a reason for hiding this comment

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

This double func is super hard to read. Why do we need to do it this way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted

if err != nil {
t.Fatalf("Failed to create xDS resolver for testing: %v", err)
}
port, cleanup2 := clientSetup(t, &testService{})
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: there is no var cleanup for this to be cleanup2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@easwars easwars assigned menghanl and unassigned easwars Jan 5, 2022
@menghanl menghanl assigned easwars and unassigned menghanl Jan 6, 2022
@easwars easwars assigned menghanl and unassigned easwars Jan 7, 2022
@menghanl menghanl merged commit 77b478d into grpc:master Jan 7, 2022
@menghanl menghanl deleted the xds_fed_test branch January 7, 2022 19:26
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants