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.

Sign up for GitHub

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

Default Result Assertions Fix (again) #1429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c28e1c6
if statement
JordanllHarper Jan 28, 2023
9a8d1a7
Resolve conflicts
JordanllHarper May 6, 2023
002cfb4
Fixes + tests passing
JordanllHarper May 6, 2023
e1c6d4d
Spacing
JordanllHarper May 6, 2023
602e32e
Comment samples
JordanllHarper May 6, 2023
4b6cec8
Uncomment spec
JordanllHarper May 6, 2023
a4b4d48
Remove todo
JordanllHarper May 7, 2023
f1d1b31
Remove comment out tests
JordanllHarper May 8, 2023
c8079a6
Amend test deletion
JordanllHarper May 8, 2023
c0f0734
Remove todos
JordanllHarper May 8, 2023
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
127 changes: 64 additions & 63 deletions .../kotlin/ch/tutteli/atrium/api/fluent/en_GB/kotlin_1_3/samples/ResultExpectationSamples.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,69 +57,70 @@ class ResultExpectationSamples {
}
}

//TODO 1.1.0 activate once we have the workaround for #1234 implemented
// @Test
// fun toBeAFailureFeature() {
// val message = "wrong argument"
// val failure = Result.failure<Int>(IllegalArgumentException(message))
//
// expect(failure)
// .toBeAFailure<IllegalArgumentException>() // subject is now of type IllegalArgumentException
// .messageToContain("argument")
//
//
// fails { // because sub-expectation fails
// expect(failure)
// .toBeAFailure<IllegalArgumentException>()
// .messageToContain("parameter") // fails
// }
//
// fails { // because wrong Expectation type expected
// expect(failure)
// .toBeAFailure<ArithmeticException>() // fails
// .messageToContain("parameter") // not evaluated/reported because toBeAFailure already fails
// // use `toBeAFailure<...> { ... }` if you want that all expectations are evaluated
// }
//
// fails { // because it was a Success
// expect(Result.success(10))
// .toBeAFailure<IllegalArgumentException>() // fails
// .messageToContain("parameter") // not evaluated/reported because toBeAFailure already fails
// // use `toBeAFailure<...> { ... }` if you want that all expectations are evaluated
// }
// }
//
// @Test
// fun toBeAFailure() {
// val errorMessage = "can not divide by zero"
// val failure = Result.failure<Int>(ArithmeticException(errorMessage))
//
// expect(failure).toBeAFailure<ArithmeticException> { // subject within this expectation-group is of type ArithmeticException
// messageToContain("by zero")
// } // subject here is back to type Result<Int>
//
// fails { // because sub-expectation fails
// expect(failure).toBeAFailure<IllegalArgumentException> {
// messageToContain("parameter") // fails
// message.toStartWith("wrong") // still evaluated even though messageToContain already fails
// // use `.toBeAFailure.` if you want a fail fast behaviour
// }
// }
//
// fails { // because wrong Expectation type expected, but since we use an expectation-group...
// expect(failure).toBeAFailure<ArithmeticException> {
// messageToContain("parameter") // ...reporting mentions that subject's message was expected `to contain: "parameter"``
// // use `.toBeAFailure.` if you want a fail fast behaviour
// }
// }
//
// fails { // because it was a Success, but since we use a block
// expect(Result.success(10)).toBeAFailure<IllegalArgumentException> {
// messageToContain("parameter") // ...reporting mentions that subject's message was expected `to contain: "parameter"``
// // use `.toBeAFailure.` if you want a fail fast behaviour
// }
// }
// }


@Test
fun toBeAFailureFeature() {
val message = "wrong argument"
val failure = Result.failure<Int>(IllegalArgumentException(message))

expect(failure)
.toBeAFailure<IllegalArgumentException>() // subject is now of type IllegalArgumentException
.messageToContain("argument")


fails { // because sub-expectation fails
expect(failure)
.toBeAFailure<IllegalArgumentException>()
.messageToContain("parameter") // fails
}

fails { // because wrong Expectation type expected
expect(failure)
.toBeAFailure<ArithmeticException>() // fails
.messageToContain("parameter") // not evaluated/reported because toBeAFailure already fails
// use `toBeAFailure<...> { ... }` if you want that all expectations are evaluated
}

fails { // because it was a Success
expect(Result.success(10))
.toBeAFailure<IllegalArgumentException>() // fails
.messageToContain("parameter") // not evaluated/reported because toBeAFailure already fails
// use `toBeAFailure<...> { ... }` if you want that all expectations are evaluated
}
}

@Test
fun toBeAFailure() {
val errorMessage = "can not divide by zero"
val failure = Result.failure<Int>(ArithmeticException(errorMessage))

expect(failure).toBeAFailure<ArithmeticException> { // subject within this expectation-group is of type ArithmeticException
messageToContain("by zero")
} // subject here is back to type Result<Int>

fails { // because sub-expectation fails
expect(failure).toBeAFailure<IllegalArgumentException> {
messageToContain("parameter") // fails
message.toStartWith("wrong") // still evaluated even though messageToContain already fails
// use `.toBeAFailure.` if you want a fail fast behaviour
}
}

fails { // because wrong Expectation type expected, but since we use an expectation-group...
expect(failure).toBeAFailure<ArithmeticException> {
messageToContain("parameter") // ...reporting mentions that subject's message was expected `to contain: "parameter"``
// use `.toBeAFailure.` if you want a fail fast behaviour
}
}

fails { // because it was a Success, but since we use a block
expect(Result.success(10)).toBeAFailure<IllegalArgumentException> {
messageToContain("parameter") // ...reporting mentions that subject's message was expected `to contain: "parameter"``
// use `.toBeAFailure.` if you want a fail fast behaviour
}
}
}


}
41 changes: 39 additions & 2 deletions .../src/commonMain/kotlin/ch/tutteli/atrium/logic/kotlin_1_3/impl/DefaultResultAssertions.kt
View file
Open in desktop
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
50 changes: 24 additions & 26 deletions ...specs/src/commonMain/kotlin/ch/tutteli/atrium/specs/integration/ResultExpectationsSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,15 @@ 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) {} } }
// )
// ) {})
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 +119,21 @@ 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...\""
// )
// }
// }
// }
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