Improve assertSoftly compatibility

AUTHORS.md

@@ -41,3 +41,4 @@
 1. Keivan Esbati - [@Tenkei](https://github.com/Tenkei) ([Contributions](https://github.com/MarkusAmshove/Kluent/commits?author=Tenkei))
 1. Sam Neirinck - [@samneirinck](https://github.com/samneirinck) ([Contributions](https://github.com/MarkusAmshove/Kluent/commits?author=samneirinck)) 
 1. Maxim Ivanov [@drcolombo](https://github.com/drcolombo) ([Contributions](https://github.com/MarkusAmshove/Kluent/commits?author=drcolombo))
+1. Piotr Bakalarski [@piotrb5e3](https://github.com/piotrb5e3) ([Contributions](https://github.com/MarkusAmshove/Kluent/commits?author=piotrb5e3))

CONTRIBUTING.md

@@ -100,6 +100,13 @@ and [Native](https://github.com/MarkusAmshove/Kluent/blob/master/native/src/main
 
 If you're still unsure how to make something platform independent, we can have a look together inside the PR :-)
 
+## Compatibility with assertSoftly
+When adding assertions, do not throw AssertionError directly.
+Insead, use existing assertions, internal helpers or the `fail` method.
+This way your assertion will be compatible with `assertSoftly`.
+
+If your assertion can't fail softly, for example `fun T?.shouldNotBeNull(): T = ...`, then call `hardFail`.
+
 ## Coding Style
 
 Our coding style is the default code style coming with IntelliJ.

common/src/main/kotlin/org/amshove/kluent/Basic.kt

@@ -30,7 +30,7 @@ fun <T : Any> T?.shouldNotBeNull(): T {
         returns() implies (this@shouldNotBeNull != null)
     }
 
-    return this ?: fail("Expected non null value, but value was null")
+    return this ?: hardFail("Expected non null value, but value was null")
 }
 
 @UseExperimental(ExperimentalContracts::class)
@@ -75,13 +75,38 @@ fun Char.shouldNotBeDigit() = this.apply { assertTrue("Expected '$this' to be no
 
 infix fun <T> T.should(assertion: T.() -> Boolean) = should("Expected the assertion to return true, but returned false", assertion)
 
-fun <T> T.should(message: String, assertion: T.() -> Boolean): T = try {
-    if (assertion()) this else fail(message)
-} catch (t: Throwable) {
-    fail("""$message
+fun <T> T.should(message: String, assertion: T.() -> Boolean): T = also {
+    try {
+        if (!assertion()) {
+            fail(message)
+        }
+    } catch (t: Throwable) {
+        fail("""$message
         |
         | An exception occured:
         |   ${t.platformClassName()}: ${t.message}
         |   ${"\tat "}${t.platformJoinStackTrace()}
     """.trimMargin())
+    }
 }
+
+/**
+ * Provides an assertSoftly-compatible way of reporting a failed assertion
+ * All assertions should rely on it for error reporting.
+ * Assertions that don't work with assertSoftly (for example shouldNotBeNull) can use hardFail
+ */
+fun fail(message: String?) {
+    try {
+        throw AssertionError(message)
+    } catch (ex: AssertionError) {
+        if (errorCollector.getCollectionMode() == ErrorCollectionMode.Soft) {
+            errorCollector.pushError(ex)
+        } else {
+            throw assertionError(ex)
+        }
+    }
+}
+
+/** Use this function in places where a soft fail in assertSoftly would not make sense - for example shouldNotBeNull. */
+@PublishedApi
+internal fun hardFail(message: String?): Nothing = throw AssertionError(message)

common/src/main/kotlin/org/amshove/kluent/Collections.kt

@@ -722,11 +722,11 @@ infix fun <T, I : Iterable<T>> I.shouldBeSortedAccordingTo(comparator: Comparato
 
 @Deprecated("Equality should not be tested on sequences", level = DeprecationLevel.ERROR)
 infix fun <T, S : Sequence<T>> S.shouldEqual(expected: Sequence<T>): S =
-        fail("Equality should not be tested on sequences")
+        hardFail("Equality should not be tested on sequences")
 
 @Deprecated("Equality should not be tested on sequences", level = DeprecationLevel.ERROR)
 infix fun <T, S : Sequence<T>> S.shouldNotEqual(expected: Sequence<T>): S =
-        fail("Equality should not be tested on sequences")
+        hardFail("Equality should not be tested on sequences")
 
 infix fun <T, S : Sequence<T>> S.shouldBeSortedAccordingTo(comparator: Comparator<T>) = apply {
     var index = 0
@@ -860,12 +860,10 @@ infix fun <T : Comparable<T>> ClosedRange<T>.shouldNotBeInRange(input: ClosedRan
     )
 }
 
-fun <E> Iterable<E>.shouldMatchAtLeastOneOf(predicate: (E) -> Boolean): Iterable<E> {
-    this.forEach {
-        if (predicate(it))
-            return this
+fun <E> Iterable<E>.shouldMatchAtLeastOneOf(predicate: (E) -> Boolean): Iterable<E> = also {
+    if (it.none(predicate)) {
+        fail("Iterable had no matching items")
     }
-    fail("Iterable had no matching items")
 }
 
 fun <E> Iterable<E>.shouldMatchAllWith(predicate: (E) -> Boolean): Iterable<E> {

common/src/main/kotlin/org/amshove/kluent/internal/Assertions.kt

@@ -1,26 +1,14 @@
 package org.amshove.kluent.internal
 
-import org.amshove.kluent.*
+import org.amshove.kluent.fail
 import kotlin.jvm.JvmName
 import kotlin.reflect.KClass
 import kotlin.test.asserter
 
 internal fun assertTrue(message: String, boolean: Boolean) = assertTrue(boolean, message)
 internal fun assertTrue(actual: Boolean, message: String? = null) {
     if (!actual) {
-        if (errorCollector.getCollectionMode() == ErrorCollectionMode.Soft) {
-            try {
-                throw AssertionError(message)
-            } catch (ex: AssertionError) {
-                errorCollector.pushError(ex)
-            }
-        } else {
-            try {
-                throw AssertionError(message)
-            } catch (ex: AssertionError) {
-                throw assertionError(ex)
-            }
-        }
+        fail(message)
     }
 }
 
@@ -100,19 +88,19 @@ internal fun <K, V, M : Map<K, V>> mapsEqualUnordered(m1: M?, m2: M?): Boolean {
     return true
 }
 
-internal fun failExpectedActual(message: String, expected: String?, actual: String?): Nothing = fail("""
+internal fun failExpectedActual(message: String, expected: String?, actual: String?) = fail("""
     |$message
     |   Expected:   $expected
     |   Actual:     $actual
 """.trimMargin())
 
-internal fun failCollectionWithDifferentItems(message: String, expected: String?, actual: String?): Nothing = fail("""
+internal fun failCollectionWithDifferentItems(message: String, expected: String?, actual: String?) = fail("""
     |$message
     |${if (!expected.isNullOrEmpty()) "Items included on the expected collection but not in the actual: $expected" else ""}
     |${if (!actual.isNullOrEmpty()) "Items included on the actual collection but not in the expected: $actual" else ""}
 """.trimMargin())
 
-internal fun failFirstSecond(message: String, first: String?, second: String?): Nothing = fail("""
+internal fun failFirstSecond(message: String, first: String?, second: String?) = fail("""
     |$message
     |   First:      $first
     |   Second:     $second

common/src/main/kotlin/org/amshove/kluent/internal/Utility.kt

@@ -10,9 +10,6 @@ internal fun joinKeys(map: Map<*, *>) = "Keys: [${join(map.keys)}]"
 internal fun joinValues(map: Map<*, *>) = "Values: [${join(map.values)}]"
 internal fun joinPairs(map: Map<*, *>) = "Pairs: [${map.map { it.toPair() }.joinToString(", ")}]"
 
-internal fun fail(message: String): Nothing {
-    throw AssertionError(message)
-}
 
 internal fun messagePrefix(message: String?) = if (message == null) "" else "$message. "
 

common/src/test/kotlin/org/amshove/kluent/tests/basic/ShouldShould.kt

@@ -1,7 +1,6 @@
 package org.amshove.kluent.tests.basic
 
 import org.amshove.kluent.internal.assertFails
-import org.amshove.kluent.internal.assertTrue
 import kotlin.test.Test
 import org.amshove.kluent.should
 import org.amshove.kluent.tests.Person
@@ -55,7 +54,10 @@ class ShouldShould {
         try {
             peter.shouldHaveUppercaseName()
         } catch (e: AssertionError) {
-            assertTrue(e.message!!.startsWith("The name of $peter should be uppercase"))
+            e.message!!.replace("\\s+|\\t|\\n".toRegex(), " ").trim().startsWith("""
+                The following assertion failed:
+                The name of $peter should be uppercase"""
+                    .replace("\\s+|\\t|\\n".toRegex(), " ").trim())
         }
     }
 

docs/BasicAssertions.md

@@ -33,3 +33,8 @@ isMale.shouldBeFalse()
 isMale.shouldNotBeTrue()
 isMale.shouldNotBeFalse()
 ```
+
+## Failing with a message
+```kt
+fail("You did not know the airspeed velocity of an unladen swallow!")
+```

docs/SoftlyAssertions.md

@@ -162,3 +162,9 @@ If you like, you can use a bit different syntax achieving the same result:
             guests.size.shouldBeEqualTo(6)
         }
 ```
+
+## Compatibility note
+The following assertions are not compatible with `assertSoftly` and exit the test immediately after failure:
+* `assertFails`
+* `shouldNotBeNull`
+* `a.shouldBeInstanceOf<B>()`

js/src/main/kotlin/org/amshove/kluent/internal/checkResultIsFailure.kt

@@ -1,19 +1,20 @@
 package org.amshove.kluent.internal
 
+import org.amshove.kluent.hardFail
 import kotlin.reflect.KClass
 
 @PublishedApi
 internal actual fun <T : Throwable> checkResultIsFailure(exceptionClass: KClass<T>, message: String?, blockResult: Result<Unit>): T {
     blockResult.fold(
             onSuccess = {
-                fail(messagePrefix(message) + "Expected an exception of $exceptionClass to be thrown, but was completed successfully.")
+                hardFail(messagePrefix(message) + "Expected an exception of $exceptionClass to be thrown, but was completed successfully.")
             },
             onFailure = { e ->
                 if (exceptionClass.isInstance(e)) {
                     @Suppress("UNCHECKED_CAST")
                     return e as T
                 }
-                fail(messagePrefix(message) + "Expected an exception of $exceptionClass to be thrown, but was $e")
+                hardFail(messagePrefix(message) + "Expected an exception of $exceptionClass to be thrown, but was $e")
             }
     )
 }

jvm/src/main/kotlin/org/amshove/kluent/Equivalency.kt

@@ -1,6 +1,5 @@
 package org.amshove.kluent
 
-import org.amshove.kluent.internal.fail
 import java.lang.reflect.InvocationTargetException
 import java.util.*
 import kotlin.reflect.KProperty1

jvm/src/main/kotlin/org/amshove/kluent/Reflection.kt

@@ -9,13 +9,17 @@ infix fun Any?.shouldBeInstanceOf(className: Class<*>) = assertTrue("Expected $t
 
 infix fun Any?.shouldBeInstanceOf(className: KClass<*>) = assertTrue("Expected $this to be an instance of $className", className.isInstance(this))
 
-inline fun <reified T> Any?.shouldBeInstanceOf(): T = if (this is T) this else throw AssertionError("Expected $this to be an instance or subclass of ${T::class.qualifiedName}")
+inline fun <reified T> Any?.shouldBeInstanceOf(): T = if (this is T) this else hardFail("Expected $this to be an instance or subclass of ${T::class.qualifiedName}")
 
 infix fun Any?.shouldNotBeInstanceOf(className: Class<*>) = assertFalse("Expected $this to not be an instance of $className", className.isInstance(this))
 
 infix fun Any?.shouldNotBeInstanceOf(className: KClass<*>) = assertFalse("Expected $this to not be an instance of $className", className.isInstance(this))
 
-inline fun <reified T> Any?.shouldNotBeInstanceOf() = if (this !is T) this else throw AssertionError("Expected $this to not be an instance or subclass of ${T::class.qualifiedName}")
+inline fun <reified T> Any?.shouldNotBeInstanceOf() = also {
+    if (this is T) {
+        fail("Expected $this to not be an instance or subclass of ${T::class.qualifiedName}")
+    }
+}
 
 infix fun <T : Any> T?.shouldHaveTheSameClassAs(other: Any) = apply {
     if (!haveSameClasses(this, other)) {

jvm/src/main/kotlin/org/amshove/kluent/internal/checkResultIsFailure.kt

@@ -1,5 +1,6 @@
 package org.amshove.kluent.internal
 
+import org.amshove.kluent.hardFail
 import kotlin.reflect.KClass
 
 /** Asserts that a [blockResult] is a failure with the specific exception type being thrown. */
@@ -8,15 +9,15 @@ internal actual fun <T : Throwable> checkResultIsFailure(exceptionClass: KClass<
     blockResult.fold(
             onSuccess = {
                 val msg = messagePrefix(message)
-                fail(msg + "Expected an exception of ${exceptionClass.java} to be thrown, but was completed successfully.")
+                hardFail(msg + "Expected an exception of ${exceptionClass.java} to be thrown, but was completed successfully.")
             },
             onFailure = { e ->
                 if (exceptionClass.java.isInstance(e)) {
                     @Suppress("UNCHECKED_CAST")
                     return e as T
                 }
 
-                fail(messagePrefix(message) + "Expected an exception of ${exceptionClass.java} to be thrown, but was $e")
+                hardFail(messagePrefix(message) + "Expected an exception of ${exceptionClass.java} to be thrown, but was $e")
             }
     )
 }

jvm/src/test/kotlin/org/amshove/kluent/tests/assertions/softly/AssertSoftly.kt

@@ -181,4 +181,27 @@ class AssertSoftly {
         }
         assertEquals(true, errorThrown)
     }
+
+    @Test
+    fun assertSoftlyCollections() {
+        // arrange
+        val list = listOf('x', 'y', 'z')
+
+        // assert
+        try {
+            assertSoftly {
+                list shouldHaveSize 2
+                list shouldContainSame listOf('x', 'z')
+            }
+        } catch (e: Throwable) {
+            e.message!!.replace("\\s+|\\t|\\n".toRegex(), " ").trim().shouldBeEqualTo("""
+                The following 2 assertions failed:
+                1) Expected collection size to be 2 but was 3
+                    at org.amshove.kluent.tests.assertions.softly.AssertSoftly.assertSoftlyCollections(AssertSoftly.kt:193)
+                2) The collection doesn't have the same items Items included on the actual collection but not in the expected: y
+                    at org.amshove.kluent.tests.assertions.softly.AssertSoftly.assertSoftlyCollections(AssertSoftly.kt:194)
+                """.replace("\\s+|\\t|\\n".toRegex(), " ").trim())
+        }
+    }
+
 }

native/src/main/kotlin/org/amshove/kluent/internal/checkResultIsFailure.kt

@@ -1,19 +1,20 @@
 package org.amshove.kluent.internal
 
+import org.amshove.kluent.hardFail
 import kotlin.reflect.KClass
 
 @PublishedApi
 internal actual fun <T : Throwable> checkResultIsFailure(exceptionClass: KClass<T>, message: String?, blockResult: Result<Unit>): T {
     blockResult.fold(
             onSuccess = {
-                fail(messagePrefix(message) + "Expected an exception of $exceptionClass to be thrown, but was completed successfully.")
+                hardFail(messagePrefix(message) + "Expected an exception of $exceptionClass to be thrown, but was completed successfully.")
             },
             onFailure = { e ->
                 if (exceptionClass.isInstance(e)) {
                     @Suppress("UNCHECKED_CAST")
                     return e as T
                 }
-                fail(messagePrefix(message) + "Expected an exception of $exceptionClass to be thrown, but was $e")
+                hardFail(messagePrefix(message) + "Expected an exception of $exceptionClass to be thrown, but was $e")
             }
     )
 }