> (4. potentially migrating other existing tests to AssertJ in the long term)
Highest value conversions would probably be assertTrue/False() assertions as I believe those offer the least feedback on failure. I would imagine that exception assertions would follow that especially when leaning on @Test(expected = ...) sort of syntax or using explicit try/catch with fail() logic in tests. AssertJ exception assertions tend to be much more readable with test intention more clear. On Wed, Mar 11, 2020 at 10:59 AM Kevin Gallardo <kevin.galla...@datastax.com> wrote: > 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> >
