Use a proper stack for soft failures so they compose correctly (#254)

Before pushing a soft failure when there was none would be ignored. This causes a problem when using the failIf feature as that condition will be ignored too. Fixes #253
willowtreeapps · Nov 8, 2019 · 1d73503 · 1d73503
1 parent 9cb6b93
commit 1d73503
Show file tree

Hide file tree

Showing 7 changed files with 54 additions and 53 deletions.
diff --git a/src/commonMain/kotlin/assertk/assert.kt b/src/commonMain/kotlin/assertk/assert.kt
@@ -278,7 +278,7 @@ fun <T> Assert<T>.all(body: Assert<T>.() -> Unit) {
 internal fun <T> Assert<T>.all(
     message: String,
     body: Assert<T>.() -> Unit,
-    failIf: (List<AssertionError>) -> Boolean
+    failIf: (List<Throwable>) -> Boolean
 ) {
     SoftFailure(message, failIf).run {
         body()

diff --git a/src/commonMain/kotlin/assertk/failure.kt b/src/commonMain/kotlin/assertk/failure.kt
@@ -3,27 +3,24 @@ package assertk
 import assertk.Failure.Companion.soft
 import com.willowtreeapps.opentest4k.AssertionFailedError
 import com.willowtreeapps.opentest4k.MultipleFailuresError
+import com.willowtreeapps.opentest4k.failures
 
 /**
  * Assertions are run in a failure context which captures failures to report them.
  */
 internal object FailureContext {
-    private val failureRef = ThreadLocalRef<Failure> { SimpleFailure }
+    private val failureRef = ThreadLocalRef<MutableList<Failure>> { mutableListOf(SimpleFailure) }
 
-    fun pushFailure(failure: Failure): Failure {
-        val previousFailure = failureRef.value
-        if (previousFailure == SimpleFailure) {
-            failureRef.value = failure
-        }
-        return previousFailure
+    fun pushFailure(failure: Failure) {
+        failureRef.value.add(failure)
     }
 
-    fun popFailure(previousFailure: Failure) {
-        failureRef.value = previousFailure
+    fun popFailure() {
+        failureRef.value.apply { if (size > 1) { removeAt(size - 1) } }
     }
 
-    fun fail(error: AssertionError) {
-        failureRef.value.fail(error)
+    fun fail(error: Throwable) {
+        failureRef.value.last().fail(error)
     }
 }
 
@@ -37,7 +34,7 @@ internal interface Failure {
     /**
      * Record a failure. Depending on the implementation this may throw an exception or collect the failure for later.
      */
-    fun fail(error: AssertionError)
+    fun fail(error: Throwable)
 
     /**
      * Triggers any collected failures.
@@ -48,21 +45,17 @@ internal interface Failure {
     /**
      * Pushes this failure making it the current context for use with [fail]. You should prefer using [run] instead as
      * it will properly pop the failure for you.
-     *
-     * @return The previous failure. This should be passed to [pop].
      */
-    fun pushFailure(): Failure {
-        return FailureContext.pushFailure(this)
+    fun pushFailure() {
+        FailureContext.pushFailure(this)
     }
 
     /**
      * Pops this failure making the current context throw immediately again. You should prefer using [run] instead as
      * it will properly call this for you.
-     *
-     * @param previousFailure The previous failure, returned from [push]
      */
-    fun popFailure(previousFailure: Failure) {
-        FailureContext.popFailure(previousFailure)
+    fun popFailure() {
+        FailureContext.popFailure()
     }
 
     companion object {
@@ -74,25 +67,23 @@ internal interface Failure {
 }
 
 /**
- * Run the given block of assertions with its Failure. If we are already in a Failure a new one will not be
- * created.
+ * Run the given block of assertions with its Failure.
  */
 @PublishedApi
 internal inline fun <T> Failure.run(f: () -> T): T {
-    val previousFailure = pushFailure()
+    pushFailure()
     try {
         return f()
     } finally {
-        popFailure(previousFailure)
+        popFailure()
         invoke()
-    }
-}
+    } }
 
 /**
  * Failure that immediately thrown an exception.
  */
 internal object SimpleFailure : Failure {
-    override fun fail(error: AssertionError) {
+    override fun fail(error: Throwable) {
         failWithNotInStacktrace(error)
     }
 }
@@ -102,13 +93,18 @@ internal object SimpleFailure : Failure {
  */
 internal class SoftFailure(
     val message: String = defaultMessage,
-    val failIf: (List<AssertionError>) -> Boolean = { it.isNotEmpty() }
+    val failIf: (List<Throwable>) -> Boolean = { it.isNotEmpty() }
 ) :
     Failure {
-    private val failures: MutableList<AssertionError> = ArrayList()
+    private val failures: MutableList<Throwable> = ArrayList()
 
-    override fun fail(error: AssertionError) {
-        failures.add(error)
+    override fun fail(error: Throwable) {
+        // flatten multiple failures into this one.
+        if (error is MultipleFailuresError) {
+            failures.addAll(error.failures)
+        } else {
+            failures.add(error)
+        }
     }
 
     override fun invoke() {
@@ -117,7 +113,7 @@ internal class SoftFailure(
         }
     }
 
-    private fun compositeErrorMessage(errors: List<AssertionError>): AssertionError {
+    private fun compositeErrorMessage(errors: List<Throwable>): Throwable {
         return if (errors.size == 1) {
             errors.first()
         } else {
@@ -158,7 +154,7 @@ fun notifyFailure(e: Throwable) {
     FailureContext.fail(if (e is AssertionError) e else AssertionError(e))
 }
 
-internal expect inline fun failWithNotInStacktrace(error: AssertionError): Nothing
+internal expect inline fun failWithNotInStacktrace(error: Throwable): Nothing
 
 /*
  * Copyright (C) 2018 Touchlab, Inc.

diff --git a/src/commonMain/kotlin/assertk/table.kt b/src/commonMain/kotlin/assertk/table.kt
@@ -3,9 +3,9 @@ package assertk
 import assertk.assertions.support.show
 
 private class TableFailure(private val table: Table) : Failure {
-    private val failures: MutableMap<Int, MutableList<AssertionError>> = LinkedHashMap()
+    private val failures: MutableMap<Int, MutableList<Throwable>> = LinkedHashMap()
 
-    override fun fail(error: AssertionError) {
+    override fun fail(error: Throwable) {
         failures.getOrPut(table.index, { ArrayList() }).plusAssign(error)
     }
 
@@ -15,14 +15,14 @@ private class TableFailure(private val table: Table) : Failure {
         }
     }
 
-    private fun compositeErrorMessage(errors: Map<Int, List<AssertionError>>): AssertionError {
+    private fun compositeErrorMessage(errors: Map<Int, List<Throwable>>): AssertionError {
         return TableFailuresError(table, errors)
     }
 }
 
 internal class TableFailuresError(
     private val table: Table,
-    private val errors: Map<Int, List<AssertionError>>
+    private val errors: Map<Int, List<Throwable>>
 ) : AssertionError() {
     override val message: String?
         get() {

diff --git a/src/commonTest/kotlin/test/assertk/assertions/IterableTest.kt b/src/commonTest/kotlin/test/assertk/assertions/IterableTest.kt
@@ -1,5 +1,6 @@
 package test.assertk.assertions
 
+import assertk.all
 import assertk.assertThat
 import assertk.assertions.*
 import kotlin.test.Test
@@ -57,27 +58,27 @@ class IterableTest {
 
     //region none
     @Test fun none_empty_list_passes() {
-        assertThat(emptyList<Int>() as Iterable<Int>).none { it -> it.isEqualTo(1) }
+        assertThat(emptyList<Int>() as Iterable<Int>).none { it.isEqualTo(1) }
     }
 
     @Test fun none_matching_content_fails() {
         val error = assertFails {
-            assertThat(listOf(1, 2) as Iterable<Int>).none { it -> it.isGreaterThan(0) }
+            assertThat(listOf(1, 2) as Iterable<Int>).none { it.isGreaterThan(0) }
         }
         assertEquals(
             "expected none to pass", error.message
         )
     }
 
     @Test fun each_non_matching_content_passes() {
-        assertThat(listOf(1, 2, 3) as Iterable<Int>).none { it -> it.isLessThan(2) }
+        assertThat(listOf(1, 2, 3) as Iterable<Int>).none { it.isLessThan(2) }
     }
     //endregion
 
     //region atLeast
     @Test fun atLeast_too_many_failures_fails() {
         val error = assertFails {
-            assertThat(listOf(1, 2, 3)).atLeast(2) { it -> it.isGreaterThan(2) }
+            assertThat(listOf(1, 2, 3)).atLeast(2) { it.isGreaterThan(2) }
         }
         assertEquals(
             """expected to pass at least 2 times (2 failures)
@@ -88,39 +89,43 @@ class IterableTest {
     }
 
     @Test fun atLeast_no_failures_passes() {
-        assertThat(listOf(1, 2, 3) as Iterable<Int>).atLeast(2) { it -> it.isGreaterThan(0) }
+        assertThat(listOf(1, 2, 3) as Iterable<Int>).atLeast(2) { it.isGreaterThan(0) }
     }
 
     @Test fun atLeast_less_than_times_failures_passes() {
-        assertThat(listOf(1, 2, 3) as Iterable<Int>).atLeast(2) { it -> it.isGreaterThan(1) }
+        assertThat(listOf(1, 2, 3) as Iterable<Int>).atLeast(2) { it.isGreaterThan(1) }
+    }
+
+    @Test fun atLeast_works_in_a_soft_assert_context() {
+        assertThat(listOf(1, 2,3) as Iterable<Int>).all { atLeast(2) { it.isGreaterThan(1) } }
     }
     //endregion
 
     //region atMost
     @Test fun atMost_more_than_times_passed_fails() {
         val error = assertFails {
-            assertThat(listOf(1, 2, 3) as Iterable<Int>).atMost(2) { it -> it.isGreaterThan(0) }
+            assertThat(listOf(1, 2, 3) as Iterable<Int>).atMost(2) { it.isGreaterThan(0) }
         }
         assertEquals(
             """expected to pass at most 2 times""".trimMargin(), error.message
         )
     }
 
     @Test fun atMost_exactly_times_passed_passes() {
-        assertThat(listOf(1, 2, 3) as Iterable<Int>).atMost(2) { it -> it.isGreaterThan(1) }
+        assertThat(listOf(1, 2, 3) as Iterable<Int>).atMost(2) { it.isGreaterThan(1) }
     }
 
 
     @Test fun atMost_less_than_times_passed_passes() {
-        assertThat(listOf(1, 2) as Iterable<Int>).atMost(2) { it -> it.isGreaterThan(1) }
+        assertThat(listOf(1, 2) as Iterable<Int>).atMost(2) { it.isGreaterThan(1) }
     }
     //endregion
 
 
     //region exactly
     @Test fun exactly_too_few_passes_fails() {
         val error = assertFails {
-            assertThat(listOf(1, 2, 3)).exactly(2) { it -> it.isGreaterThan(2) }
+            assertThat(listOf(1, 2, 3)).exactly(2) { it.isGreaterThan(2) }
         }
         assertEquals(
             """expected to pass exactly 2 times (2 failures)
@@ -132,15 +137,15 @@ class IterableTest {
 
     @Test fun exactly_too_many_passes_fails() {
         val error = assertFails {
-            assertThat(listOf(5, 4, 3)).exactly(2) { it -> it.isGreaterThan(2) }
+            assertThat(listOf(5, 4, 3)).exactly(2) { it.isGreaterThan(2) }
         }
         assertEquals(
             """expected to pass exactly 2 times""".trimMargin(), error.message
         )
     }
 
     @Test fun exactly_times_passed_passes() {
-        assertThat(listOf(1, 2) as Iterable<Int>).atMost(2) { it -> it.isGreaterThan(0) }
+        assertThat(listOf(1, 2) as Iterable<Int>).atMost(2) { it.isGreaterThan(0) }
     }
 
     //endregion

diff --git a/src/jsMain/kotlin/assertk/failure.kt b/src/jsMain/kotlin/assertk/failure.kt
@@ -1,7 +1,7 @@
 package assertk
 
 @Suppress("NOTHING_TO_INLINE")
-internal actual inline fun failWithNotInStacktrace(error: AssertionError): Nothing {
+internal actual inline fun failWithNotInStacktrace(error: Throwable): Nothing {
     throw error
 }
 

diff --git a/src/jvmMain/kotlin/assertk/failure.kt b/src/jvmMain/kotlin/assertk/failure.kt
@@ -3,7 +3,7 @@
 package assertk
 
 @Suppress("NOTHING_TO_INLINE")
-internal actual inline fun failWithNotInStacktrace(error: AssertionError): Nothing {
+internal actual inline fun failWithNotInStacktrace(error: Throwable): Nothing {
     val filtered = error.stackTrace
         .dropWhile { it.className.startsWith("assertk") }
         .toTypedArray()

diff --git a/src/nativeMain/kotlin/assertk/failure.kt b/src/nativeMain/kotlin/assertk/failure.kt
@@ -4,7 +4,7 @@ import kotlin.native.concurrent.ThreadLocal
 import kotlin.native.concurrent.AtomicInt
 
 @Suppress("NOTHING_TO_INLINE")
-internal actual inline fun failWithNotInStacktrace(error: AssertionError): Nothing {
+internal actual inline fun failWithNotInStacktrace(error: Throwable): Nothing {
     throw error
 }