StringSubject.isNullOrEmpty
I just migrated our tests from fest-assert to Truth. It was almost drop-in replacement, or I just had to do mass replacement of containsOnly to containsExactly, etc. , but I didn't find the replacement for assertThat("string").isNullOrEmpty and assertThat("String").isEqualToIgnoringCase("string").
Would you accept PR with these two methods added to StringSubject?
Do you have many tests with isNullOrEmpty() method in your project? I think, you can split test like that into two separate unit tests and then use isNull() and isEmpty() methods in a separate assertions. After that, you'll have more detailed and granulated tests. Nevertheless, isNullOrEmpty() may be useful method for some purposes.
Additionally, as far as I can see, method for isEqualToIngoreCase(string) is created in PR #181, but it wasn't merged to the master branch yet.
I have missed the other PR somehow, thanks for pointing it out.
I have very few isNullOrEmpty() usages. For now I'm just doing assertThat(Strings.isNullOrEmpty()).isTrue(), and I'll probably end up changing it to assertThat(string).isNull() and enforcing this contract. I just thought this might be useful for other developers switching to Truth from other assertion libs.
I think, idea of isNullOrEmpty() method is not bad in terms of migration from other testing frameworks as you wrote. Let's see what's opinion of the maintainers of this project. :-)
I think it's pretty rare that a test wants to allow either null or empty string. If the code under test is deterministic, then the string will either be null or empty string.
I realize there's not 100% overlap between fest and Truth, but that's never been an explicit goal.
I'm going to close this issue, but please follow along on #181 - I'm also skeptical of that assertion (for similar reasons), but any additional data you can add to the request would be great.
It turns out not to be as rare as one might think, especially where string parameters are handled or in systems with stringly typing. I see a lot of isNullOrEmpty logic in Android contexts. It definitely hits us on a FEST/AssertJ migration, but I also think we have examples where the logic really is or-linked. The value can be null, or it can be empty, and these both mean the same thing.
Moving to Kotlin, we'd probably do something like: assertThat(foo ?: "").isEmpty() but again, that's massaging the subject's actual() ahead of time just to make the assertion work, rather than having the actual assertion available.
I can't open this issue (no permissions) but I think it's worth re-opening the discussion.
It occurs to me that you can write:
assertThat(foo).isAnyOf(null, "");
I'm sure it gets old when you have to write it a lot, though, especially since it doesn't autocomplete nicely.
And, to copy what I said over email:
I suspect that Kurt felt that it would get overused as the "safe" option, even when it's not justified. But I do see a number of assertThat(isNullOrEmpty(...)).isTrue() calls inside Google, so feel free to reopen, and we'll have a look "someday." I know that it comes up for URL parameters, among other things.
... damn. I didn't think at all about assertThat(foo).isAnyOf(null, ""); . I actually kind of like it, though yeah, it's not obviously completable.
The other thing is that you can't extend this to whitespace. But that might be too much to ask. We have some "is null or empty or whitespace" stuff, which ends up being assertThat(foo.trim())...
We implemented an alternate string subject with "compressingWhitespace" which is more heavy handed, but also an AssertJ migration aide. 🤷♂️
Getting back to the thread - yeah, url parameters and other passed parameters are the key here, I think. As to Kurt's comment about default safe option... it is a slightly "looser" check, and that can result in slightly looser behavior, but at some point it's up to developers if they want their APIs returning null, empty, or both. We can't enforce that, especially when they have received APIs that are this sloppy. We can fix a lot with tools, but not every code smell.
We added support for ignoring case, so I'll retitle this bug.