Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Additional config options for test pairs #2004

Merged
merged 9 commits into from
Jun 9, 2021
Merged
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: 7 additions & 1 deletion docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -661,13 +661,19 @@ flank:
## Default: false
# keep-file-path: false

### Additional App/Test APKS
### Additional App/Test APKS
## Include additional app/test apk pairs in the run. Apks are unique by just filename and not by path!
## If app is omitted, then the top level app is used for that pair.
## You can overwrite global config per each test pair.
## Currently supported options are: max-test-shards, test-targets, client-details, environment-variables, device
# additional-app-test-apks:
# - app: ../test_projects/android/apks/app-debug.apk
# test: ../test_projects/android/apks/app1-debug-androidTest.apk
# device:
# - model: Nexus6P
# version: 27
# - test: ../test_projects/android/apks/app2-debug-androidTest.apk
# max-test-shards: 5

### Run Timeout
## The max time this test run can execute before it is cancelled (default: unlimited).
Expand Down
25 changes: 18 additions & 7 deletions integration_tests/src/test/kotlin/integration/MultipleApksIT.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package integration
import FlankCommand
import com.google.common.truth.Truth.assertThat
import integration.config.AndroidTest
import org.junit.Assert.assertEquals
import org.junit.Test
import org.junit.experimental.categories.Category
import run
Expand All @@ -19,9 +20,9 @@ import utils.assertTestResultContainsWebLinks
import utils.findTestDirectoryFromOutput
import utils.json
import utils.loadAsTestSuite
import utils.multipleFailedTests
import utils.multipleSuccessfulTests
import utils.removeUnicode
import utils.testResults.TestSuite
import utils.toJUnitXmlFile
import utils.toOutputReportFile

Expand All @@ -48,10 +49,20 @@ class MultipleApksIT {
"MainActivity_robo_script.json"
)

resOutput.findTestDirectoryFromOutput().toJUnitXmlFile().loadAsTestSuite().run {
assertTestResultContainsWebLinks()
assertTestPass(multipleSuccessfulTests)
assertTestFail(multipleFailedTests)
val xmlResult = resOutput.findTestDirectoryFromOutput().toJUnitXmlFile().loadAsTestSuite()

xmlResult.assertTestResultContainsWebLinks()
xmlResult.assertTestPass(multipleSuccessfulTests)
xmlResult.assertTestFail(listOf("test2"))

xmlResult.testSuites.groupBy { it.name }.mapValues { it.value.flatMap(TestSuite::testCases) }.run {
assertEquals(20, get("NexusLowRes-28-en-portrait")?.size)
assertEquals(1, get("Pixel2-28-en-portrait")?.size)
assertEquals("com.example.test_app.InstrumentedTest", get("Pixel2-28-en-portrait")?.get(0)?.classname)
assertEquals("test2", get("Pixel2-28-en-portrait")?.get(0)?.name)
assertEquals(1, get("Nexus6P-27-en-portrait")?.size)
assertEquals("com.example.test_app.InstrumentedTest", get("Nexus6P-27-en-portrait")?.get(0)?.classname)
assertEquals("test", get("Nexus6P-27-en-portrait")?.get(0)?.name)
}

val outputReport = resOutput.findTestDirectoryFromOutput().toOutputReportFile().json().asOutputReport()
Expand All @@ -69,7 +80,7 @@ class MultipleApksIT {
.map { it.testAxises }
.flatten()

assertThat(testsResults.sumOf { it.suiteOverview.failures }).isEqualTo(5)
assertThat(testsResults.sumOf { it.suiteOverview.total }).isEqualTo(41)
assertThat(testsResults.sumOf { it.suiteOverview.failures }).isEqualTo(1)
assertThat(testsResults.sumOf { it.suiteOverview.total }).isEqualTo(22)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,16 @@ flank:
output-style: single
additional-app-test-apks:
- test: ../test_runner/src/test/kotlin/ftl/fixtures/tmp/apk/app-multiple-success-debug-androidTest.apk
max-test-shards: 2
- test: ../test_runner/src/test/kotlin/ftl/fixtures/tmp/apk/app-multiple-error-debug-androidTest.apk
device:
- model: Pixel2
version: 28
test-targets:
- class com.example.test_app.InstrumentedTest#test2
- test: gs://flank-open-source.appspot.com/integration/app-single-success-debug-androidTest.apk
device:
- model: Nexus6P
version: 27
disable-usage-statistics: true
output-report: json
6 changes: 6 additions & 0 deletions test_runner/flank.yml
Original file line number Diff line number Diff line change
Expand Up @@ -295,10 +295,16 @@ flank:
### Additional App/Test APKS
## Include additional app/test apk pairs in the run. Apks are unique by just filename and not by path!
## If app is omitted, then the top level app is used for that pair.
## You can overwrite global config per each test pair.
## Currently supported options are: max-test-shards, test-targets, client-details, environment-variables, device
# additional-app-test-apks:
# - app: ../test_projects/android/apks/app-debug.apk
# test: ../test_projects/android/apks/app1-debug-androidTest.apk
# device:
# - model: Nexus6P
# version: 27
# - test: ../test_projects/android/apks/app2-debug-androidTest.apk
# max-test-shards: 5

### Run Timeout
## The max time this test run can execute before it is cancelled (default: unlimited).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import com.google.testing.model.TestMatrix as GoogleTestMatrix

object GoogleTestMatrixAndroid :
TestMatrixAndroid.Execute,
(TestMatrixAndroid.Config, List<TestMatrixAndroid.Type>) -> List<TestMatrix.Data> by { config, types ->
(List<TestMatrixAndroid.TestSetup>) -> List<TestMatrix.Data> by { configTypePairs ->
runBlocking {
executeAndroidTests(config, types).map(GoogleTestMatrix::toApiModel)
executeAndroidTests(configTypePairs).map(GoogleTestMatrix::toApiModel)
}
}
7 changes: 6 additions & 1 deletion test_runner/src/main/kotlin/ftl/api/TestAndroidMatrix.kt
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,12 @@ object TestMatrixAndroid {
) : Type()
}

interface Execute : (Config, List<Type>) -> List<TestMatrix.Data>
data class TestSetup(
val config: Config,
val type: Type
)

interface Execute : (List<TestSetup>) -> List<TestMatrix.Data>
}

typealias ShardChunks = List<List<String>>
43 changes: 35 additions & 8 deletions test_runner/src/main/kotlin/ftl/args/ArgsToString.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,62 @@ package ftl.args
import ftl.args.yml.AppTestPair

private const val NEW_LINE = '\n'
private const val SPACESx8 = " "
private const val SPACESx10 = " "

object ArgsToString {

fun mapToString(map: Map<String, String>?): String {
fun mapToString(
map: Map<String, String>?,
transform: (Map.Entry<String, String>) -> String = { (key, value) -> "$SPACESx8$key: $value" }
): String {
if (map.isNullOrEmpty()) return ""
return NEW_LINE + map.map { (key, value) -> " $key: $value" }
return NEW_LINE + map.map(transform)
.joinToString(System.lineSeparator())
}

fun listToString(list: List<String?>?): String {
fun listToString(list: List<String?>?, transform: (String) -> String = { "$SPACESx8- $it" }): String {
if (list.isNullOrEmpty()) return ""
return NEW_LINE + list.filterNotNull()
.joinToString(System.lineSeparator()) { dir -> " - $dir" }
.joinToString(System.lineSeparator(), transform = transform)
}

fun objectsToString(objects: List<Any?>?): String {
fun objectsToString(objects: List<Any?>?, transform: (Any) -> String = { "$it" }): String {
if (objects.isNullOrEmpty()) return ""
return NEW_LINE + objects.filterNotNull()
.joinToString(System.lineSeparator()) { "$it" }
.joinToString(System.lineSeparator(), transform = transform)
}

fun listOfListToString(listOfList: List<List<String?>>?): String {
if (listOfList.isNullOrEmpty()) return ""
return NEW_LINE + listOfList.map { list -> list.joinToString(",") { " $it" } }
.joinToString(System.lineSeparator()) { " - $it" }
.joinToString(System.lineSeparator()) { "$SPACESx8- $it" }
}

fun apksToString(devices: List<AppTestPair>): String {
if (devices.isNullOrEmpty()) return ""
return NEW_LINE + devices.joinToString(System.lineSeparator()) { (app, test) -> " - app: $app\n test: $test" }
return NEW_LINE + devices.joinToString(System.lineSeparator()) { pair ->
buildString {
if (pair.app == null) appendLine("$SPACESx8- test: ${pair.test}")
else {
appendLine("$SPACESx8- app: ${pair.app}")
appendLine("$SPACESx8 test: ${pair.test}")
}
pair.maxTestShards?.let { appendLine("$SPACESx8 max-test-shards: $it") }
pair.clientDetails?.let {
append("$SPACESx8 client-details:${mapToString(it) { (key, value) -> "$SPACESx10 $key: $value" }}")
}
pair.testTargets?.let { list ->
appendLine("$SPACESx8 test-targets:${listToString(list) { "$SPACESx10- $it" }}")
}
pair.devices?.let { list ->
appendLine("$SPACESx8 device:")
list
.map { " $it" }
.map { it.replace("\n", "\n ") }
.forEach { append(it) }
}
}.trimEnd()
}
piotradamczyk5 marked this conversation as resolved.
Show resolved Hide resolved
}
}
12 changes: 1 addition & 11 deletions test_runner/src/main/kotlin/ftl/args/CreateAndroidArgs.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package ftl.args

import ftl.args.yml.AppTestPair
import ftl.config.AndroidConfig
import ftl.config.android.AndroidFlankConfig
import ftl.config.android.AndroidGcloudConfig
Expand Down Expand Up @@ -33,18 +32,9 @@ fun createAndroidArgs(

// flank
additionalAppTestApks = flank.additionalAppTestApks?.map {
// if additional-pair did not provide certain values, set as top level ones
val mergedClientDetails = mutableMapOf<String, String>().apply {
// merge additionalAppTestApk's client-details with top-level client-details
putAll(commonArgs.clientDetails ?: emptyMap())
putAll(it.clientDetails)
}
AppTestPair(
it.copy(
app = it.app?.normalizeFilePath(),
test = it.test.normalizeFilePath(),
environmentVariables = it.environmentVariables,
maxTestShards = it.maxTestShards ?: commonArgs.maxTestShards,
clientDetails = mergedClientDetails
)
} ?: emptyList(),
useLegacyJUnitResult = flank::useLegacyJUnitResult.require(),
Expand Down
16 changes: 16 additions & 0 deletions test_runner/src/main/kotlin/ftl/args/ValidateAndroidArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,22 @@ private fun AndroidArgs.assertAdditionalAppTestApks() {
.filter { (app, _) -> app == null }
.map { File(it.test).name }
.run { if (isNotEmpty()) throw FlankConfigurationError("Cannot resolve app apk pair for $this") }

additionalAppTestApks
.map {
copy(
appApk = it.app ?: appApk,
testApk = it.test,
commonArgs = commonArgs.copy(
maxTestShards = it.maxTestShards ?: maxTestShards,
devices = it.devices ?: devices
),
additionalAppTestApks = emptyList()
)
}
.forEach {
it.validate()
}
}

private fun AndroidArgs.assertApkFilePaths() {
Expand Down
7 changes: 6 additions & 1 deletion test_runner/src/main/kotlin/ftl/args/yml/AppTestPair.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ftl.args.yml

import com.fasterxml.jackson.annotation.JsonIgnoreProperties
import com.fasterxml.jackson.annotation.JsonProperty
import ftl.config.Device

@JsonIgnoreProperties(ignoreUnknown = true)
data class AppTestPair(
Expand All @@ -12,5 +13,9 @@ data class AppTestPair(
@JsonProperty("max-test-shards")
val maxTestShards: Int? = null,
@JsonProperty("client-details")
var clientDetails: Map<String, String> = emptyMap()
val clientDetails: Map<String, String>? = null,
@JsonProperty("test-targets")
val testTargets: List<String>? = null,
@JsonProperty("device")
val devices: List<Device>? = null
)
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,10 @@ import kotlinx.coroutines.awaitAll
import kotlinx.coroutines.coroutineScope

suspend fun executeAndroidTests(
config: TestMatrixAndroid.Config,
testMatrixTypes: List<TestMatrixAndroid.Type>,
): List<TestMatrix> = testMatrixTypes
.foldIndexed(emptyList<Deferred<TestMatrix>>()) { testMatrixTypeIndex, testMatrices, testMatrixType ->
testMatrices + executeAndroidTestMatrix(testMatrixType, testMatrixTypeIndex, config)
testSetups: List<TestMatrixAndroid.TestSetup>
): List<TestMatrix> = testSetups
.foldIndexed(emptyList<Deferred<TestMatrix>>()) { testMatrixTypeIndex, testMatrices, setUp ->
testMatrices + executeAndroidTestMatrix(setUp.type, testMatrixTypeIndex, setUp.config)
}.awaitAll()

private suspend fun executeAndroidTestMatrix(
Expand All @@ -58,10 +57,8 @@ private fun createAndroidTestMatrix(
runIndex: Int
): Testing.Projects.TestMatrices.Create {

val clientDetails = config.clientInfo(testMatrixType)

val testMatrix = TestMatrix()
.setClientInfo(clientDetails)
.setClientInfo(config.clientInfo)
.setTestSpecification(getTestSpecification(testMatrixType, config))
.setResultStorage(config.resultsStorage(contextIndex, runIndex))
.setEnvironmentMatrix(config.environmentMatrix)
Expand All @@ -73,18 +70,11 @@ private fun createAndroidTestMatrix(
}.getOrElse { e -> throw FlankGeneralError(e) }
}

fun TestMatrixAndroid.Config.clientInfo(matrix: TestMatrixAndroid.Type): ClientInfo {
return if (matrix is TestMatrixAndroid.Type.Instrumentation && matrix.clientDetails.isNotEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@pawelpasterz I am sure that there were changes done here for a fix by @asadsalman. Can we confirm that it wont be effected by this change as it seems like a revert to the previous version of the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right - this will revert those changes. But this is intentional, previous implementation merges client details (from main and additional test pair). All the other, 'additional' configs overwrite 'main' configs. I think we should be consistent here. Saying that merging is not the best option and clientDetails should be overwritten as well.

ClientInfo()
.setName("Flank")
.setClientInfoDetails(matrix.clientDetails.toClientInfoDetailList())
} else {
// https://github.com/bootstraponline/studio-google-cloud-testing/blob/203ed2890c27a8078cd1b8f7ae12cf77527f426b/firebase-testing/src/com/google/gct/testing/launcher/CloudTestsLauncher.java#L120
ClientInfo()
.setName("Flank")
.setClientInfoDetails(clientDetails?.toClientInfoDetailList())
}
}
// https://github.com/bootstraponline/studio-google-cloud-testing/blob/203ed2890c27a8078cd1b8f7ae12cf77527f426b/firebase-testing/src/com/google/gct/testing/launcher/CloudTestsLauncher.java#L120
private val TestMatrixAndroid.Config.clientInfo
get() = ClientInfo()
.setName("Flank")
.setClientInfoDetails(clientDetails?.toClientInfoDetailList())

private val TestMatrixAndroid.Config.environmentMatrix
get() = EnvironmentMatrix()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ data class AndroidFlankConfig @JsonIgnore constructor(
names = ["--additional-app-test-apks"],
split = ",",
description = [
"A list of app & test apks to include in the run. " +
"Useful for running multiple module tests within a single Flank run."
"A list of app & test apks to include in the run. Useful for running multiple module tests " +
"within a single Flank run.",
"You can overwrite global config per each test pair. Currently supported options are: " +
"max-test-shards, test-targets, client-details, environment-variables, device"
]
)
fun additionalAppTestApks(map: Map<String, String>?) {
Expand All @@ -34,9 +36,10 @@ data class AndroidFlankConfig @JsonIgnore constructor(
if (testApk != null) {
additionalAppTestApks?.add(
AppTestPair(
app = appApk,
test = testApk,
maxTestShards = map["max-test-shards"]?.toInt()
app = appApk.toString(),
test = testApk.toString(),
maxTestShards = map["max-test-shards"]?.toInt(),
testTargets = map["test-targets"]?.split(",")
)
)
}
Expand Down
21 changes: 12 additions & 9 deletions test_runner/src/main/kotlin/ftl/run/model/AndroidTestContext.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package ftl.run.model

import ftl.api.FileReference
import ftl.api.ShardChunks
import ftl.args.AndroidArgs
import ftl.args.IgnoredTestCases
import ftl.shard.Chunk

sealed class AndroidTestContext
sealed class AndroidTestContext(open val args: AndroidArgs)

data class InstrumentationTestContext(
val app: FileReference,
Expand All @@ -14,21 +15,23 @@ data class InstrumentationTestContext(
val ignoredTestCases: IgnoredTestCases = emptyList(),
val environmentVariables: Map<String, String> = emptyMap(),
val testTargetsForShard: ShardChunks = emptyList(),
val maxTestShards: Int? = null,
val clientDetails: Map<String, String> = emptyMap(),
) : AndroidTestContext()
override val args: AndroidArgs
) : AndroidTestContext(args)

data class RoboTestContext(
val app: FileReference,
val roboScript: FileReference
) : AndroidTestContext()
val roboScript: FileReference,
override val args: AndroidArgs
) : AndroidTestContext(args)

data class GameLoopContext(
val app: FileReference,
val scenarioLabels: List<String>,
val scenarioNumbers: List<String>,
) : AndroidTestContext()
override val args: AndroidArgs
) : AndroidTestContext(args)

data class SanityRoboTestContext(
val app: FileReference
) : AndroidTestContext()
val app: FileReference,
override val args: AndroidArgs
) : AndroidTestContext(args)
Loading