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

Allow override of assertion context's error message display. #313

Merged
merged 1 commit into from Aug 21, 2020

Conversation

Kritarie
Copy link
Contributor

By default, contexts whose type is not known will fall back to its .toString(), which may not be desirable for some cases.

Addresses #232

override fun <R> assertThat(actual: R, name: String?): Assert<R> =
ValueAssert(actual, name, if (context != null || this.value === actual) context else this.value)
override fun <R> assertThat(actual: R, name: String?): Assert<R> {
val newContext = if (context.originatingSubject != null || this.value == actual) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passing only a flag was not enough, since the original implementation ran this comparison to determine whether an Assert is considered "transformed". For this reason, I introduced an internal type AssertingContext that simply holds the original value along with its display function.

value = actual,
name = name,
context = AssertingContext(
originatingSubject = null,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I admit this looks a little confusing, but this most closely aligns with the original implementation - the originatingSubject is null until the Assert is transformed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would making null the default value of the param make it more clear?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, think it might make sense to pull all of this into the AssertingContext's constructor.

Copy link
Contributor Author

@Kritarie Kritarie Aug 21, 2020

Choose a reason for hiding this comment

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

Do you mean like defer the currying as well?

context = AssertingContext(actual, displayActual)

Actually, would have to erase T before passing. So more like

context = AssertingContext { displayActual(actual) }

By default, contexts whose type is not known will fall back to its .toString(), which may not be desirable for some cases.
@evant evant merged commit 18a6bed into main Aug 21, 2020
@evant evant deleted the override-context-display branch August 21, 2020 20:13
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

2 participants