Skip to content
This repository has been archived by the owner on Dec 7, 2019. It is now read-only.

#125 Report Path sanitization #126

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions composer/src/main/kotlin/com/gojuno/composer/Main.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import com.google.gson.Gson
import rx.Observable
import rx.schedulers.Schedulers
import java.io.File
import java.util.*
import java.util.Date
import java.util.concurrent.TimeUnit

sealed class Exit(val code: Int, val message: String?) {
Expand Down Expand Up @@ -90,7 +90,7 @@ fun main(rawArgs: Array<String>) {
.flatMap { adbDeviceTestRun ->
writeJunit4Report(
suite = adbDeviceTestRun.toSuite(args.testPackage),
outputFile = File(File(args.outputDirectory, "junit4-reports"), "${device.id}.xml")
outputFile = File(File(args.outputDirectory, "junit4-reports"), "${device.sanitizedId()}.xml")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, this might break someones report parsing, we should emphasize this in release notes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also an edge case when filenames for multiple devices will be equal after sanitization e.g. both 123:456 and 123-456.

).toSingleDefault(adbDeviceTestRun)
}
.subscribeOn(Schedulers.io())
Expand Down Expand Up @@ -190,7 +190,9 @@ data class Device(
val id: String,
val logcat: File,
val instrumentationOutput: File
)
){
fun sanitizedId() = id.replace(":","-")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a property would look better, but I don't have strong opinion :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need tests btw :)

}

fun AdbDeviceTestRun.toSuite(testPackage: String): Suite = Suite(
testPackage = testPackage,
Expand Down
13 changes: 10 additions & 3 deletions composer/src/main/kotlin/com/gojuno/composer/TestRun.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package com.gojuno.composer

import com.gojuno.commander.android.*
import com.gojuno.commander.android.AdbDevice
import com.gojuno.commander.android.adb
import com.gojuno.commander.android.log
import com.gojuno.commander.android.pullFolder
import com.gojuno.commander.android.redirectLogcatToFile
import com.gojuno.commander.os.Notification
import com.gojuno.commander.os.nanosToHumanReadableTime
import com.gojuno.commander.os.process
Expand Down Expand Up @@ -38,6 +42,8 @@ data class AdbDeviceTest(
}
}

fun AdbDevice.sanitizedId() = id.replace(":","-")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it would be great to create simple data class for id with an extension sanitized() to avoid duplication of those methods here and in Main.kt. Or even having separate val sanitized for storing sanitized path. WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not good, we shouldn't duplicate implementation, it'll be easy to de-sync them


fun AdbDevice.runTests(
testPackageName: String,
testRunnerClass: String,
Expand All @@ -48,7 +54,7 @@ fun AdbDevice.runTests(
): Single<AdbDeviceTestRun> {

val adbDevice = this
val logsDir = File(File(outputDir, "logs"), adbDevice.id)
val logsDir = File(File(outputDir, "logs"), adbDevice.sanitizedId())
val instrumentationOutputFile = File(logsDir, "instrumentation.output")

val runTests = process(
Expand Down Expand Up @@ -157,7 +163,7 @@ data class PulledFiles(
private fun pullTestFiles(adbDevice: AdbDevice, test: InstrumentationTest, outputDir: File, verboseOutput: Boolean): Single<PulledFiles> = Single
// TODO: Add support for spoon files dir.
.fromCallable {
File(File(File(outputDir, "screenshots"), adbDevice.id), test.className).apply { mkdirs() }
File(File(File(outputDir, "screenshots"), adbDevice.sanitizedId()), test.className).apply { mkdirs() }
}
.flatMap { screenshotsFolderOnHostMachine ->
adbDevice
Expand Down Expand Up @@ -225,3 +231,4 @@ private fun saveLogcat(adbDevice: AdbDevice, logsDir: File): Observable<Pair<Str
private fun logcatFileForDevice(logsDir: File) = File(logsDir, "full.logcat")

private fun logcatFileForTest(logsDir: File, className: String, testName: String): File = File(File(logsDir, className), "$testName.logcat")

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ data class HtmlDevice(
)

fun Device.toHtmlDevice(htmlReportDir: File) = HtmlDevice(
id = id,
id = sanitizedId(),
logcatPath = logcat.relativePathTo(htmlReportDir),
instrumentationOutputPath = instrumentationOutput.relativePathTo(htmlReportDir)
)
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.gojuno.composer.html

import com.gojuno.composer.AdbDeviceTest
import com.gojuno.composer.sanitizedId
import com.google.gson.annotations.SerializedName
import java.io.File
import java.util.concurrent.TimeUnit.NANOSECONDS
Expand Down Expand Up @@ -85,7 +86,7 @@ fun AdbDeviceTest.toHtmlFullTest(suiteId: String, htmlReportDir: File) = HtmlFul
else -> null
},
logcatPath = logcat.relativePathTo(htmlReportDir),
deviceId = adbDevice.id,
deviceId = adbDevice.sanitizedId(),
properties = emptyMap(), // TODO: add properties support.
filePaths = files.map { it.relativePathTo(htmlReportDir) },
screenshots = screenshots.map {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.gojuno.composer.html

import com.gojuno.composer.Suite
import com.gojuno.composer.sanitizedId
import com.google.gson.Gson
import org.apache.commons.lang3.StringEscapeUtils
import rx.Completable
Expand Down Expand Up @@ -66,7 +67,7 @@ fun writeHtmlReport(gson: Gson, suites: List<Suite>, outputDir: File, date: Date

suite
.tests
.map { it to File(File(suitesDir, "$suiteId"), it.adbDevice.id).apply { mkdirs() } }
.map { it to File(File(suitesDir, "$suiteId"), it.adbDevice.sanitizedId()).apply { mkdirs() } }
.map { (test, testDir) -> Triple(test, test.toHtmlFullTest(suiteId = "$suiteId", htmlReportDir = testDir), testDir) }
.forEach { (test, htmlTest, testDir) ->
val testJson = gson.toJson(htmlTest)
Expand Down