willowtreeapps / assertk

assertions for kotlin inspired by assertj
MIT License
757 stars 84 forks source link

assertThat().all is misleading #544

Open asuras-coding opened 1 month ago

asuras-coding commented 1 month ago

Hey there,

I like to use assertk as an assertj drop in for kotlin code. Recently I stumbled across the assertThat().all and assertThat().each methods.

I did something like

val listOfLists = listOf(listOf(1), listOf(2), listOf(3), listOf(4))
assertThat(listOfLists).all { hasSize(1) }

This assertion fails because it does not assert that each list has size one, but it asserts that the outer list has size 1. assertThat(listOfLists).each { it.hasSize(1) } is the way to go here.

Why did I try to use all in the first place? Well there is standard-lib function all which works on lists and checks if all elements of the list match the given predicate: assertTrue(listOfLists.all { it.size == 1 })

So I think naming of all and each assertions could be improved to save other fellow developers from the trap I fell into.

evant commented 1 month ago

It's to mirror assertAll for soft assertions, naming is hard 😂 open to suggestions

JakeWharton commented 1 month ago

Definitely been struggling to come up with something good here.

Best I can think is that we need a verb suffix to all/any/none (I'm assuming we can change the design to support any/none here) such as "match" (allMatch { }, anyMatch { }, noneMatch { }). But we don't use that verb anywhere else (to my knowledge), and there are really no other verbs that fit which are currently in use.

In a playground project where I've been experimenting with a new design for this library that supports things such as "any" and "none" I have also been toying with the idea of making Assert's Companion be an implementation of the "unit" Assert (similar to the design of Compose UI's Modifier). One thing this enables is that the entire library becomes instance-based rather than split between top-level functions and instance functions, at the downside of requiring that someone does the more verbose Assert.allMatch { } for the equivalent of today's assertAll. Although the latter could simply call into the former, but perhaps named assertAllMatch?