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

Add skip functionality #87

Closed
JohannesPtaszyk opened this issue Mar 2, 2022 · 6 comments · Fixed by #95
Closed

Add skip functionality #87

JohannesPtaszyk opened this issue Mar 2, 2022 · 6 comments · Fixed by #95

Comments

@JohannesPtaszyk
Copy link
Contributor

JohannesPtaszyk commented Mar 2, 2022

Sometimes it might be good to have a way to skip a certain number of events to have a cleaner test without multiple await calls, which are then not part of any assertion.
For my codebase I added an extension function like this:

suspend fun <T> FlowTurbine<T>.skipItem(amount: Int = 1) = repeat(amount) {
    awaitItem()
}

Does it make sense to add that functionality on library level?

@JakeWharton
Copy link
Member

Maybe? I'm trying to figure out if it is a useful API rather than just calling repeat(5) { awaitItem() }.

@JohannesPtaszyk
Copy link
Contributor Author

JohannesPtaszyk commented Mar 4, 2022

I was also unsure if its worth or not.
On the other hand: It feels and looks nicer within the test.

Happy to discuss!

Edit: Another option would be to go with something like:

public suspend fun awaitItems(count: Int): List<T>

But still unsure if its a nice thing to have or not.
For us the usecase was about skipping the first X events.

@JohannesPtaszyk
Copy link
Contributor Author

I kept thinking about this for a while and it would be a good addition to explicitly add a skip or ignore function.
It will highlight that we don't care about the X items, since they are not subject of what the test is trying to assert.
Readability will be better and the intention of the author will also be easier to get.
Also I think the benefit of ease of use outweighs the cost of the extra function.

@JakeWharton
Copy link
Member

I'm okay with it if @mattprecious is. Want to send a PR?

@mattprecious
Copy link

I think to make it a worthwhile addition over repeat(5) { awaitItem() }, there should be a helpful error message when it fails. Ex: Expected 5 items, but got 4, or Expected 5 items, but got 4 and Error. That's the kind of logic that I wouldn't want to repeat in all my tests.

@JohannesPtaszyk
Copy link
Contributor Author

Will open a PR on the weekend, so it will be easier to have a base for further discussions.

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

Successfully merging a pull request may close this issue.

3 participants