Skip to content

xds: generic xds client ads stream tests#8307

Merged
purnesh42H merged 6 commits intogrpc:masterfrom
purnesh42H:generic-xds-client-ads-stream-tests
May 15, 2025
Merged

xds: generic xds client ads stream tests#8307
purnesh42H merged 6 commits intogrpc:masterfrom
purnesh42H:generic-xds-client-ads-stream-tests

Conversation

@purnesh42H
Copy link
Copy Markdown
Contributor

RELEASE NOTES: None

@purnesh42H purnesh42H requested review from dfawley and easwars May 9, 2025 15:59
@purnesh42H purnesh42H added Type: Testing Area: xDS Includes everything xDS related, including LB policies used with xDS. labels May 9, 2025
@purnesh42H purnesh42H added this to the 1.73 Release milestone May 9, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented May 9, 2025

Codecov Report

Attention: Patch coverage is 81.25000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 82.30%. Comparing base (d00f4ac) to head (19248d1).
Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
xds/internal/clients/xdsclient/ads_stream.go 73.91% 4 Missing and 2 partials ⚠️
xds/internal/clients/internal/testutils/channel.go 75.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8307      +/-   ##
==========================================
+ Coverage   82.22%   82.30%   +0.07%     
==========================================
  Files         419      419              
  Lines       41954    42025      +71     
==========================================
+ Hits        34497    34589      +92     
+ Misses       5995     5979      -16     
+ Partials     1462     1457       -5     
Files with missing lines Coverage Δ
xds/internal/clients/xdsclient/xdsclient.go 80.25% <100.00%> (+2.05%) ⬆️
xds/internal/clients/internal/testutils/channel.go 89.28% <75.00%> (-10.72%) ⬇️
xds/internal/clients/xdsclient/ads_stream.go 79.77% <73.91%> (+0.14%) ⬆️

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.


// SetStreamBackOffForTesting override the default stream backoff function
// with provided function.
func (c *XDSClient) SetStreamBackOffForTesting(streamBackoff func(int) time.Duration) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please put this in an internal subdirectory and set it to an unexported function here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done for both expiry and backoff

@purnesh42H purnesh42H force-pushed the generic-xds-client-ads-stream-tests branch from 900a1bf to ffe6b75 Compare May 10, 2025 04:17
@purnesh42H purnesh42H requested a review from dfawley May 10, 2025 04:17
@purnesh42H purnesh42H force-pushed the generic-xds-client-ads-stream-tests branch from ffe6b75 to b9afcfa Compare May 10, 2025 04:22
@purnesh42H purnesh42H force-pushed the generic-xds-client-ads-stream-tests branch from b9afcfa to 72bdea5 Compare May 10, 2025 04:24
Copy link
Copy Markdown
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

I am going to defer to @easwars to review this since he has a lot more experience/familiarity with the original xdsclient and its tests.

@@ -0,0 +1,275 @@
/*
*
* Copyright 2024 gRPC authors.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this moved from elsewhere? You have several "new" files with 2024, so unless they're copies, please update.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah this is copied from internal and then modified. Similar to how done in previous PRs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see. That should make it a lot easier to review @easwars.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These tests needs to be deleted from internal xdsclient as part of migration PR #8310 as they need to override internal resource watching implementation.

@dfawley dfawley assigned purnesh42H and unassigned dfawley May 12, 2025
@purnesh42H purnesh42H removed their assignment May 12, 2025
@purnesh42H purnesh42H requested review from dfawley and easwars May 13, 2025 17:18
@purnesh42H purnesh42H assigned easwars and unassigned purnesh42H May 13, 2025
@easwars easwars assigned purnesh42H and unassigned easwars May 13, 2025
@purnesh42H purnesh42H assigned easwars and unassigned purnesh42H May 14, 2025
@easwars easwars assigned purnesh42H and unassigned easwars May 14, 2025
@purnesh42H purnesh42H force-pushed the generic-xds-client-ads-stream-tests branch from 3956e5b to 19248d1 Compare May 15, 2025 03:52
@purnesh42H purnesh42H merged commit 1ecde18 into grpc:master May 15, 2025
23 of 24 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Area: xDS Includes everything xDS related, including LB policies used with xDS. Type: Testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants