Re: [proposal] Introduce AssertJ in test framework

2020-03-11 Thread Stephen Mallette
>  (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 
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
>  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 
> 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 
> 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 
> 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 
> > 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_=DwIFaQ=adz96Xi0w1RHqtPMowiL2g=Jad7nE1Oab1mebx31r7AOfSsa0by8th6tCxpykmmOBA=WrkWi_LeOnAl7rqft1DM27OEkXD7sc2fnZMy_-c7IS8=D4FAaGRQi2WlwKAOQbQMfyt_cRqsOuZdePUDgchdLhA=
> > > > > > >
> > > > > > > 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")
> > > > > > > 

Re: [proposal] Introduce AssertJ in test framework

2020-03-11 Thread Kevin Gallardo
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
 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 
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  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  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 
> 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 
> > > > 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_=DwIFaQ=adz96Xi0w1RHqtPMowiL2g=Jad7nE1Oab1mebx31r7AOfSsa0by8th6tCxpykmmOBA=WrkWi_LeOnAl7rqft1DM27OEkXD7sc2fnZMy_-c7IS8=D4FAaGRQi2WlwKAOQbQMfyt_cRqsOuZdePUDgchdLhA=
> > > > > >
> > > > > > 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 

Re: [proposal] Introduce AssertJ in test framework

2020-03-10 Thread Joshua McKenzie
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  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  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  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 
> > > 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_=DwIFaQ=adz96Xi0w1RHqtPMowiL2g=Jad7nE1Oab1mebx31r7AOfSsa0by8th6tCxpykmmOBA=WrkWi_LeOnAl7rqft1DM27OEkXD7sc2fnZMy_-c7IS8=D4FAaGRQi2WlwKAOQbQMfyt_cRqsOuZdePUDgchdLhA=
> > > > >
> > > > > 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.
> > > > > 

Re: [proposal] Introduce AssertJ in test framework

2020-03-10 Thread DuyHai Doan
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  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  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 
> > 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_=DwIFaQ=adz96Xi0w1RHqtPMowiL2g=Jad7nE1Oab1mebx31r7AOfSsa0by8th6tCxpykmmOBA=WrkWi_LeOnAl7rqft1DM27OEkXD7sc2fnZMy_-c7IS8=D4FAaGRQi2WlwKAOQbQMfyt_cRqsOuZdePUDgchdLhA=
> > > >
> > > > 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 

Re: [proposal] Introduce AssertJ in test framework

2020-03-10 Thread Jordan West
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 
wrote:

> +1
>
> On Tue, Mar 10, 2020 at 6:18 PM Jon Haddad  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 
> 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_=DwIFaQ=adz96Xi0w1RHqtPMowiL2g=Jad7nE1Oab1mebx31r7AOfSsa0by8th6tCxpykmmOBA=WrkWi_LeOnAl7rqft1DM27OEkXD7sc2fnZMy_-c7IS8=D4FAaGRQi2WlwKAOQbQMfyt_cRqsOuZdePUDgchdLhA=
> > >
> > > 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.
> > > >
> > >
> >
>


Re: [proposal] Introduce AssertJ in test framework

2020-03-10 Thread Benjamin Lerer
+1

On Tue, Mar 10, 2020 at 6:18 PM Jon Haddad  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  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_=DwIFaQ=adz96Xi0w1RHqtPMowiL2g=Jad7nE1Oab1mebx31r7AOfSsa0by8th6tCxpykmmOBA=WrkWi_LeOnAl7rqft1DM27OEkXD7sc2fnZMy_-c7IS8=D4FAaGRQi2WlwKAOQbQMfyt_cRqsOuZdePUDgchdLhA=
> >
> > 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.
> > >
> >
>


Re: [proposal] Introduce AssertJ in test framework

2020-03-10 Thread Jon Haddad
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  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 
> wrote:
>
> > I would like to propose adding AssertJ 
> 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.
> >
>


Re: [proposal] Introduce AssertJ in test framework

2020-03-10 Thread David Capwell
+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 
wrote:

> I would like to propose adding AssertJ  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.
>


[proposal] Introduce AssertJ in test framework

2020-03-10 Thread Kevin Gallardo
I would like to propose adding AssertJ  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.