willowtreeapps / assertk

assertions for kotlin inspired by assertj
MIT License
760 stars 85 forks source link

Consider eliminating `assertThat(body: () -> T)` entrypoint #464

Closed JakeWharton closed 1 year ago

JakeWharton commented 1 year ago

With #461 on track to land (🤞), I don't think the lambda-accepting assertThat holds its weight anymore.

Expressions whose evaluation is expected to succeed can flow directly into assertThat to get wrapped into an Assert<T>. Either:

assertThat(foo()).…

or

val foo = foo()
assertThat(foo).…

Expressions whose evaluation is expected to fail can be enclosed within assertFailure to get wrapped into an Assert<Throwable>.

assertFailure { foo() }.…

Both of these handle the unexpected case.

assertThat(foo()) with a throwing foo() simply sees the failure propagate to fail the test directly without ever entering this library.

assertFailure { foo() } with a foo() that completes successfully results in an AssertionError from assertFailure.

What about Result<T>? I think the only reason you would see a Result<T> now is if it was returned directly to you, and the library still has narrowing assertions extensions on Assert<Result<T>> to handle that case.

fun trySomething(): Result<String> { .. }

assertThat(trySomething()).isSuccess().isEqualTo("success!")
// or
assertThat(trySometing()).isFailure().isInstanceOf<EOFException>()

There's no need to use assertThat { .. } anywhere.

However, I have one final argument against it and it's the strongest one:

https://user-images.githubusercontent.com/66577/236091121-81f9fde3-1ec7-48ac-944e-59505c9a05a7.mov

It auto-completes ahead of assertThat(T) and that's basically never what you want!

evant commented 1 year ago

It auto-completes ahead of assertThat(T) and that's basically never what you want!

ok yeah I've been annoyed by that. Pretty strong argument imo.