Any failures override any pending statuses in assertion groups

Fixes #243
robfletcher · Apr 11, 2021 · 47842d8 · 47842d8
1 parent df2e5d8
commit 47842d8
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.30.1 Ascending Ivory = 2021-04-11
+
+- Fixes an issue where pending assertion chains in a block took precedence over failures, causing false positives. See [#243](https://github.com/robfletcher/strikt/issues/243)
+
 ## 0.30.0 Sorrowful Glass - 2021-03-21
 
 - All transitive dependencies are now resolvable from Maven Central rather than needing JCenter.

diff --git a/strikt-core/src/main/kotlin/strikt/internal/AssertionNode.kt b/strikt-core/src/main/kotlin/strikt/internal/AssertionNode.kt
@@ -63,8 +63,8 @@ internal class AssertionSubject<S>(
   override val status: Status
     get() = when {
       children.isEmpty() -> Pending
-      children.any { it.status is Pending } -> Pending
       children.any { it.status is Failed } -> Failed()
+      children.any { it.status is Pending } -> Pending
       else -> Passed()
     }
 }
@@ -95,8 +95,8 @@ internal class AssertionChain<S>(
   override val status: Status
     get() = when {
       children.isEmpty() -> Pending
-      children.any { it.status is Pending } -> Pending
       children.any { it.status is Failed } -> Failed()
+      children.any { it.status is Pending } -> Pending
       else -> Passed()
     }
 
@@ -130,8 +130,8 @@ internal class AssertionChainedGroup<S>(
   override val status: Status
     get() = when {
       children.isEmpty() -> Pending
-      children.any { it.status is Pending } -> Pending
       children.any { it.status is Failed } -> Failed()
+      children.any { it.status is Pending } -> Pending
       else -> Passed()
     }
 }

diff --git a/strikt-core/src/test/kotlin/strikt/Block.kt b/strikt-core/src/test/kotlin/strikt/Block.kt
@@ -6,8 +6,10 @@ import org.junit.jupiter.api.assertThrows
 import strikt.api.expectThat
 import strikt.assertions.contains
 import strikt.assertions.endsWith
+import strikt.assertions.filter
 import strikt.assertions.getValue
 import strikt.assertions.hasLength
+import strikt.assertions.hasSize
 import strikt.assertions.isA
 import strikt.assertions.isEqualTo
 import strikt.assertions.isGreaterThan
@@ -21,6 +23,7 @@ import java.time.LocalDate
 @DisplayName("assertions in blocks")
 internal class Block {
   @Test
+  @Suppress("RedundantNullableReturnType")
   fun `all assertions in a block are evaluated even if some fail`() {
     assertThrows<AssertionError> {
       val subject: Any? = "fnord"
@@ -45,6 +48,7 @@ internal class Block {
   }
 
   @Test
+  @Suppress("RedundantNullableReturnType")
   fun `chains inside of blocks break on the first failure`() {
     assertThrows<AssertionError> {
       val subject: Any? = "fnord"
@@ -84,6 +88,7 @@ internal class Block {
   }
 
   @Test
+  @Suppress("RedundantNullableReturnType")
   fun `get chained after a failing assertion is not evaluated`() {
     assertThrows<AssertionError> {
       val subject: Any? = "fnord"
@@ -102,6 +107,7 @@ internal class Block {
   }
 
   @Test
+  @Suppress("RedundantNullableReturnType")
   fun `assertions in a block can be negated`() {
     assertThrows<AssertionError> {
       val subject: Any? = "fnord"
@@ -126,6 +132,7 @@ internal class Block {
   }
 
   @Test
+  @Suppress("RedundantNullableReturnType")
   fun `assertions in a block can be negated in a not block`() {
     assertThrows<AssertionError> {
       val subject: Any? = "fnord"
@@ -150,6 +157,7 @@ internal class Block {
   }
 
   @Test
+  @Suppress("RedundantNullableReturnType")
   fun `an and block can be negated`() {
     val subject: Any? = "fnord"
     expectThat(subject).not().and {
@@ -292,6 +300,7 @@ internal class Block {
    * @see https://github.com/robfletcher/strikt/issues/203
    */
   @Test
+  @Suppress("RedundantNullableReturnType")
   fun `failing mappings inside an and block throw correct exception type`() {
     assertThrows<AssertionError> {
       class Person(val firstName: String, val hobbies: List<String>)
@@ -325,6 +334,7 @@ internal class Block {
    * @see https://github.com/robfletcher/strikt/issues/204
    */
   @Test
+  @Suppress("RedundantNullableReturnType")
   fun `nested and is not evaluated if preceding assertion failed`() {
     assertThrows<AssertionError> {
       class Person(val name: String, val friend: Person?)
@@ -339,4 +349,20 @@ internal class Block {
       }
     }
   }
+
+  /**
+   * @see https://github.com/robfletcher/strikt/issues/243
+   */
+  @Test
+  fun `unterminated mapping does not mask failing assertions`() {
+    assertThrows<AssertionError> {
+      val subject = listOf("catflap", "rubberplant", "marzipan")
+      expectThat(subject) {
+        // this assertion should cause the block to fail
+        hasSize(2)
+        // this mapping is valid but is not followed by an assertion
+        filter { it == "catflap" }.single()
+      }
+    }
+  }
 }