Are your assertions fluent enough?

[scordio]

Are you happy with the fluency of your assertions? Do you have a use case you wish would be less verbose?

Put your use case here 👇 and let's see what we can do!

[scordio]

https://stackoverflow.com/a/69211323/9714611

assertThatExceptionOfType(DataIntegrityViolationException.class).isThrownBy( () -> xrepo.save(abc) )
    .havingCause()
    .asInstanceOf(type(ConstraintViolationException.class))
    .returns("XXXX", from(ConstraintViolationException::getConstraintName));

Type Narrowing

havingCause() + asInstanceOf(InstanceOfAssertFactory) could be shrunk to a single havingCause(InstanceOfAssertFactory).

Readability

Readability could be improved. Considering the proposal above, it would read as:

assertThatExceptionOfType <type> isThrownBy <code> havingCause returns <value> from <getter>

The havingCause returns <value> from <getter> part doesn't sound proper.

I see two options:

• havingCauseThat returns <value> from <getter> --> requires a rename of havingCause (i.e., deprecation + new method in ThrowableAssertAlternative)
• havingCause returning <value> from <getter> --> requires a potential breaking change as returns comes from AbstractObjectAssert, current parent type of ThrowableAssertAlternative

[armandino]

From #2616 (comment):

Currently usingRecursiveComparison() is a little verbose when comparing enums with their string representations:

assertThat(entity)
        .usingRecursiveComparison()
        .withEqualsForFields((String s, PhoneType t) -> PhoneType.valueOf(s) == t, "phones.phoneType")
        .withEqualsForFields((String s, AddressType t) -> AddressType.valueOf(s) == t, "addresses.addressType")
        // etc..
        .isEqualTo(dto);

It would be nice if this was supported OOTB via an option, e.g.:

assertThat(entity)
        .usingRecursiveComparison()
        .withEnumStringComparison()
        .isEqualTo(dto);

[joel-costigliola]

is there an issue already opened for this?

[scordio]

I reopened #2616 to address this.

[mramsden]

Is there an option to supporting working with Kotlin Coroutines in a slightly cleaner way? My thought would be that this could be done as an extension module for assertj. Perhaps adding assertj-kotlin as an extension module. Is this something that has been considered and already discounted (this is assertj, not assertk after all 😄)

This would primarily improve the usage of code block based assertions.

At the moment working with this library I can fairly cleanly write a test that is using an assertThat chain:

fun testMeaningOfLife() {
    val result = runBlocking {
        meaningOfLife()
    }
    assertThat(result).isEqualTo(42)
}

The runBlocking function is required to allow the test to block on the coroutine function to completion. There are some niceties that we can take advantage of to reduce the level of nesting within the function, for example:

fun testMeaningOfLife(): Unit = runBlocking {
    val result = meaningOfLife()
    assertThat(result).isEqualTo(42)
}

Where this could be tweaked is with the code based assertions (assertThatCode, assertThatThrownBy, isThrownBy are there other cases that I am missing here?):

import org.assertj.kotlin.api.Assertions.isThrownBy

fun testMeaningOfLife() {
    assertThatNoException().isThrownBy {
        // No runBlocking call required here as the callable on isThrownBy is marked as a `suspend`-ing method
        meaningOfLife()
    }
}

The change here is that the isThrownBy would be tweaked with an extension function that takes a different interface to ThrowingCallable:

fun NotThrownAssert.isThrownBy(callable: suspend () -> Unit) {
    val throwable = catchSuspendedThrowable(callable)
    assertThat(throwable).`as`(description).doesNotThrowAnyException()
}

fun catchSuspendedThrowable(callable: suspend () -> Unit): Throwable? {
    try {
        runBlocking {
            callable.call()
        }
    } catch (t: Throwable) {
        return t
    }
}

The challenge here is that things like description from the NotThrownAssert is a protected type. Extension functions are a bit of syntactic sugar from Kotlin and are just a means to allow calling an object that is not otherwise open to extension.

[scordio]

Hi @mramsden, some compatibility issues with Kotlin have already been reported, not sure if yours overlaps with one of these.

Whenever possible, we try to provide a better experience for Kotlin users, however, some adjustments might require breaking changes which will be delivered with AssertJ 4 only. I'll have a deeper look at your issue and see if it's something that can still be part of 3.x.

Thank you so much for providing concrete examples! This helps us a lot in shaping those changes.

[mramsden]

Thanks @scordio that's great to hear that this is on the cards! Looking through the issues that are already mentioned I think this is a bit of a different use case as this is explicitly to do with coroutines which may add a few rough edges to the approach.

Looking at the issues you have listed there #2896 is a pretty close match for this. Although the cases described do impact a different part of the API. Would it be any help if I provided a small example module with some proposals of the kind of helpers that might be involved?

I've had a bit of a play with this and I do have some helper functions from an existing project that I would be able to share.

[scordio]

Sure, that would help!

[ascopes]

I kind of find some assertions can be a bit jarring due to how they are worded. One of the main ones I see is assertThatCode. I've tried to suggest to my team to prefer assertThatNoException or assertThatExceptionOfType is preferred for a couple of reasons...

assertThatCode(this::createPotatoExceptionFactory)
    .isInstanceOf(PotatoException.class);

In this case, it is hard to tell that this is testing an exception is thrown rather than something being returned.

In addition, the methods that consume lambdas as the first call can make messy code very easy compared to those that have a chained call first. Some auto formatters make these very hard to read. An example of how a formatter might make these awkward when using static imports:

assertThatThrownBy(() -> potatoFactory
    .newInstance().createPotato())
    .isInstanceOf(PotatoException.class);

...where it is hard to see that there is a lambda and where it ends without reading the whole thing first.

Given there is assertThatNoException and assertThatExceptionOfType, and those work much better for this, I do wonder if the former methods could be deprecated going forwards in an attempt to enforce "one good way to do things" and keep the assertions as close to fluent language as possible without compromising code readability.

AssertJ does provide a lot of ways of doing certain things. While that is usually very helpful, it can make enforcing consistent tests in a team somewhat difficult because of stuff like this.

[scordio]

Have you seen #1652 and #2519?

[michaldo]

Map assertion may get as expected list of entries or other map . containsExactly(entries) and containsExactlyEntriesOf(map)
It looks sensible
It does not look sensible that equivalent of containsExactlyInAnyOrderEntriesOf(map) is containsOnly(entries)
Should be containsOnly(entries) and containsOnlyEntriesOf(map)
or containsExactlyInAnyOrder(entries) and containsExactlyInAnyOrderEntriesOf(map)

[joel-costigliola]

Fair enough, we should deprecate containsOnly(entries) for containsExactlyInAnyOrder(entries) which is clearer IMO.