Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assertThat().all is misleading #544

Open
asuras-coding opened this issue Jul 11, 2024 · 2 comments
Open

assertThat().all is misleading #544

asuras-coding opened this issue Jul 11, 2024 · 2 comments

Comments

@asuras-coding
Copy link

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
Copy link
Collaborator

evant commented Jul 11, 2024

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

@JakeWharton
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants