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

Default Result Assertions Fix (again) #1429

Merged
merged 10 commits into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,22 @@ class ResultExpectationSamples {
}
}

//TODO 1.1.0 activate once we have the workaround for #1234 implemented


// @Test
// fun toBeAFailureResultFix() {
// val message = "wrong argument"
// val failure = Result.failure<Int>(IllegalArgumentException(message))
//
//
//
// expect(failure) {
// toBeAFailure<IllegalArgumentException>()
// }
//
//
// }
//
Copy link
Owner

Choose a reason for hiding this comment

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

remove this

// @Test
// fun toBeAFailureFeature() {
// val message = "wrong argument"
Copy link
Owner

Choose a reason for hiding this comment

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

uncomment all the tests instead

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ package ch.tutteli.atrium.logic.kotlin_1_3.impl

import ch.tutteli.atrium.core.ExperimentalNewExpectTypes
import ch.tutteli.atrium.core.Option
import ch.tutteli.atrium.core.getOrElse
import ch.tutteli.atrium.core.polyfills.cast
import ch.tutteli.atrium.creating.AssertionContainer
import ch.tutteli.atrium.creating.ExperimentalComponentFactoryContainer
import ch.tutteli.atrium.creating.FeatureExpect
import ch.tutteli.atrium.creating.FeatureExpectOptions
import ch.tutteli.atrium.logic.changeSubject
import ch.tutteli.atrium.logic.creating.FeatureExpectOptions
import ch.tutteli.atrium.logic.creating.transformers.FeatureExtractorBuilder
import ch.tutteli.atrium.logic.creating.transformers.SubjectChangerBuilder
import ch.tutteli.atrium.logic.creating.transformers.impl.ThrowableThrownFailureHandler
Expand All @@ -33,8 +36,42 @@ class DefaultResultAssertions : ResultAssertions {
override fun <TExpected : Throwable> isFailureOfType(
container: AssertionContainer<out Result<*>>,
expectedType: KClass<TExpected>
): SubjectChangerBuilder.ExecutionStep<Throwable?, TExpected> =
container.manualFeature(EXCEPTION) { exceptionOrNull() }.transform().let { previousExpect ->
): SubjectChangerBuilder.ExecutionStep<Throwable?, TExpected> =
container.manualFeature(EXCEPTION) {


//fix is here for bug in kotlin 1.3, 1.4, 1.5 (fixed in 1.6) => https://youtrack.jetbrains.com/issue/KT-50974
//Comments below explain the fix
val badResult = exceptionOrNull()
//mapping manually to get internal value throwable - exception or null just produced null + we lost the exception
val maybeSubjectResult = container.maybeSubject.map {
onSuccess { null }
onFailure { it.cause }
}
Comment on lines +47 to +50
Copy link
Owner

Choose a reason for hiding this comment

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

sorry, I did not really find the time beforehand to take a proper look. This code does nothign as your onSuccess and onFailure only perform side effects. They don't return something which means, maybeSubjectResult is the same as container.maybeSubject. You don't have to do anything. I will improve the code and let you know my result in the end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see the error I made sorry about that. Would the solution to be to return some sort of value from the success or failure states and check that?

Copy link
Owner

Choose a reason for hiding this comment

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

the statement doesn't do anything and you still have fixed the issue which means these lines we can just delete :)



if (maybeSubjectResult.getOrElse { null } != null && badResult == null) {
//there is something inside first option type so unwrap
val successError = maybeSubjectResult.getOrElse { null }

if (successError == null) {

return@manualFeature exceptionOrNull()
}
//if the result is a success we have another value to check
if (successError.isSuccess) {

val valueInsideSuccess = getOrNull() as Result<*>
if (valueInsideSuccess.isFailure) {
return@manualFeature valueInsideSuccess.exceptionOrNull()
}
}

}
exceptionOrNull()


}.transform().let { previousExpect ->
FeatureExpect(
previousExpect,
FeatureExpectOptions(representationInsteadOfFeature = { it ?: IS_NOT_FAILURE })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,16 @@ abstract class ResultExpectationsSpec(
toBeASuccessNullable.forAssertionCreatorSpec("$toEqualDescr: 2") { toEqual(2) }
) {})

//TODO 1.1.0 activate once we have the workaround for #1234 implemented
// include(object : AssertionCreatorSpec<Result<Int>>(
// "$describePrefix[failure] ", Result.failure(IllegalArgumentException("oh no...")),
// assertionCreatorSpecTriple(
// toBeAFailure.name,
// "${DescriptionCharSequenceExpectation.VALUE.getDefault()}: \"oh no...\"",
// { apply { toBeAFailure.invoke(this) { messageToContain("oh no...") } } },
// { apply { toBeAFailure.invoke(this) {} } }
// )
// ) {})
//TODO 0.20.0 activate once we have the workaround for #1234 implemented
Copy link
Owner

Choose a reason for hiding this comment

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

remove todo

include(object : AssertionCreatorSpec<Result<Int>>(
"$describePrefix[failure] ", Result.failure(IllegalArgumentException("oh no...")),
assertionCreatorSpecTriple(
toBeAFailure.name,
"${DescriptionCharSequenceExpectation.VALUE.getDefault()}: \"oh no...\"",
{ apply { toBeAFailure.invoke(this) { messageToContain("oh no...") } } },
{ apply { toBeAFailure.invoke(this) {} } }
)
) {})

fun describeFun(vararg pairs: SpecPair<*>, body: Suite.() -> Unit) =
describeFunTemplate(describePrefix, pairs.map { it.name }.toTypedArray(), body = body)
Expand Down Expand Up @@ -120,22 +120,22 @@ abstract class ResultExpectationsSpec(
}
}

//TODO 1.1.0 activate once we have the workaround for #1234 implemented
// failureFunctions.forEach { (name, toBeAFailureFun, _) ->
// it("$name - can perform sub-assertion which holds") {
// expect(resultFailure).toBeAFailureFun { messageToContain("oh no...") }
// }
// it("$name - can perform sub-assertion which fails, throws AssertionError") {
// expect {
// expect(resultFailure).toBeAFailureFun { messageToContain("oh yes...") }
// }.toThrow<AssertionError> {
// messageToContain(
// "$exceptionDescr: ${IllegalArgumentException::class.fullName}",
// DescriptionCharSequenceExpectation.TO_CONTAIN.getDefault(), "${DescriptionCharSequenceExpectation.VALUE.getDefault()}: \"oh yes...\""
// )
// }
// }
// }
//TODO 0.20.0 activate once we have the workaround for #1234 implemented
Copy link
Owner

Choose a reason for hiding this comment

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

remove todo

failureFunctions.forEach { (name, toBeAFailureFun, _) ->
it("$name - can perform sub-assertion which holds") {
expect(resultFailure).toBeAFailureFun { messageToContain("oh no...") }
}
it("$name - can perform sub-assertion which fails, throws AssertionError") {
expect {
expect(resultFailure).toBeAFailureFun { messageToContain("oh yes...") }
}.toThrow<AssertionError> {
messageToContain(
"$exceptionDescr: ${IllegalArgumentException::class.fullName}",
DescriptionCharSequenceExpectation.TO_CONTAIN.getDefault(), "${DescriptionCharSequenceExpectation.VALUE.getDefault()}: \"oh yes...\""
)
}
}
}
}
}

Expand Down