-
Notifications
You must be signed in to change notification settings - Fork 378
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
IsBlankString matcher is inconsistent with String.isBlank() #325
Comments
For those of us with more experience it doesn't imply anything at all, because "blank" is such an ambiguous concept that any method in a library claiming to detect "blankness" is immediately banned for use by anyone who cares about what they're letting into their project. My own Hamcrest cheat sheet marks them with skull marks for this reason: |
In fact, "blank" does have a precise meaning in the Java ecosystem as I pointed out in my original post -- it should be consistent with the I did learn from the associated PR that Hamcrest still supports older versions of Java that predate the If you don't want to use the |
No, in the "Java ecosystem" you speak of, there are multiple definitions of whitespace. Just because you only know of one of them does not mean that the others do not exist. Kevin Bourrillion compiled this spreadsheet: https://docs.google.com/spreadsheets/d/1kq4ECwPjHX9B8QUCTPclgsDCXYaj7T-FlT4tB5q3ahk/edit#gid=0 You're right that it's a deficiency that can be fixed, but perhaps it should be fixed by removing the method. |
Note that |
@peterdemaeyer understood, and I have updated the linked PR to remain compatible with that JDK. I do think that since we're currently in rampdown for JDK16, that it's reasonable that devs will start using As mentioned previously, |
The naming of the matcher implies it would give consistent results with
String.isBlank()
. However, it does not. The matcher uses a regex with pattern\\s*
which will only match ASCII whitespace characters.String.isBlank()
eventually delegates toCharacter.isWhitespace()
which checks for Unicode whitespace as well. See https://stackoverflow.com/a/57998178 for a detailed breakdown, summarized below:I discovered this when using JUnit Quickcheck with an assumption to prevent handling whitespace strings of the form:
Assume.assumeThat(string, not(blankString()));
The assumption passed, and my code would check the string via
String.isBlank()
and fail the test.The text was updated successfully, but these errors were encountered: