Skip to content

Commit

Permalink
Fix parsing empty testcase (#402)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bootstraponline
bootstraponline authored Nov 29, 2018
1 parent fcbaf02 commit cd9c15f
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 50 deletions.
4 changes: 2 additions & 2 deletions test_runner/src/main/kotlin/ftl/gc/GcStorage.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}

Expand Down
28 changes: 14 additions & 14 deletions test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
11 changes: 8 additions & 3 deletions test_runner/src/main/kotlin/ftl/reports/xml/model/JUnitTestCase.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <testcase/>
@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
Expand All @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions test_runner/src/main/kotlin/ftl/reports/xml/model/JUnitTestSuite.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
Expand Down
8 changes: 5 additions & 3 deletions test_runner/src/main/kotlin/ftl/shard/Shard.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}

Expand All @@ -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()
}
}
}

Expand Down
14 changes: 7 additions & 7 deletions test_runner/src/test/kotlin/ftl/reports/HtmlErrorReportTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
39 changes: 26 additions & 13 deletions test_runner/src/test/kotlin/ftl/reports/xml/JUnitXmlTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,22 @@ class JUnitXmlTest {
""".trimIndent()
}

@Test
fun empty_testcase() {
val xml = """
<testsuites>
<testsuite name="" tests="0" failures="0" errors="0" skipped="0" time="0.0" timestamp="2018-11-22T00:56:07" hostname="localhost">
<testcase/>
</testsuite>
</testsuites>
""".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")
Expand Down Expand Up @@ -57,7 +70,7 @@ junit.framework.Assert.fail(Assert.java:50)</failure>

@Test
fun merge_ios() {
val merged = parseIosXml(iosPassXml).merge(parseIosXml(iosFailXml)).xmlToString()
val merged = parseAllSuitesXml(iosPassXml).merge(parseAllSuitesXml(iosFailXml)).xmlToString()
val expected = """
<?xml version='1.0' encoding='UTF-8' ?>
<testsuites>
Expand All @@ -78,13 +91,13 @@ junit.framework.Assert.fail(Assert.java:50)</failure>

@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()

Expand All @@ -108,7 +121,7 @@ junit.framework.Assert.fail(Assert.java:50)</failure>

@Test
fun junitXmlToString_androidPassXml() {
val parsed = parseAndroidXml(androidPassXml).xmlToString()
val parsed = parseOneSuiteXml(androidPassXml).xmlToString()
val expected = """
<?xml version='1.0' encoding='UTF-8' ?>
<testsuites>
Expand All @@ -124,7 +137,7 @@ junit.framework.Assert.fail(Assert.java:50)</failure>

@Test
fun junitXmlToString_androidFailXml() {
val parsed = parseAndroidXml(androidFailXml).xmlToString()
val parsed = parseOneSuiteXml(androidFailXml).xmlToString()
val expected = """
<?xml version='1.0' encoding='UTF-8' ?>
<testsuites>
Expand All @@ -144,7 +157,7 @@ junit.framework.Assert.fail(Assert.java:50)</failure>

@Test
fun junitXmlToString_iosPassXml() {
val parsed = parseIosXml(iosPassXml).xmlToString()
val parsed = parseAllSuitesXml(iosPassXml).xmlToString()
val expected = """
<?xml version='1.0' encoding='UTF-8' ?>
<testsuites>
Expand All @@ -161,7 +174,7 @@ junit.framework.Assert.fail(Assert.java:50)</failure>

@Test
fun junitXmlToString_iosFailXml() {
val parsed = parseIosXml(iosFailXml).xmlToString()
val parsed = parseAllSuitesXml(iosFailXml).xmlToString()
val expected = """
<?xml version='1.0' encoding='UTF-8' ?>
<testsuites>
Expand All @@ -181,7 +194,7 @@ junit.framework.Assert.fail(Assert.java:50)</failure>

@Test
fun parse_androidPassXml() {
val testSuite = parseAndroidXml(androidPassXml)
val testSuite = parseOneSuiteXml(androidPassXml)
.testsuites?.first() ?: throw RuntimeException("Missing test suite")

with(testSuite) {
Expand Down Expand Up @@ -210,7 +223,7 @@ junit.framework.Assert.fail(Assert.java:50)</failure>

@Test
fun parse_androidFailXml() {
val testSuite = parseAndroidXml(androidFailXml)
val testSuite = parseOneSuiteXml(androidFailXml)
.testsuites?.first() ?: throw RuntimeException("Missing test suite")

with(testSuite) {
Expand Down Expand Up @@ -245,7 +258,7 @@ junit.framework.Assert.fail(Assert.java:50)</failure>

@Test
fun parse_iosPassXml() {
val testSuites = parseIosXml(iosPassXml)
val testSuites = parseAllSuitesXml(iosPassXml)
val testSuite = testSuites.testsuites?.first()
assertThat(testSuite).isNotNull()

Expand Down Expand Up @@ -277,7 +290,7 @@ junit.framework.Assert.fail(Assert.java:50)</failure>

@Test
fun parse_iosFailXml() {
val testSuites = parseIosXml(iosFailXml)
val testSuites = parseAllSuitesXml(iosFailXml)
val testSuite = testSuites.testsuites?.first()
assertThat(testSuite).isNotNull()

Expand Down Expand Up @@ -362,7 +375,7 @@ junit.framework.Assert.fail(Assert.java:50)</failure>
// * 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 = """
<?xml version='1.0' encoding='UTF-8' ?>
<testsuites>
Expand Down

0 comments on commit cd9c15f

Please sign in to comment.