Hi, Once you get to know assertJ, it's impossible to go back to basic > assertions of JUnit
Definitely with you on that :) Great to see people are excited about this, thanks for the responses. Given the positive feedback I have created CASSANDRA-15631 <https://issues.apache.org/jira/browse/CASSANDRA-15631> as a first step. My proposal to try to get this going smoothly would be: 1. introduce the dependency and rewrite an existing test suite to showcase it as a reference 2. encourage new tests to be written with AssertJ once the dep is added 3. add a mention of the preferred assertion library in the contributing guidelines and link to the reference for an example (4. potentially migrating other existing tests to AssertJ in the long term) wdyt? On Tue, Mar 10, 2020 at 6:24 PM Joshua McKenzie <jmcken...@apache.org> wrote: > Strong +1 here. Many of you know I'm a C# / LINQ junkie though. ;) > > On Tue, Mar 10, 2020 at 3:55 PM DuyHai Doan <doanduy...@gmail.com> wrote: > > > Definitely +1 > > > > Coding in Java every day, I can't write test without assertJ. Once you > get > > to know assertJ, it's impossible to go back to basic assertions of JUnit > > that feels really boilerplate > > > > > > > > On Tue, Mar 10, 2020 at 8:53 PM Jordan West <jorda...@gmail.com> wrote: > > > > > If it encourages more and higher quality test writing +1 (nb). Also, > low > > > risk given it’s a test dep. > > > > > > Using QuickTheories as an example, merging it with a new or updated > test > > > could be a good way to get it merged > > > > > > Jordan > > > > > > On Tue, Mar 10, 2020 at 10:33 AM Benjamin Lerer < > > > benjamin.le...@datastax.com> > > > wrote: > > > > > > > +1 > > > > > > > > On Tue, Mar 10, 2020 at 6:18 PM Jon Haddad <j...@jonhaddad.com> > wrote: > > > > > > > > > I've used assertj in a lot of projects, I prefer it by a wide > margin > > > over > > > > > using only junit. > > > > > > > > > > On Tue, Mar 10, 2020 at 9:45 AM David Capwell <dcapw...@gmail.com> > > > > wrote: > > > > > > > > > > > +1 from me > > > > > > > > > > > > In CASSANDRA-15564 I build my own assert chain to make the tests > > > > cleaner; > > > > > > did it since assertj wasn't there. > > > > > > > > > > > > On Tue, Mar 10, 2020, 9:28 AM Kevin Gallardo < > > > > > kevin.galla...@datastax.com> > > > > > > wrote: > > > > > > > > > > > > > I would like to propose adding AssertJ < > > > > > > > > > > > > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__assertj.github.io_doc_&d=DwIFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=Jad7nE1Oab1mebx31r7AOfSsa0by8th6tCxpykmmOBA&m=WrkWi_LeOnAl7rqft1DM27OEkXD7sc2fnZMy_-c7IS8&s=D4FAaGRQi2WlwKAOQbQMfyt_cRqsOuZdePUDgchdLhA&e= > > > > > > > > > > > > as > > > > > > > a test dependency and therefore have it available for writing > > > > > > > unit/distributed/any test assertions. > > > > > > > > > > > > > > In addition to the examples mentioned on the AssertJ docs page > > > > (allows > > > > > to > > > > > > > do elaborate and comprehensible assertions on Collections, > > String, > > > > and > > > > > > > *custom > > > > > > > assertions*), here's an example of a dtest I was looking at, > that > > > > could > > > > > > be > > > > > > > translated to AssertJ syntax, just to give an idea of how the > > > syntax > > > > > > would > > > > > > > apply: > > > > > > > > > > > > > > *JUnit asserts*: > > > > > > > try { > > > > > > > [...] > > > > > > > } catch (Exception e) { > > > > > > > Assert.assertTrue(e instanceof RuntimeException); > > > > > > > RuntimeException re = ((RuntimeException) e); > > > > > > > Assert.assertTrue(re.getCause() instanceof > > > ReadTimeoutException); > > > > > > > ReadTimeoutException rte = ((ReadTimeoutException) > > > e.getCause()); > > > > > > > Assert.assertTrue(rte.getMessage().contains("blabla") > > > > > > > && > rte.getMessage().contains("andblablo")); > > > > > > > } > > > > > > > > > > > > > > *AssertJ style:* > > > > > > > try { > > > > > > > [...] > > > > > > > } catch (Exception e) { > > > > > > > Assertions.assertThat(e) > > > > > > > .isInstanceOf(RuntimeException.class) > > > > > > > > > .hasCauseExactlyInstanceOf(ReadTimeoutException.class) > > > > > > > .hasMessageContaining("blabla") > > > > > > > .hasMessageContaining("andblablo"); > > > > > > > } > > > > > > > > > > > > > > The syntax is more explicit and more comprehensible, but more > > > > > > importantly, > > > > > > > when one of the JUnit assertTrue() fails, you don't know *why*, > > you > > > > > only > > > > > > > know that the resulting boolean expression is false. > > > > > > > If a failure happened with the assertJ tests, the failure would > > say > > > > > > > "Exception > > > > > > > did not contain expected message, expected "blabla", actual > > > > > "notblabla"" > > > > > > > (same for a lot of other situations), this makes debugging a > > > failure, > > > > > > after > > > > > > > a test ran and failed much easier. With JUnit asserts you would > > > have > > > > to > > > > > > > additionally add a message explaining what the expected value > is > > > > *and* > > > > > > > what the > > > > > > > actual value is, for each assert that is more complex than a > > > > > assertEquals > > > > > > > on a number, I suppose. I have seen a lot of tests so far that > > only > > > > > test > > > > > > > the expected behavior via assertTrue and does not show the > > > incorrect > > > > > > values > > > > > > > when the test fails, which would come for free with AssertJ. > > > > > > > > > > > > > > Other examples randomly picked from the test suite: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > *org.apache.cassandra.repair.RepairJobTest#testNoTreeRetainedAfterDistance:* > > > > > > > Replace assertion: > > > > > > > assertTrue(messages.stream().allMatch(m -> m.verb() == > > > > Verb.SYNC_REQ)); > > > > > > > With: > > > > > > > assertThat(messages) > > > > > > > .extracting(Message::verb) > > > > > > > .containsOnly(Verb.SYNC_REQ); > > > > > > > > > > > > > > As a result, if any of the messages is not a Verb.SYNC_REQ, the > > > test > > > > > > > failure will show the actual "Verb"s of messages. > > > > > > > > > > > > > > Replace: > > > > > > > assertTrue(millisUntilFreed < TEST_TIMEOUT_S * 1000); > > > > > > > With: > > > > > > > assertThat(millisUntilFreed) > > > > > > > .isLessThan(TEST_TIMEOUT_S * 1000); > > > > > > > > > > > > > > Same effect if the condition is not satisfied, more explicit > > error > > > > > > message > > > > > > > explaining why the test failed. > > > > > > > > > > > > > > AssertJ also allows Custom assertions which are also very > useful > > > and > > > > > > could > > > > > > > potentially be leveraged in the future. > > > > > > > > > > > > > > This would only touch on the tests' assertions, the rest of the > > > test > > > > > > setup > > > > > > > and execution remains untouched (still uses JUnit for the test > > > > > > execution). > > > > > > > > > > > > > > Thanks. > > > > > > > > > > > > > > -- > > > > > > > Kévin Gallardo. > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- Kévin Gallardo. kgdo.me Senior Software Engineer at DataStax. <http://datastax.com>
