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

fix: Always dump shards #1221

Merged
merged 17 commits into from
Oct 13, 2020
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
3 changes: 2 additions & 1 deletion test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ data class AndroidArgs(
val testRunnerClass: String?,
val testTargets: List<String>,
val additionalAppTestApks: List<AppTestPair>,
override val useLegacyJUnitResult: Boolean
override val useLegacyJUnitResult: Boolean,
val obfuscateDumpShards: Boolean
piotradamczyk5 marked this conversation as resolved.
Show resolved Hide resolved
) : IArgs by commonArgs {
companion object : AndroidArgsCompanion()

Expand Down
5 changes: 3 additions & 2 deletions test_runner/src/main/kotlin/ftl/args/AndroidArgsCompanion.kt
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ open class AndroidArgsCompanion : IArgs.ICompanion {
internal fun load(yamlReader: Reader, cli: AndroidRunCommand? = null) =
createAndroidArgs(
config = defaultAndroidConfig() +
loadAndroidConfig(reader = yamlReader) +
cli?.config
loadAndroidConfig(reader = yamlReader) +
cli?.config,
obfuscate = cli?.obfuscate ?: false
)
}
2 changes: 1 addition & 1 deletion test_runner/src/main/kotlin/ftl/args/CommonArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,5 @@ data class CommonArgs(
override val disableResultsUpload: Boolean,
override val defaultTestTime: Double,
override val defaultClassTestTime: Double,
override val useAverageTestTimeForNewTests: Boolean,
override val useAverageTestTimeForNewTests: Boolean
) : IArgs
4 changes: 3 additions & 1 deletion test_runner/src/main/kotlin/ftl/args/CreateAndroidArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ fun createAndroidArgs(
config: AndroidConfig? = null,
gcloud: AndroidGcloudConfig = config!!.platform.gcloud,
flank: AndroidFlankConfig = config!!.platform.flank,
commonArgs: CommonArgs = config!!.prepareAndroidCommonConfig()
commonArgs: CommonArgs = config!!.prepareAndroidCommonConfig(),
obfuscate: Boolean = false
) = AndroidArgs(
commonArgs = commonArgs,
// gcloud
Expand Down Expand Up @@ -37,5 +38,6 @@ fun createAndroidArgs(
)
} ?: emptyList(),
useLegacyJUnitResult = flank.useLegacyJUnitResult!!,
obfuscateDumpShards = obfuscate,
grantPermissions = gcloud.grantPermissions
)
12 changes: 8 additions & 4 deletions test_runner/src/main/kotlin/ftl/args/CreateIosArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,27 @@ import ftl.config.ios.IosFlankConfig
import ftl.config.ios.IosGcloudConfig

fun createIosArgs(
config: IosConfig
config: IosConfig,
obfuscate: Boolean = false
) = createIosArgs(
gcloud = config.platform.gcloud,
flank = config.platform.flank,
commonArgs = config.common.createCommonArgs(config.data)
commonArgs = config.common.createCommonArgs(config.data),
obfuscate = obfuscate
)

private fun createIosArgs(
gcloud: IosGcloudConfig,
flank: IosFlankConfig,
commonArgs: CommonArgs
commonArgs: CommonArgs,
obfuscate: Boolean = false
) = IosArgs(
commonArgs = commonArgs.copy(maxTestShards = convertToShardCount(commonArgs.maxTestShards)),
xctestrunZip = gcloud.test?.normalizeFilePath().orEmpty(),
xctestrunFile = gcloud.xctestrunFile?.normalizeFilePath().orEmpty(),
xcodeVersion = gcloud.xcodeVersion,
testTargets = flank.testTargets?.filterNotNull().orEmpty()
testTargets = flank.testTargets?.filterNotNull().orEmpty(),
obfuscateDumpShards = obfuscate
)

private fun convertToShardCount(inputValue: Int) =
Expand Down
3 changes: 2 additions & 1 deletion test_runner/src/main/kotlin/ftl/args/IosArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ data class IosArgs(
val xctestrunZip: String,
val xctestrunFile: String,
val xcodeVersion: String?,
val testTargets: List<String>
val testTargets: List<String>,
val obfuscateDumpShards: Boolean
) : IArgs by commonArgs {

override val useLegacyJUnitResult = true
Expand Down
5 changes: 3 additions & 2 deletions test_runner/src/main/kotlin/ftl/args/IosArgsCompanion.kt
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ open class IosArgsCompanion : IArgs.ICompanion {
internal fun load(yamlReader: Reader, cli: IosRunCommand? = null) =
createIosArgs(
config = defaultIosConfig() +
loadIosConfig(reader = yamlReader) +
cli?.config
loadIosConfig(reader = yamlReader) +
cli?.config,
obfuscate = cli?.obfuscate ?: false
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class AndroidRunCommand : CommonRunCommand(), Runnable {

val config = AndroidArgs.load(Paths.get(configPath), cli = this).validate()
runBlocking {
if (dumpShards) dumpShards(args = config, obfuscatedOutput = obfuscate)
if (dumpShards) dumpShards(args = config)
else newTestRun(config)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class IosRunCommand : CommonRunCommand(), Runnable {
val config = IosArgs.load(Paths.get(configPath), cli = this).validate()

if (dumpShards) {
dumpShards(args = config, obfuscatedOutput = obfuscate)
dumpShards(args = config)
} else runBlocking {
newTestRun(config)
}
Expand Down
8 changes: 3 additions & 5 deletions test_runner/src/main/kotlin/ftl/run/DumpShards.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import java.nio.file.Paths
suspend fun dumpShards(
args: AndroidArgs,
shardFilePath: String = ANDROID_SHARD_FILE,
obfuscatedOutput: Boolean = false
) {
if (!args.isInstrumentationTest) throw FlankConfigurationError(
"Cannot dump shards for non instrumentation test, ensure test apk has been set."
Expand All @@ -25,24 +24,23 @@ suspend fun dumpShards(
shardFilePath = shardFilePath,
shards = shards,
size = shards.size,
obfuscatedOutput = obfuscatedOutput
obfuscatedOutput = args.obfuscateDumpShards
)
}

fun dumpShards(
args: IosArgs,
shardFilePath: String = IOS_SHARD_FILE,
obfuscatedOutput: Boolean = false
) {
saveShardChunks(
shardFilePath = shardFilePath,
shards = args.testShardChunks.testCases,
size = args.testShardChunks.size,
obfuscatedOutput = obfuscatedOutput
obfuscatedOutput = args.obfuscateDumpShards
)
}

private fun saveShardChunks(
fun saveShardChunks(
shardFilePath: String,
shards: Any,
size: Int,
Expand Down
23 changes: 21 additions & 2 deletions test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ package ftl.run.platform
import com.google.api.services.testing.Testing
import com.google.api.services.testing.model.TestMatrix
import ftl.args.AndroidArgs
import ftl.args.isInstrumentationTest
import ftl.gc.GcAndroidDevice
import ftl.gc.GcAndroidTestMatrix
import ftl.gc.GcStorage
import ftl.gc.GcToolResults
import ftl.http.executeWithRetry
import ftl.run.ANDROID_SHARD_FILE
import ftl.run.model.InstrumentationTestContext
import ftl.run.model.TestResult
import ftl.run.platform.android.createAndroidTestConfig
Expand All @@ -18,6 +21,10 @@ import ftl.run.platform.common.afterRunTests
import ftl.run.platform.common.beforeRunMessage
import ftl.run.platform.common.beforeRunTests
import ftl.run.exception.FlankGeneralError
import ftl.run.model.AndroidMatrixTestShards
import ftl.run.model.AndroidTestContext
import ftl.run.platform.android.asMatrixTestShards
import ftl.run.saveShardChunks
import ftl.shard.Chunk
import ftl.shard.testCases
import kotlinx.coroutines.Deferred
Expand All @@ -40,8 +47,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
val otherGcsFiles = args.uploadOtherFiles(runGcsPath)
val additionalApks = args.uploadAdditionalApks(runGcsPath)

args.createAndroidTestContexts()
.upload(args.resultsBucket, runGcsPath)
args.createAndroidTestContexts().dumpShards(args).upload(args.resultsBucket, runGcsPath)
.forEachIndexed { index, context ->
if (context is InstrumentationTestContext) {
ignoredTestsShardChunks += context.ignoredTestCases
Expand All @@ -64,13 +70,26 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
if (testMatrices.isEmpty()) throw FlankGeneralError("There are no tests to run.")

println(beforeRunMessage(args, allTestShardChunks))

TestResult(
matrixMap = afterRunTests(testMatrices.awaitAll(), runGcsPath, stopwatch, args),
shardChunks = allTestShardChunks.testCases,
ignoredTests = ignoredTestsShardChunks.flatten()
)
}

private fun List<AndroidTestContext>.dumpShards(config: AndroidArgs) = takeIf { config.isInstrumentationTest }?.apply {
filterIsInstance<InstrumentationTestContext>().asMatrixTestShards().saveShards(config.obfuscateDumpShards)
if (config.disableResultsUpload.not()) GcStorage.upload(ANDROID_SHARD_FILE, config.resultsBucket, config.resultsDir)
} ?: this

private fun AndroidMatrixTestShards.saveShards(obfuscateOutput: Boolean) = saveShardChunks(
shardFilePath = ANDROID_SHARD_FILE,
shards = this,
size = size,
obfuscatedOutput = obfuscateOutput
)

private suspend fun executeAndroidTestMatrix(
runCount: Int,
createTestMatrix: () -> Testing.Projects.TestMatrices.Create
Expand Down
6 changes: 6 additions & 0 deletions test_runner/src/main/kotlin/ftl/run/platform/RunIosTests.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import ftl.gc.GcStorage
import ftl.gc.GcToolResults
import ftl.http.executeWithRetry
import ftl.ios.Xctestrun
import ftl.run.IOS_SHARD_FILE
import ftl.run.dumpShards
import ftl.run.model.TestResult
import ftl.run.platform.common.afterRunTests
import ftl.run.platform.common.beforeRunMessage
Expand Down Expand Up @@ -36,6 +38,10 @@ internal suspend fun runIosTests(iosArgs: IosArgs): TestResult = coroutineScope
val shardCounter = ShardCounter()
val history = GcToolResults.createToolResultsHistory(iosArgs)

dumpShards(iosArgs)
if (iosArgs.disableResultsUpload.not())
GcStorage.upload(IOS_SHARD_FILE, iosArgs.resultsBucket, iosArgs.resultsDir)

// Upload only after parsing shards to detect missing methods early.
val xcTestGcsPath = getXcTestGcPath(iosArgs, runGcsPath)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ suspend fun AndroidArgs.getAndroidMatrixShards(): AndroidMatrixTestShards = this
.filterIsInstance<InstrumentationTestContext>()
.asMatrixTestShards()

private fun List<InstrumentationTestContext>.asMatrixTestShards(): AndroidMatrixTestShards =
fun List<InstrumentationTestContext>.asMatrixTestShards(): AndroidMatrixTestShards =
map { testApks ->
AndroidTestShards(
app = testApks.app.local,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
package ftl.cli.firebase.test.android

import com.google.common.truth.Truth.assertThat
import ftl.args.AndroidArgs
import ftl.args.yml.AppTestPair
import ftl.config.Device
import ftl.config.FtlConstants
import ftl.gc.GcStorage
import ftl.run.ANDROID_SHARD_FILE
import ftl.run.exception.FlankConfigurationError
import ftl.run.platform.android.createAndroidTestContexts
import ftl.run.saveShardChunks
import ftl.test.util.FlankTestRunner
import io.mockk.coVerify
import io.mockk.mockkObject
import io.mockk.mockkStatic
import io.mockk.verify
import org.junit.Assert.assertEquals
import org.junit.Rule
import org.junit.Test
Expand Down Expand Up @@ -523,4 +532,37 @@ class AndroidRunCommandTest {
)
cmd.run()
}

@Test
fun `should dump shards on android test run`() {
mockkStatic("ftl.run.DumpShardsKt")
val runCmd = AndroidRunCommand()
runCmd.configPath = "./src/test/kotlin/ftl/fixtures/simple-android-flank.yml"
runCmd.run()
verify { saveShardChunks(any(), any(), any(), any()) }
}

@Test
fun `should dump shards on android test run and not upload when disable-upload-results set`() {
mockkStatic("ftl.run.DumpShardsKt")
mockkObject(GcStorage) {
val runCmd = AndroidRunCommand()
runCmd.configPath = "./src/test/kotlin/ftl/fixtures/simple-android-flank.yml"
CommandLine(runCmd).parseArgs("--disable-results-upload")
runCmd.run()
verify { saveShardChunks(any(), any(), any(), any()) }
verify(inverse = true) { GcStorage.upload(ANDROID_SHARD_FILE, any(), any()) }
}
}

@Test
fun `should calculate shards only one time on newRun`() {
mockkStatic("ftl.run.DumpShardsKt", "ftl.run.platform.android.CreateAndroidTestContextKt")
mockkObject(GcStorage) {
val runCmd = AndroidRunCommand()
runCmd.configPath = "./src/test/kotlin/ftl/fixtures/simple-android-flank.yml"
runCmd.run()
coVerify(exactly = 1) { any<AndroidArgs>().createAndroidTestContexts() }
}
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
package ftl.cli.firebase.test.ios

import com.google.common.truth.Truth.assertThat
import ftl.args.IosArgs
import ftl.config.Device
import ftl.config.FtlConstants
import ftl.config.FtlConstants.isWindows
import ftl.gc.GcStorage
import ftl.run.IOS_SHARD_FILE
import ftl.run.dumpShards
import ftl.test.util.FlankTestRunner
import io.mockk.mockkObject
import io.mockk.mockkStatic
import io.mockk.verify
import org.junit.Assume.assumeFalse
import org.junit.Rule
import org.junit.Test
Expand Down Expand Up @@ -337,4 +344,26 @@ class IosRunCommandTest {

assertThat(cmd.config.common.flank.useAverageTestTimeForNewTests).isTrue()
}

@Test
fun `should dump shards on ios test run`() {
mockkStatic("ftl.run.DumpShardsKt")
val runCmd = IosRunCommand()
runCmd.configPath = "./src/test/kotlin/ftl/fixtures/simple-ios-flank.yml"
runCmd.run()
verify { dumpShards(any<IosArgs>(), any()) }
}

@Test
fun `should dump shards on ios test run and not upload when disable-upload-results set`() {
mockkStatic("ftl.run.DumpShardsKt")
mockkObject(GcStorage) {
val runCmd = IosRunCommand()
runCmd.configPath = "./src/test/kotlin/ftl/fixtures/simple-ios-flank.yml"
CommandLine(runCmd).parseArgs("--disable-results-upload")
runCmd.run()
verify { dumpShards(any<IosArgs>(), any()) }
verify(inverse = true) { GcStorage.upload(IOS_SHARD_FILE, any(), any()) }
}
}
}
6 changes: 4 additions & 2 deletions test_runner/src/test/kotlin/ftl/run/DumpShardsKtTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package ftl.run
import com.google.common.truth.Truth.assertThat
import ftl.args.AndroidArgs
import ftl.args.IosArgs
import ftl.cli.firebase.test.android.AndroidRunCommand
import ftl.cli.firebase.test.ios.IosRunCommand
import ftl.config.FtlConstants
import ftl.doctor.assertEqualsIgnoreNewlineStyle
import ftl.test.util.FlankTestRunner
Expand Down Expand Up @@ -123,7 +125,7 @@ class DumpShardsKtTest {

// when
val actual = runBlocking {
dumpShards(AndroidArgs.load(mixedConfigYaml), TEST_SHARD_FILE, true)
dumpShards(AndroidArgs.load(mixedConfigYaml, AndroidRunCommand().apply { obfuscate = true }), TEST_SHARD_FILE)
File(TEST_SHARD_FILE).apply { deleteOnExit() }.readText()
}

Expand Down Expand Up @@ -183,7 +185,7 @@ class DumpShardsKtTest {
if (FtlConstants.isWindows) return // TODO Windows Linux subsytem does not contain all expected commands
// when
val actual = runBlocking {
dumpShards(IosArgs.load(ios2ConfigYaml), TEST_SHARD_FILE, true)
dumpShards(IosArgs.load(ios2ConfigYaml, IosRunCommand().apply { obfuscate = true }), TEST_SHARD_FILE)
File(TEST_SHARD_FILE).apply { deleteOnExit() }.readText()
}

Expand Down