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: fix the EDS dups test so it doesn't assume address order #9786

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

sergiitk
Copy link
Member

@sergiitk sergiitk commented Jan 4, 2023

Fixes RingHashLoadBalancerTest.duplicateAddresses() test.

Multiset entries are not ordered. This PR updates the test so it's not assuming the order of addresses in the error message.

@sergiitk sergiitk requested a review from larry-safran January 4, 2023 17:48
@sergiitk
Copy link
Member Author

sergiitk commented Jan 4, 2023

cc @ejona86 - could you confirm if we should care about the order in the error (status) description? We could use the ordered multiset if we want to make it deterministic.

+ "addresses: Address: FakeSocketAddress-server1, count: 2; "
+ "Address: FakeSocketAddress-server2, count: 3");
String description = result.getStatus().getDescription();
assertThat(description).startsWith(
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't really needed. If someone decides to change the text in the future that is fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

We're always on the fence whether error messages should be tested. Anyway, I've noticed it after merging :)

@larry-safran
Copy link
Contributor

The order doesn't matter. I added the detail of which addresses and the count of them for convenience of debugging.
(If we did care about order, it would be alphabetical rather than the order they were sent from the server).

@sergiitk sergiitk merged commit 040e283 into grpc:master Jan 4, 2023
@sergiitk sergiitk deleted the xds-fix-ringhash-dups-test branch January 4, 2023 18:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 5, 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.

2 participants