alexruiz / fest-assert-2.x

FEST Fluent Assertions 2.x
http://fest.easytesting.org
Apache License 2.0
402 stars 69 forks source link

Booleans and assertThat #136

Closed andrewrlee closed 11 years ago

andrewrlee commented 11 years ago

Hi,

I was wondering if it you have considered making single BooleanAsserts validate based on their sole call:

so you could do:

    assertThat(list.isEmpty());

rather than:

    assertThat(list.isEmpty()).isEqualTo(true);

When first using festAssertions I occasionally forgot and this lead to me being stung, by thinking that invalid assumptions were actually valid (as they were not being evaluated).

I guess your implementation may not allow this (which I confess I haven't read) - but if it is possible then I would argue that it is readable and less error prone.

I apologize if this has been asked before but I couldn't find anything in the issue search.

Cheers, Andy

joel-costigliola commented 11 years ago

Hi Andy,

The implementation indeed does not allow your suggestion but you have several options to write an equivalent assertion, let me show you them.

First option : use specific boolean assertion

assertThat(list.isEmpty()).isTrue();

This is lighter than using isEqualTo and more "boolean" oriented, I would pick this one over your option

But there is another option which is better :

assertThat(list).isEmpty();

As readable as your suggestion, as a strongly typed assertion (on List type), it has the advantage of giving a relevant error message because it knows you are checking some collection size (which could not be done with your suggestion).

My advice would be to always use assertThat(objectToCheck). and then use IDE auto completion to see what assertions are available and use the more type specific ones.

Hope I have convinced you ! :) (in that case, please close this issue) Cheers,

Joel

andrewrlee commented 11 years ago

The isEmpty on list was just an example - it's quite often useful to check boolean flags on custom objects.

I had missed the: assertThat(list.isEmpty()).isTrue();

and that is nice and I will use that in the future rather than isEqualTo.

I still think that it is easy for new users to mistakenly just assume that you can do an assertThat(boolean) without any chaining and that it will be checked but fair enough, I'll close the issue.

Thanks, Andy