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

Adding new assertFailureWith #525

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

robson-alecio
Copy link

Adding the new assertFailureWith to assert exception with a specific type and verify properties of this type.

@Goooler
Copy link
Contributor

Goooler commented Mar 27, 2024

This addresses #511.

Copy link
Contributor

@JakeWharton JakeWharton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer this variant of assertFailure, but it was asked to be omitted when that function was first introduced. You'll have to wait to hear from a maintainer on #511

Comment on lines +200 to +202
assertFailure.isInstanceOf(T::class)
@Suppress("UNCHECKED_CAST")
return assertFailure as Assert<T>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns a specialized Assert with the new type

Suggested change
assertFailure.isInstanceOf(T::class)
@Suppress("UNCHECKED_CAST")
return assertFailure as Assert<T>
return assertFailure.isInstanceOf(T::class)

/**
* Asserts the throwable with a specific type have the expected properties for it.
*/
fun <T: Throwable> Assert<T>.hasProperties(vararg pairs: Pair<KProperty1<T, Any>, Any>) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unrelated to assertFailsWith. It's not clear why it's type parameter is bound to Throwable subtypes since it's a general-purpose utility. I would separate it into its own PR, or even start with an issue discussing the use case and why regular .all { } in insufficient.

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 this pull request may close these issues.

None yet

3 participants