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

Adjust randomization in ResolveClusterActionResponseTests #105932

Merged

Conversation

quux00
Copy link
Contributor

@quux00 quux00 commented Mar 4, 2024

to avoid failures in testEqualsAndHashcode tests.

Fixes #105898

@quux00 quux00 added >non-issue :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v8.14.0 labels Mar 4, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@quux00 quux00 requested a review from javanna March 4, 2024 22:11
Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

LGTM, I added a comment that might make this fix even more succinct though.

@@ -52,6 +63,6 @@ protected Writeable.Reader<ResolveClusterActionResponse> instanceReader() {

@Override
protected ResolveClusterActionResponse mutateInstance(ResolveClusterActionResponse response) {
return new ResolveClusterActionResponse(randomResolveClusterInfoMap());
return new ResolveClusterActionResponse(randomResolveClusterInfoMap(() -> randomResolveClusterInfoWithErrorString()));
Copy link
Member

Choose a reason for hiding this comment

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

This is fine, but alternatively you could use the "randomValueOtherThan" helper method from ESTestCase that randomizes until it has created an object thats not equal to the input. Might save a bit of extra code here and be a bit more readably on the long run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, cool. Thanks for the tip. Very helpful method! I've changed the code to use on the next push.

@quux00 quux00 merged commit f1035bb into elastic:main Mar 5, 2024
14 checks passed
jedrazb pushed a commit to jedrazb/elasticsearch that referenced this pull request Mar 6, 2024
gmarouli pushed a commit to gmarouli/elasticsearch that referenced this pull request Mar 6, 2024
fang-xing-esql pushed a commit to fang-xing-esql/Elasticsearch that referenced this pull request Mar 8, 2024
quux00 added a commit to quux00/elasticsearch that referenced this pull request Mar 26, 2024
quux00 added a commit that referenced this pull request Mar 26, 2024
…106756)

to avoid failures in `testEqualsAndHashcode` tests.

Fixes #105898
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] ResolveClusterActionResponseTests testEqualsAndHashcode failing
4 participants