From 68f38bf6589006c9c3579d724245de8034ca6a28 Mon Sep 17 00:00:00 2001 From: bootstraponline Date: Thu, 29 Nov 2018 12:51:14 -0500 Subject: [PATCH] Fix parsing empty testcase --- .../src/main/kotlin/ftl/gc/GcStorage.kt | 4 +- .../kotlin/ftl/reports/util/ReportManager.kt | 8 ++-- .../main/kotlin/ftl/reports/xml/JUnitXml.kt | 28 ++++++------- .../ftl/reports/xml/model/JUnitTestCase.kt | 11 ++++-- .../ftl/reports/xml/model/JUnitTestSuite.kt | 8 ++-- .../src/main/kotlin/ftl/shard/Shard.kt | 8 ++-- .../kotlin/ftl/reports/HtmlErrorReportTest.kt | 14 +++---- .../kotlin/ftl/reports/xml/JUnitXmlTest.kt | 39 ++++++++++++------- 8 files changed, 70 insertions(+), 50 deletions(-) diff --git a/test_runner/src/main/kotlin/ftl/gc/GcStorage.kt b/test_runner/src/main/kotlin/ftl/gc/GcStorage.kt index 782811ed01..ed35d1b446 100644 --- a/test_runner/src/main/kotlin/ftl/gc/GcStorage.kt +++ b/test_runner/src/main/kotlin/ftl/gc/GcStorage.kt @@ -10,7 +10,7 @@ import ftl.args.IosArgs import ftl.config.FtlConstants import ftl.config.FtlConstants.GCS_PREFIX import ftl.reports.xml.model.JUnitTestResult -import ftl.reports.xml.parseIosXml +import ftl.reports.xml.parseAllSuitesXml import ftl.reports.xml.xmlToString import ftl.util.Utils.fatalError import ftl.util.Utils.join @@ -85,7 +85,7 @@ object GcStorage { fun downloadJunitXml(args: IArgs): JUnitTestResult? { val oldXmlPath = download(args.smartFlankGcsPath, ignoreError = true) if (oldXmlPath.isNotEmpty()) { - return parseIosXml(Paths.get(oldXmlPath)) + return parseAllSuitesXml(Paths.get(oldXmlPath)) } return null diff --git a/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt b/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt index 3db6dfb96b..ffc83df5bf 100644 --- a/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt +++ b/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt @@ -9,8 +9,8 @@ import ftl.reports.HtmlErrorReport import ftl.reports.JUnitReport import ftl.reports.MatrixResultsReport import ftl.reports.xml.model.JUnitTestResult -import ftl.reports.xml.parseAndroidXml -import ftl.reports.xml.parseIosXml +import ftl.reports.xml.parseOneSuiteXml +import ftl.reports.xml.parseAllSuitesXml import ftl.util.ArtifactRegex import ftl.util.resolveLocalRunPath import java.io.File @@ -66,9 +66,9 @@ object ReportManager { private fun parseTestSuite(matrices: MatrixMap, args: IArgs): JUnitTestResult? { val iosXml = args is IosArgs return if (iosXml) { - processXml(matrices, ::parseIosXml) + processXml(matrices, ::parseAllSuitesXml) } else { - processXml(matrices, ::parseAndroidXml) + processXml(matrices, ::parseOneSuiteXml) } } diff --git a/test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt b/test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt index 023addd60d..0fec073d21 100644 --- a/test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt +++ b/test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt @@ -25,36 +25,36 @@ fun JUnitTestResult?.xmlToString(): String { return prefix + xmlPrettyWriter.writeValueAsString(this) } -fun parseAndroidXml(bytes: ByteArray): JUnitTestResult { +fun parseOneSuiteXml(bytes: ByteArray): JUnitTestResult { return JUnitTestResult(mutableListOf(xmlMapper.readValue(bytes, JUnitTestSuite::class.java))) } -fun parseAndroidXml(path: Path): JUnitTestResult { - return parseAndroidXml(xmlBytes(path)) +fun parseOneSuiteXml(path: Path): JUnitTestResult { + return parseOneSuiteXml(xmlBytes(path)) } -fun parseAndroidXml(path: File): JUnitTestResult { - return parseAndroidXml(xmlBytes(path.toPath())) +fun parseOneSuiteXml(path: File): JUnitTestResult { + return parseOneSuiteXml(xmlBytes(path.toPath())) } -fun parseAndroidXml(data: String): JUnitTestResult { - return parseAndroidXml(data.toByteArray()) +fun parseOneSuiteXml(data: String): JUnitTestResult { + return parseOneSuiteXml(data.toByteArray()) } // -- -fun parseIosXml(bytes: ByteArray): JUnitTestResult { +fun parseAllSuitesXml(bytes: ByteArray): JUnitTestResult { return xmlMapper.readValue(bytes, JUnitTestResult::class.java) } -fun parseIosXml(path: Path): JUnitTestResult { - return parseIosXml(xmlBytes(path)) +fun parseAllSuitesXml(path: Path): JUnitTestResult { + return parseAllSuitesXml(xmlBytes(path)) } -fun parseIosXml(path: File): JUnitTestResult { - return parseIosXml(path.toPath()) +fun parseAllSuitesXml(path: File): JUnitTestResult { + return parseAllSuitesXml(path.toPath()) } -fun parseIosXml(data: String): JUnitTestResult { - return parseIosXml(data.toByteArray()) +fun parseAllSuitesXml(data: String): JUnitTestResult { + return parseAllSuitesXml(data.toByteArray()) } diff --git a/test_runner/src/main/kotlin/ftl/reports/xml/model/JUnitTestCase.kt b/test_runner/src/main/kotlin/ftl/reports/xml/model/JUnitTestCase.kt index 0a8da493f3..09510bb259 100644 --- a/test_runner/src/main/kotlin/ftl/reports/xml/model/JUnitTestCase.kt +++ b/test_runner/src/main/kotlin/ftl/reports/xml/model/JUnitTestCase.kt @@ -13,12 +13,13 @@ private class FilterNotNull { // https://android.googlesource.com/platform/tools/base/+/tools_r22/ddmlib/src/main/java/com/android/ddmlib/testrunner/XmlTestRunListener.java#256 data class JUnitTestCase( + // name, classname, and time are always present except for empty test cases @JacksonXmlProperty(isAttribute = true) - val name: String, + val name: String?, @JacksonXmlProperty(isAttribute = true) - val classname: String, + val classname: String?, @JacksonXmlProperty(isAttribute = true) - val time: String, + val time: String?, // iOS contains multiple failures for a single test. // JUnit XML allows arbitrary amounts of failure/error tags @@ -37,6 +38,10 @@ data class JUnitTestCase( @JsonInclude(JsonInclude.Include.NON_NULL) var webLink: String? = null + fun empty(): Boolean { + return name == null || classname == null || time == null + } + fun failed(): Boolean { return failures?.isNotEmpty() == true || errors?.isNotEmpty() == true } diff --git a/test_runner/src/main/kotlin/ftl/reports/xml/model/JUnitTestSuite.kt b/test_runner/src/main/kotlin/ftl/reports/xml/model/JUnitTestSuite.kt index 28d5eeb55f..1b3e5b85e6 100644 --- a/test_runner/src/main/kotlin/ftl/reports/xml/model/JUnitTestSuite.kt +++ b/test_runner/src/main/kotlin/ftl/reports/xml/model/JUnitTestSuite.kt @@ -86,11 +86,11 @@ data class JUnitTestSuite( var mergedTime = 0.0 this.testcases?.forEach { testcase -> - // if test was skipped, then continue to skip it. - if (testcase.skipped()) return@forEach + // if test was skipped or empty, then continue to skip it. + if (testcase.skipped() || testcase.empty()) return@forEach // if the test succeeded, use the new time value - if (testcase.successful()) { + if (testcase.successful() && testcase.time != null) { mergedTime += testcase.time.toDouble() mergedTestCases.add( JUnitTestCase( @@ -107,7 +107,7 @@ data class JUnitTestSuite( it.successful() && it.name == testcase.name && it.classname == testcase.classname } ?: return@forEach - mergedTime += lastSuccessfulRun.time.toDouble() + if (lastSuccessfulRun.time != null) mergedTime += lastSuccessfulRun.time.toDouble() mergedTestCases.add( JUnitTestCase( name = testcase.name, diff --git a/test_runner/src/main/kotlin/ftl/shard/Shard.kt b/test_runner/src/main/kotlin/ftl/shard/Shard.kt index ab54557dd5..2ace9c991a 100644 --- a/test_runner/src/main/kotlin/ftl/shard/Shard.kt +++ b/test_runner/src/main/kotlin/ftl/shard/Shard.kt @@ -48,7 +48,7 @@ object Shard { private fun JUnitTestCase.iosKey(): String { // FTL iOS XML appends `()` to each test name. ex: `testBasicSelection()` // xctestrun file requires classname/name with no `()` - val testName = name.substringBefore('(') + val testName = name?.substringBefore('(') return "$classname/$testName" } @@ -65,8 +65,10 @@ object Shard { // Create a map with information from previous junit run oldTestResult.testsuites?.forEach { testsuite -> testsuite.testcases?.forEach { testcase -> - val key = if (android) testcase.androidKey() else testcase.iosKey() - junitMap[key] = testcase.time.toDouble() + if (!testcase.empty() && testcase.time != null) { + val key = if (android) testcase.androidKey() else testcase.iosKey() + junitMap[key] = testcase.time.toDouble() + } } } diff --git a/test_runner/src/test/kotlin/ftl/reports/HtmlErrorReportTest.kt b/test_runner/src/test/kotlin/ftl/reports/HtmlErrorReportTest.kt index 737615df12..0b8571553c 100644 --- a/test_runner/src/test/kotlin/ftl/reports/HtmlErrorReportTest.kt +++ b/test_runner/src/test/kotlin/ftl/reports/HtmlErrorReportTest.kt @@ -2,21 +2,21 @@ package ftl.reports import com.google.common.truth.Truth.assertThat import ftl.reports.xml.JUnitXmlTest -import ftl.reports.xml.parseAndroidXml -import ftl.reports.xml.parseIosXml +import ftl.reports.xml.parseOneSuiteXml +import ftl.reports.xml.parseAllSuitesXml import org.junit.Test class HtmlErrorReportTest { @Test fun reactJson_androidPassXml() { - val results = HtmlErrorReport.groupItemList(parseAndroidXml(JUnitXmlTest.androidPassXml)) + val results = HtmlErrorReport.groupItemList(parseOneSuiteXml(JUnitXmlTest.androidPassXml)) assertThat(results).isNull() } @Test fun reactJson_androidFailXml() { - val results = HtmlErrorReport.groupItemList(parseAndroidXml(JUnitXmlTest.androidFailXml)) + val results = HtmlErrorReport.groupItemList(parseOneSuiteXml(JUnitXmlTest.androidFailXml)) ?: throw RuntimeException("null") val group = results.first @@ -41,7 +41,7 @@ class HtmlErrorReportTest { @Test fun reactJson_androidFailXml_merged() { // 4 tests - 2 pass, 2 fail. we should have 2 failures in the report - val mergedXml = parseAndroidXml(JUnitXmlTest.androidFailXml) + val mergedXml = parseOneSuiteXml(JUnitXmlTest.androidFailXml) mergedXml.merge(mergedXml) assertThat(mergedXml.testsuites?.first()?.testcases?.size).isEqualTo(4) @@ -72,14 +72,14 @@ class HtmlErrorReportTest { @Test fun reactJson_iosPassXml() { - val results = HtmlErrorReport.groupItemList(parseIosXml(JUnitXmlTest.iosPassXml)) + val results = HtmlErrorReport.groupItemList(parseAllSuitesXml(JUnitXmlTest.iosPassXml)) assertThat(results).isNull() } @Test fun reactJson_iosFailXml() { val results = - HtmlErrorReport.groupItemList(parseIosXml(JUnitXmlTest.iosFailXml)) ?: throw RuntimeException("null") + HtmlErrorReport.groupItemList(parseAllSuitesXml(JUnitXmlTest.iosFailXml)) ?: throw RuntimeException("null") val group = results.first assertThat(group.size).isEqualTo(1) diff --git a/test_runner/src/test/kotlin/ftl/reports/xml/JUnitXmlTest.kt b/test_runner/src/test/kotlin/ftl/reports/xml/JUnitXmlTest.kt index 20ba68bd83..b8764345b2 100644 --- a/test_runner/src/test/kotlin/ftl/reports/xml/JUnitXmlTest.kt +++ b/test_runner/src/test/kotlin/ftl/reports/xml/JUnitXmlTest.kt @@ -23,9 +23,22 @@ class JUnitXmlTest { """.trimIndent() } + @Test + fun empty_testcase() { + val xml = """ + + + + + + """.trimIndent() + + parseAllSuitesXml(xml) + } + @Test fun merge_android() { - val mergedXml = parseAndroidXml(androidPassXml).merge(parseAndroidXml(androidFailXml)) + val mergedXml = parseOneSuiteXml(androidPassXml).merge(parseOneSuiteXml(androidFailXml)) val merged = mergedXml.xmlToString() val testSuite = mergedXml.testsuites?.first() ?: throw java.lang.RuntimeException("no test suite") @@ -57,7 +70,7 @@ junit.framework.Assert.fail(Assert.java:50) @Test fun merge_ios() { - val merged = parseIosXml(iosPassXml).merge(parseIosXml(iosFailXml)).xmlToString() + val merged = parseAllSuitesXml(iosPassXml).merge(parseAllSuitesXml(iosFailXml)).xmlToString() val expected = """ @@ -78,13 +91,13 @@ junit.framework.Assert.fail(Assert.java:50) @Test fun parse_androidSkipped() { - val parsed = parseAndroidXml(androidSkipped) + val parsed = parseOneSuiteXml(androidSkipped) assertThat(parsed.testsuites?.first()?.testcases?.first()?.skipped).isNull() } @Test fun merge_androidSkipped() { - val merged = parseAndroidXml(androidSkipped) + val merged = parseOneSuiteXml(androidSkipped) merged.merge(merged) val actual = merged.xmlToString() @@ -108,7 +121,7 @@ junit.framework.Assert.fail(Assert.java:50) @Test fun junitXmlToString_androidPassXml() { - val parsed = parseAndroidXml(androidPassXml).xmlToString() + val parsed = parseOneSuiteXml(androidPassXml).xmlToString() val expected = """ @@ -124,7 +137,7 @@ junit.framework.Assert.fail(Assert.java:50) @Test fun junitXmlToString_androidFailXml() { - val parsed = parseAndroidXml(androidFailXml).xmlToString() + val parsed = parseOneSuiteXml(androidFailXml).xmlToString() val expected = """ @@ -144,7 +157,7 @@ junit.framework.Assert.fail(Assert.java:50) @Test fun junitXmlToString_iosPassXml() { - val parsed = parseIosXml(iosPassXml).xmlToString() + val parsed = parseAllSuitesXml(iosPassXml).xmlToString() val expected = """ @@ -161,7 +174,7 @@ junit.framework.Assert.fail(Assert.java:50) @Test fun junitXmlToString_iosFailXml() { - val parsed = parseIosXml(iosFailXml).xmlToString() + val parsed = parseAllSuitesXml(iosFailXml).xmlToString() val expected = """ @@ -181,7 +194,7 @@ junit.framework.Assert.fail(Assert.java:50) @Test fun parse_androidPassXml() { - val testSuite = parseAndroidXml(androidPassXml) + val testSuite = parseOneSuiteXml(androidPassXml) .testsuites?.first() ?: throw RuntimeException("Missing test suite") with(testSuite) { @@ -210,7 +223,7 @@ junit.framework.Assert.fail(Assert.java:50) @Test fun parse_androidFailXml() { - val testSuite = parseAndroidXml(androidFailXml) + val testSuite = parseOneSuiteXml(androidFailXml) .testsuites?.first() ?: throw RuntimeException("Missing test suite") with(testSuite) { @@ -245,7 +258,7 @@ junit.framework.Assert.fail(Assert.java:50) @Test fun parse_iosPassXml() { - val testSuites = parseIosXml(iosPassXml) + val testSuites = parseAllSuitesXml(iosPassXml) val testSuite = testSuites.testsuites?.first() assertThat(testSuite).isNotNull() @@ -277,7 +290,7 @@ junit.framework.Assert.fail(Assert.java:50) @Test fun parse_iosFailXml() { - val testSuites = parseIosXml(iosFailXml) + val testSuites = parseAllSuitesXml(iosFailXml) val testSuite = testSuites.testsuites?.first() assertThat(testSuite).isNotNull() @@ -362,7 +375,7 @@ junit.framework.Assert.fail(Assert.java:50) // * c() failed in newRun and passed in oldRun. timing info copied over from oldRun // * d() was skipped in newRun and successful in oldRun. d() is excluded from the merged result - val merged = parseIosXml(newRun).mergeTestTimes(parseIosXml(oldRun)).xmlToString() + val merged = parseAllSuitesXml(newRun).mergeTestTimes(parseAllSuitesXml(oldRun)).xmlToString() val expected = """