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.

Sign up for GitHub

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

Jump to bottom

xdsclient: improve LDS watchers test #5691

Merged
merged 3 commits into from
Oct 11, 2022
Merged

xdsclient: improve LDS watchers test #5691

merged 3 commits into from
Oct 11, 2022

Conversation

easwars
Copy link
Contributor

@easwars easwars commented Oct 6, 2022

Improve the existing LDS watchers test:

  • For tests which have subtests, move the creation of the xdsclient into t.Run(). This will remove the flakiness we have been observing on GitHub Actions.
  • Add a couple of more tests. These scenarios were being tested for some of the other xDS watchers. So, I added them here as well.
  • Change the NewWithConfigForTesting API to allow configuring the authority idle timeout. This will be used in upcoming PRs.

#resource-agnostic-xdsclient-api

Fixes #5643

RELEASE NOTES: n/a

@easwars easwars requested a review from zasweq October 6, 2022 16:51
@easwars easwars added this to the 1.51 Release milestone Oct 6, 2022
zasweq
zasweq reviewed Oct 11, 2022
View reviewed changes
Copy link
Contributor

@zasweq zasweq left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the flake. Some minor comments.

xds/internal/xdsclient/client_new.go Outdated Show resolved Hide resolved
xds/internal/xdsclient/e2e_test/lds_watchers_test.go Show resolved Hide resolved
xds/internal/xdsclient/e2e_test/lds_watchers_test.go Show resolved Hide resolved
xds/internal/xdsclient/e2e_test/lds_watchers_test.go Show resolved Hide resolved
xds/internal/xdsclient/e2e_test/lds_watchers_test.go Outdated Show resolved Hide resolved
xds/internal/xdsclient/e2e_test/lds_watchers_test.go Outdated Show resolved Hide resolved
@zasweq zasweq assigned easwars and unassigned zasweq Oct 11, 2022
@easwars easwars assigned zasweq and unassigned easwars Oct 11, 2022
zasweq
zasweq approved these changes Oct 11, 2022
View reviewed changes
Copy link
Contributor

@zasweq zasweq left a comment

Choose a reason for hiding this comment

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

LGTM.

@zasweq zasweq assigned easwars and unassigned zasweq Oct 11, 2022
@easwars easwars merged commit e81d0a2 into grpc:master Oct 11, 2022
1 check passed
@easwars easwars deleted the lds_watchers_test branch October 11, 2022 23:37
easwars added a commit to easwars/grpc-go that referenced this pull request Oct 11, 2022
@easwars
rebase and pull in comments from grpc#5691
e73d362
easwars added a commit to easwars/grpc-go that referenced this pull request Oct 11, 2022
@easwars
rebase and pull in comments from grpc#5691
e6d4baa
easwars added a commit to easwars/grpc-go that referenced this pull request Oct 12, 2022
@easwars
rebase and pull in comments from grpc#5691
13cb06f
easwars added a commit to easwars/grpc-go that referenced this pull request Oct 19, 2022
@easwars
rebase and pull in comments from grpc#5691
3364817
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@zasweq zasweq zasweq approved these changes

Assignees

@easwars easwars

Labels
Type: Testing
Projects
None yet
Milestone
1.51 Release
Development

Successfully merging this pull request may close these issues.

flaky test: Test/LDSWatch
2 participants
@easwars @zasweq