-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Introduce ESTestCase.randomArrayOtherThan() methods #136707
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
Introduce ESTestCase.randomArrayOtherThan() methods #136707
Conversation
The existing randomValueOtherThan() method uses Objects.equals() to determine if a randomly generated object is equal to the provided argument. This approach does not work as expected for arrays, however, since two arrays with identical contents will not be considered equal by Objects.equals(). See elastic#136652 This commit introduces randomValueOtherThanArray() methods for Object arrays and primitive arrays and converts all places in the code that were previously passing arrays into randomValueOtherThan() to use the new array-specific methods
Pinging @elastic/ml-core (Team:ML) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/** | ||
* helper to get a random value in a certain range that's different from the input, for object arrays | ||
*/ | ||
public static <T> T[] randomValueOtherThanArray(T[] input, Supplier<T[]> randomSupplier) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it would make sense to rename this method to randomArrayOtherThan
or similar to avoid the impression that the method is actually not returning array?
Maybe it's not an issue because the randomValueOtherThanMany
also sounds a bit strange to me.
I'm leaving it up to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a better name, yeah
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Good stuff!
The existing randomValueOtherThan() method uses Objects.equals() to determine if a randomly generated object is equal to the provided argument. This approach does not work as expected for arrays, however, since two arrays with identical contents will not be considered equal by Objects.equals(). See #136652
This commit introduces randomArrayOtherThan() methods for Object arrays and primitive arrays and converts all places in the code that were previously passing arrays into randomValueOtherThan() to use the new array-specific methods