[TEST] Fixes to StringMatcherTests #88046
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Under some seeds (e.g. 3C93AEF7628D6611) prefix2 would start with prefix1, which would mean that input that was intended to test against prefix2 would actually match prefix1 Resolves: elastic#88024
In Hamcest there is now randomFrom(char[]) so this code:
randomFrom( "abc".toCharArray() )
will be treated as
randomFrom( new Object[] { new char[] { 'a', 'b', 'c' } } )
and will return a char[]
The existing code that was intending to return a random char, was
actually returning a char[] and then stringifying that into something
like
[C@5e67f506
tvernum
added
>test
|
Pinging @elastic/es-security (Team:Security) |
n1v0lg
approved these changes
Jun 27, 2022
| ); | ||
|
|
||
| final List<Character> nonAlpha = "@/#$0123456789()[]{}<>;:%&".chars() | ||
| .mapToObj(c -> Character.valueOf((char) c)) |
There was a problem hiding this comment.
Optional nit: I think boxing happens automatically here, i.e., c -> (char) c) is sufficient.
There was a problem hiding this comment.
It would, though I think explicit is preferable in this case.
I guess it's just going to come down to personal preference, but I like the clarity of the explicit boxing.
tvernum
added a commit
to tvernum/elasticsearch
that referenced
this pull request
Jun 28, 2022
This commit fixes two issues with StringMatcherTests
**Problem 1**
Under some seeds (e.g. 3C93AEF7628D6611) prefix2 would start
with the same text as prefix1. This would mean that input that was
intended to test against prefix2 would actually match prefix1, and
violate the expectations of the test
**Problem 2**
The code had the equivalent of
randomFrom( "abc".toCharArray() )
But Hamcest does not have a randomFrom(char[]) method, so this
code was treated as
randomFrom( new Object[] { new char[] { 'a', 'b', 'c' } } )
and will return a char[]. Tests that were written assuming they had
a random character, actually had an array (char[]) and would then
stringify that object into something like
[C@5e67f506
Resolves: elastic#88024
💚 Backport successful
|
elasticsearchmachine
pushed a commit
that referenced
this pull request
Jun 28, 2022
This commit fixes two issues with StringMatcherTests
**Problem 1**
Under some seeds (e.g. 3C93AEF7628D6611) prefix2 would start
with the same text as prefix1. This would mean that input that was
intended to test against prefix2 would actually match prefix1, and
violate the expectations of the test
**Problem 2**
The code had the equivalent of
randomFrom( "abc".toCharArray() )
But Hamcest does not have a randomFrom(char[]) method, so this
code was treated as
randomFrom( new Object[] { new char[] { 'a', 'b', 'c' } } )
and will return a char[]. Tests that were written assuming they had
a random character, actually had an array (char[]) and would then
stringify that object into something like
[C@5e67f506
Resolves: #88024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit fixes two issues with StringMatcherTests
Problem 1
Under some seeds (e.g. 3C93AEF7628D6611) prefix2 would start
with the same text as prefix1. This would mean that input that was
intended to test against prefix2 would actually match prefix1, and
violate the expectations of the test
Problem 2
The code had the equivalent of
But Hamcest does not have a randomFrom(char[]) method, so this
code was treated as
and will return a char[]. Tests that were written assuming they had
a random character, actually had an array (char[]) and would then
stringify that object into something like
Resolves: #88024