Use function for expected failures

dinomite · dinomite · commit 141a784062cb · 2021-01-03T15:24:10.000-05:00
diff --git a/README.md b/README.md
@@ -210,7 +210,7 @@ If you are going to write code, choose the appropriate base branch:
 
 There are a number of tests for functionality that is broken, mostly in the [failing](https://github.com/FasterXML/jackson-module-kotlin/tree/master/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing)
 package but a few as part of other test suites.  Instead of ignoring these tests (with JUnit's `@Ignore` annotation)
-or excluding them from being run as part of automated testing, the tests are written so as to demonstrate the failure
+or excluding them from being run as part of automated testing, the tests are written to demonstrate the failure
 (either making a call that throws an exception or with an assertion that fails) but not fail the build, except if the
 underlying issue is fixed.  This allows us to know when the tested functionality has been incidentally fixed by
 unrelated code changes.
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/TestHelpers.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/TestHelpers.kt
@@ -0,0 +1,38 @@
+package com.fasterxml.jackson.module.kotlin.test
+
+import kotlin.reflect.KClass
+import kotlin.test.fail
+
+/**
+ * Expect a block to throw an exception.  If a different type of exception is thrown or no
+ * exception is produced by the block, fail the test.  In the latter case, no exception being
+ * thrown, fixMessage is printed.
+ *
+ * This function is intended to allow failing tests to be written and run as part of the build
+ * without causing it to fail, except if the failure is fixed, in which case the fixMessage
+ * should make it clear what has happened (i.e. that some previously broken functionality
+ * has been fixed).
+ *
+ * @param throwableType The expected throwable
+ * @param fixMessage    The message to print when the block succeeds
+ * @param block         The block to execute
+ */
+fun <T : Throwable> expectFailure(
+        throwableType: KClass<T>,
+        fixMessage: String,
+        block: () -> Unit
+) {
+    try {
+        block()
+    } catch (e: Throwable) {
+        if (throwableType.isInstance(e)) {
+            // Expected exception, do nothing
+        } else {
+            fail("Expected $throwableType but got $e")
+        }
+
+        return
+    }
+
+    fail(fixMessage)
+}
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/TestHelpersTest.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/TestHelpersTest.kt
@@ -0,0 +1,37 @@
+package com.fasterxml.jackson.module.kotlin.test
+
+import org.junit.Assert.assertThrows
+import org.junit.Test
+import kotlin.test.assertEquals
+
+class TestHelpersTest {
+    @Test
+    fun expectFailure_ExpectedExceptionThrown() {
+        expectFailure(AssertionError::class, "This will not be printed") {
+            throw AssertionError("This is expected")
+        }
+    }
+
+    @Test
+    fun expectFailure_AnotherExceptionThrown() {
+        val throwable = assertThrows(AssertionError::class.java) {
+            expectFailure(AssertionError::class, "This will not be printed") {
+                throw Exception("This is not expected")
+            }
+        }
+
+        assertEquals("Expected class java.lang.AssertionError but got java.lang.Exception: This is not expected", throwable.message)
+    }
+
+    @Test
+    fun expectFailure_NoExceptionThrown() {
+        val fixMessage = "Test will fail with this message"
+        val throwable = assertThrows(AssertionError::class.java) {
+            expectFailure(AssertionError::class, fixMessage) {
+                // Do nothing
+            }
+        }
+
+        assertEquals(fixMessage, throwable.message)
+    }
+}
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github138.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github138.kt
@@ -8,30 +8,26 @@ import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlRootElement
 import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlText
 import com.fasterxml.jackson.module.kotlin.readValue
 import com.fasterxml.jackson.module.kotlin.registerKotlinModule
+import com.fasterxml.jackson.module.kotlin.test.expectFailure
 import org.junit.Test
-import kotlin.test.fail
 
 class TestGithub138 {
     @JacksonXmlRootElement(localName = "sms")
     @JsonIgnoreProperties(ignoreUnknown = true)
     data class Sms(
-        @JacksonXmlProperty(localName = "Phone", isAttribute = true)
-        val phone: String?,
+            @JacksonXmlProperty(localName = "Phone", isAttribute = true)
+            val phone: String?,
 
-        @JacksonXmlText
-        val text: String? = ""
-
-        )
+            @JacksonXmlText
+            val text: String? = ""
+    )
 
     @Test
     fun testDeserProblem() {
         val xml = """<sms Phone="435242423412" Id="43234324">Lorem ipsum</sms>"""
         val xmlMapper = XmlMapper().registerKotlinModule()
-        try {
+        expectFailure(InvalidDefinitionException::class, "GitHub #138 has been fixed!") {
             val jsms = xmlMapper.readValue<Sms>(xml)
-            fail("GitHub #138 has been fixed!")
-        } catch (e: InvalidDefinitionException) {
-            // Remove this try/catch and the `fail()` call above when this issue is fixed
         }
     }
 }
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github160DisableAnnotations.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github160DisableAnnotations.kt
@@ -4,20 +4,17 @@ import com.fasterxml.jackson.databind.MapperFeature
 import com.fasterxml.jackson.databind.exc.MismatchedInputException
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import com.fasterxml.jackson.module.kotlin.readValue
+import com.fasterxml.jackson.module.kotlin.test.expectFailure
 import org.junit.Test
-import kotlin.test.fail
 
 class TestGithub160 {
     data class DataClass(val blah: String)
 
     @Test
     fun dataClass() {
         val mapper = jacksonObjectMapper().configure(MapperFeature.USE_ANNOTATIONS, false)
-        try {
+        expectFailure(MismatchedInputException::class, "GitHub #160 has been fixed!") {
             mapper.readValue<DataClass>("""{"blah":"blah"}""")
-            fail("GitHub #160 has been fixed!")
-        } catch (e: MismatchedInputException) {
-            // Remove this try/catch and the `fail()` call above when this issue is fixed
         }
     }
 }
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github271AlphaSortProperties.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github271AlphaSortProperties.kt
@@ -2,9 +2,9 @@ package com.fasterxml.jackson.module.kotlin.test.failing
 
 import com.fasterxml.jackson.annotation.JsonPropertyOrder
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
+import com.fasterxml.jackson.module.kotlin.test.expectFailure
 import org.junit.Test
 import kotlin.test.assertEquals
-import kotlin.test.fail
 
 class TestGithub271 {
     @JsonPropertyOrder(alphabetic=true)
@@ -17,11 +17,8 @@ class TestGithub271 {
         val mapper = jacksonObjectMapper()
 
         val json = mapper.writeValueAsString(Foo("a", "c"))
-        try {
+        expectFailure(AssertionError::class, "GitHub #271 has been fixed!") {
             assertEquals("""{"a":"a","b":"b","c":"c"}""", json)
-            fail("GitHub #271 has been fixed!")
-        } catch (e: AssertionError) {
-            // Remove this try/catch and the `fail()` call above when this issue is fixed
         }
     }
 }
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github335.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github335.kt
@@ -7,9 +7,9 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo.As
 import com.fasterxml.jackson.annotation.JsonTypeInfo.Id
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import com.fasterxml.jackson.module.kotlin.readValue
+import com.fasterxml.jackson.module.kotlin.test.expectFailure
 import org.junit.Test
 import kotlin.test.assertNull
-import kotlin.test.fail
 
 class Github335Test {
     val mapper = jacksonObjectMapper()
@@ -30,12 +30,9 @@ class Github335Test {
         val json = mapper.writeValueAsString(oldEntity)
         val newEntity = mapper.readValue<MyEntity>(json)
 
-        try {
-            // newEntity.type is hte string "null" instead of the null value
+        expectFailure(AssertionError::class, "GitHub #335 has been fixed!") {
+            // newEntity.type is the string "null" instead of the null value
             assertNull(newEntity.type)
-            fail("GitHub #335 has been fixed!")
-        } catch (e: AssertionError) {
-            // Remove this try/catch and the `fail()` call above when this issue is fixed
         }
     }
 }
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github340.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github340.kt
@@ -4,9 +4,9 @@ import com.fasterxml.jackson.databind.ObjectMapper
 import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException
 import com.fasterxml.jackson.module.kotlin.KotlinModule
 import com.fasterxml.jackson.module.kotlin.readValue
+import com.fasterxml.jackson.module.kotlin.test.expectFailure
 import org.junit.Test
 import kotlin.test.assertEquals
-import kotlin.test.fail
 
 class OwnerRequestTest {
     private val jackson = ObjectMapper().registerModule(KotlinModule())
@@ -21,12 +21,9 @@ class OwnerRequestTest {
 
     @Test
     fun testDeserHit340() {
-        try {
+        expectFailure(UnrecognizedPropertyException::class, "GitHub #340 has been fixed!") {
             val value: IsField = jackson.readValue(json)
             assertEquals("Got a foo", value.foo)
-            fail("GitHub #340 has been fixed!")
-        } catch (e: UnrecognizedPropertyException) {
-            // Remove this try/catch and the `fail()` call above when this issue is fixed
         }
     }
 
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github396.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github396.kt
@@ -3,6 +3,7 @@ package com.fasterxml.jackson.module.kotlin.test.failing
 import com.fasterxml.jackson.databind.exc.MismatchedInputException
 import com.fasterxml.jackson.dataformat.xml.XmlMapper
 import com.fasterxml.jackson.module.kotlin.registerKotlinModule
+import com.fasterxml.jackson.module.kotlin.test.expectFailure
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.fail
@@ -17,13 +18,10 @@ class TestGithub396 {
         val mapper = XmlMapper().registerKotlinModule()
 
         val xml = "<product><stuff></stuff></product>"
-        try {
+        expectFailure(MismatchedInputException::class, "GitHub #396 has been fixed!") {
             val product: Product = mapper.readValue(xml, Product::class.java)
 
             assertEquals(Product(null), product)
-            fail("GitHub #396 has been fixed!")
-        } catch (e: MismatchedInputException) {
-            // Remove this try/catch and the `fail()` call above when this issue is fixed
         }
     }
 
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github50.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github50.kt
@@ -4,8 +4,8 @@ import com.fasterxml.jackson.annotation.JsonUnwrapped
 import com.fasterxml.jackson.databind.exc.InvalidDefinitionException
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import com.fasterxml.jackson.module.kotlin.readValue
+import com.fasterxml.jackson.module.kotlin.test.expectFailure
 import org.junit.Test
-import kotlin.test.fail
 
 class TestGithub50 {
     data class Name(val firstName: String, val lastName: String)
@@ -18,11 +18,8 @@ class TestGithub50 {
     @Test
     fun testGithub50UnwrappedError() {
         val json = """{"firstName":"John","lastName":"Smith","position":"Manager"}"""
-        try {
+        expectFailure(InvalidDefinitionException::class, "GitHub #50 has been fixed!") {
             val obj: Employee = jacksonObjectMapper().readValue(json)
-            fail("GitHub #50 has been fixed!")
-        } catch (e: InvalidDefinitionException) {
-            // Remove this try/catch and the `fail()` call above when this issue is fixed
         }
     }
 }
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github54.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github54.kt
@@ -5,6 +5,7 @@ import com.fasterxml.jackson.annotation.ObjectIdGenerators
 import com.fasterxml.jackson.databind.deser.UnresolvedForwardReference
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import com.fasterxml.jackson.module.kotlin.readValue
+import com.fasterxml.jackson.module.kotlin.test.expectFailure
 import org.junit.Test
 import kotlin.test.fail
 
@@ -21,11 +22,8 @@ class TestGithub54 {
         entity1.entity2 = entity2
 
         val json = mapper.writeValueAsString(entity1)
-        try {
+        expectFailure(UnresolvedForwardReference::class, "GitHub #54 has been fixed!") {
             mapper.readValue<Entity1>(json)
-            fail("GitHub #54 has been fixed!")
-        } catch (e: UnresolvedForwardReference) {
-            // Remove this try/catch and the `fail()` call above when this issue is fixed
         }
 
     }
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github71.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/Github71.kt
@@ -2,9 +2,9 @@ package com.fasterxml.jackson.module.kotlin.test.failing
 
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import com.fasterxml.jackson.module.kotlin.readValue
+import com.fasterxml.jackson.module.kotlin.test.expectFailure
 import org.junit.Test
 import kotlin.test.assertEquals
-import kotlin.test.fail
 
 class TestGithub71 {
     open class Identifiable {
@@ -15,13 +15,10 @@ class TestGithub71 {
     fun testInternalPropertySerliazation() {
         val json = jacksonObjectMapper().writeValueAsString(Identifiable())
 
-        try {
+        expectFailure(AssertionError::class, "GitHub #71 has been fixed!") {
             assertEquals("{\"identity\":null}", json) // fails: {"identity$jackson_module_kotlin":null}
             val newInstance = jacksonObjectMapper().readValue<Identifiable>(json)
             assertEquals(Identifiable(), newInstance)
-            fail("GitHub #71 has been fixed!")
-        } catch (e: AssertionError) {
-            // Remove this try/catch and the `fail()` call above when this issue is fixed
         }
     }
 }
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/GithubDatabind1329.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/GithubDatabind1329.kt
@@ -5,10 +5,10 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo
 import com.fasterxml.jackson.annotation.JsonTypeName
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import com.fasterxml.jackson.module.kotlin.readValue
+import com.fasterxml.jackson.module.kotlin.test.expectFailure
 import org.junit.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertNull
-import kotlin.test.fail
 
 /**
  * Broken in databind 2.8.0+ (not 2.8.0.rc2 which works) and not a problem with the Kotlin module
@@ -28,11 +28,8 @@ class GithubDatabind1329 {
         )
 
         assertEquals(InviteKind.CONTACT, invite.kind)
-        try {
+        expectFailure(AssertionError::class, "GitHub Databind issue #1329 has been fixed!") {
             assertNull(invite.kindForMapper)
-            fail("GitHub Databind issue #1329 has been fixed!")
-        } catch (e: AssertionError) {
-            // Remove this try/catch and the `fail()` call above when this issue is fixed
         }
 
         assertEquals("Foo", (invite.to as InviteToContact).name)
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/README.md b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing/README.md
@@ -1,33 +1,26 @@
 # Failing tests
 
-These are tests for filed issues on GitHub that have not been fixed.
+These are tests for filed issues on GitHub that have not been fixed. Failing tests are good to
+have because they can both concisely document known issues with the library but also serve as
+notification for when that functionality has been fixed.
 
 The tests pass because either the failing assertions have their `AssertionError` or the Jackson call
-that throws has its exception caught.  Each of those blocks has a `fail()` call after the call that
-is expected to throw; this `fail()` will fail the build if the associated test case is fixed,
-allowing us to know when an issue has been solved incidentally.
+that throws has its exception caught.
 
 ## Writing a failing test
 
-Failing tests are good to have because they can both concisely document known issues with the
-library but also serve as notification for when that functionality has been fixed.
-
 Create a failing test by writing it as you would any other test, making any calls or assertions
-necessary to demonstrate the failing behavior.  Then, suppress that error using a `try/catch`
-block with a descriptive `fail()` statement as the last line of the `try` to act as a
-[canary](https://en.wikipedia.org/wiki/Domestic_canary#Miner's_canary) for when the broken
-functionality is fixed.
+necessary to demonstrate the failing behavior.  Then, surround the failing call with the
+`expectFailure()` function, passing the expected exception and a message that will be printed
+if the failure does _not_ occur, which implies that the functionality has been fixed.
 
 See the below examples:
 
 ```kotlin
 @Test
 fun failingTest() {
-    try {
+    expectFailure(MismatchedInputException::class, "The call that fails with MismatchedInputException has been fixed!") {
         mapper.callThatFailsWithMismatchedInputException()
-        fail("The call that fails with MismatchedInputException has been fixed!")
-    } catch (e: MismatchedInputException) {
-        // Remove this try/catch and the `fail()` call above when this issue is fixed
     }
 }
 ```
@@ -39,12 +32,9 @@ fun serializeAndDeserializeTypeable() {
     val json = mapper.writeValueAsString(oldEntity)
     val newEntity = mapper.readValue<MyEntity>(json)
 
-    try {
+    expectFailure(AssertionError::class, "GitHub #335 has been fixed!") {
         // newEntity.type is hte string "null" instead of the null value
         assertNull(newEntity.type)
-        fail("GitHub #335 has been fixed!")
-    } catch (e: AssertionError) {
-        // Remove this try/catch and the `fail()` call above when this issue is fixed
     }
 }
 ```
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github153.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github153.kt
diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github27.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github27.kt
