Re: AssertJ for Assertions

Thanks! This looks good to me! -Jack Ye On Thu, Jun 17, 2021 at 12:14 AM Eduard Tudenhoefner wrote: > Thanks for bringing this up Jack. The reason for this is actually that I > mistakenly applied "*withFailMessage()*" instead of "*as(..)*" in > *AssertHelpers* and therefore the actual error repo

Re: AssertJ for Assertions

Thanks for bringing this up Jack. The reason for this is actually that I mistakenly applied "*withFailMessage()*" instead of "*as(..)*" in *AssertHelpers* and therefore the actual error reporting was overwritten. I opened PR#2706 to fix this. This is h

Re: AssertJ for Assertions

After submitting a few new PRs recently, I propose that we revert the changes made in the PR to use AssertJ in AssertHelpers. The change made the error message very unclear for what was causing the assertion to fail. For example, if I change the expected error message in a test, the current junit

Re: AssertJ for Assertions

I left a couple of review comments on the PR. Thanks, Eduard! On Thu, Jun 10, 2021 at 2:42 AM Eduard Tudenhoefner wrote: > Yes we're definitely not talking about rewriting lots of test code. @Ryan > would you like to review/comment/approve the PR then if you think the > change is good going forw

Re: AssertJ for Assertions

Yes we're definitely not talking about rewriting lots of test code. @Ryan would you like to review/comment/approve the PR then if you think the change is good going forward? Thanks for your feedback. On Thu, Jun 10, 2021 at 1:36 AM Ryan Blue wrote: > Thanks for the additional examples, Eduard. I

Re: AssertJ for Assertions

Thanks for the additional examples, Eduard. I do like the assertions here and I'm fine with the idea of gradually moving over to them. Looks like some of the CharSequence and collection comparators would definitely be easier to use than `assertEquals(ImmutableSet.of(...), actualSet)`. As long as w

Re: AssertJ for Assertions

Thanks guys for your feedback. The idea was to not replace existing code when introducing AssertJ as that would probably rather cause merge conflicts for a lot of people. The idea was rather to give people a (better) alternative when testing certain things, such as collections, exceptions, paths,

Re: AssertJ for Assertions

I mostly agree with Jack. I think that if I were starting a new project, I'd probably want to use the new assertions because they are readable, appear to be type-specific, and have some nice helpers for some types. But I don't see a lot of benefit to moving over to them right now and it would be a

Re: AssertJ for Assertions

I would be on the more cautious side when introducing a new test utils library. Based on the PR, we are mostly changing things like Assert.assertEquals to another syntax, but that syntax is not complex in the first place. If we introduce AssertJ, there will be a mixture of 2 syntaxes, which is conf

AssertJ for Assertions

Hi everyone, I was wondering what the appetite would be for introducing AssertJ to the project? I believe it's a really good testing library that makes writing assertions much more intuitive, as the assertions are written in kind-of a fluent way. The test code ends