Port all Junit assert to Truth asserts#2304
Conversation
Marcono1234
left a comment
There was a problem hiding this comment.
Thanks! Might be useful to adjust the tests to use Truth more effectively, for example in some cases there are assertions such as assertThat(... == ...).isTrue() (though these manual comparisons also existed before your changes).
I have started commenting on some of the cases below, but stopped after I noticed that commenting on every case is probably not worth it and useful.
Though I don't know how important Éamonn considers these adjustments. So feel free to ignore my suggestion.
To port this amount of tests to Truth I had to use Regex (where was possible) because do it manually would have taken too long. There are a tons of things that can be changed or/and optimized but I don't know if it's worth. My main idea was: Port the tests without change things that are not related to Anyway, thanks for the review and comments! 😄 |
eamonnmcmanus
left a comment
There was a problem hiding this comment.
We can tweak these assertions more or less indefinitely. I think it is fine if you don't. Perhaps just do the ones that @Marcono1234 commented on. This is still an improvement, and we may be able to use Error Prone to pick up further simplifications.
I already looked how to setting up the |
|
OK, I think this is a good step on the way. Thanks again! |
Marcono1234
left a comment
There was a problem hiding this comment.
Thanks for addressing my comments!
Sorry for this late follow-up review; I think in two cases the array comparison should use inOrder(). Though this might not be so important.
| for (int i = 0; i < expected.length; ++i) { | ||
| assertThat(target[i]).isEqualTo(expected[i]); | ||
| } | ||
| assertThat(target).asList().containsAnyIn(expected); |
There was a problem hiding this comment.
Should probably be containsExactly(...).inOrder() as well.
| assertThat(target).asList().containsAnyIn(expected); | |
| assertThat(target).asList().containsExactly(expected).inOrder(); |
| String[] target = gson.fromJson(json, String[].class); | ||
| assertThat(target[0]).isEqualTo("Hello"); | ||
| assertThat(target[1]).isEqualTo("World"); | ||
| assertThat(target).asList().containsExactly("Hello", "World"); |
There was a problem hiding this comment.
| assertThat(target).asList().containsExactly("Hello", "World"); | |
| assertThat(target).asList().containsExactly("Hello", "World").inOrder(); |
* Port Junit assert to Truth in `com.google.gson.stream` * Port Junit assert to Truth in `com.google.gson.regression` * Port Junit assert to Truth in `om.google.gson.reflect` * Port Junit assert to Truth in `com.google.gson.metrics` * Port Junit assert to Truth in `com.google.gson.internal` * Port Junit assert to Truth in `com.google.gson.internal.sql` * Port Junit assert to Truth in `com.google.gson.internal.reflect` * Port Junit assert to Truth in `com.google.gson.internal.bind` * Port Junit assert to Truth in `com.google.gson.internal.bind.util` * Port Junit assert to Truth in `com.google.gson.functional` * Replaces `List.of` with `Arrays.asList` to grant legacy * Simplify `==` asserts * Simplify `.contain()` asserts + Minor fixes * Simplify asserts
All test inside the path
gson/gson/src/testare ported to Truth