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

Allow runtime test discovery when sharding is disabled by not setting test-targets. This unblocks cucumber testing #822

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: 1 addition & 2 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
- [#812](https://github.com/Flank/flank/issues/812) Convert bitrise macOS workflow to github action. ([piotradamczyk5](https://github.com/piotradamczyk5))
- [#799](https://github.com/Flank/flank/pull/799) Refactor Shared object by splitting it into smaller functions. ([piotradamczyk5](https://github.com/piotradamczyk5))
- [#798](https://github.com/Flank/flank/pull/798) Remove failure nodes from tests that passed on retry so that Jenkins JUnit plugin marks them as successful. ([adamfilipow92](https://github.com/adamfilipow92))
-
-
- [#822](https://github.com/Flank/flank/pull/822) Allow runtime test discovery when sharding is disabled by not setting test-targets. This unblocks cucumber testing. ([adamfilipow92](https://github.com/adamfilipow92))

## v20.05.2

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,29 +19,37 @@ internal fun createAndroidInstrumentationTest(
}.setupTestTargets(
disableSharding = config.disableSharding,
testShards = config.testShards,
numUniformShards = config.numUniformShards
numUniformShards = config.numUniformShards,
keepTestTargetsEmpty = config.keepTestTargetsEmpty
)

internal fun AndroidInstrumentationTest.setupTestTargets(
disableSharding: Boolean,
testShards: ShardChunks,
numUniformShards: Int?
numUniformShards: Int?,
keepTestTargetsEmpty: Boolean
) = apply {
if (disableSharding) {
testTargets = testShards.flatten()
} else {
shardingOption = ShardingOption().apply {
if (numUniformShards != null) {
testTargets = testShards.flatten()
val safeNumUniformShards = if (testTargets.size > numUniformShards) numUniformShards else {
println("WARNING: num-uniform-shards ($numUniformShards) is higher than number of test cases (${testTargets.size}) from ${testApk.gcsPath}")
testTargets.size
when {
keepTestTargetsEmpty -> {
testTargets = emptyList()
}
disableSharding -> {
testTargets = testShards.flatten()
}
else -> {
shardingOption = ShardingOption().apply {
if (numUniformShards != null) {
testTargets = testShards.flatten()
val safeNumUniformShards = if (testTargets.size > numUniformShards) numUniformShards else {
println("WARNING: num-uniform-shards ($numUniformShards) is higher than number of test cases (${testTargets.size}) from ${testApk.gcsPath}")
testTargets.size
}
uniformSharding = UniformSharding().setNumShards(safeNumUniformShards)
} else {
manualSharding = ManualSharding().setTestTargetsForShard(testShards.map {
TestTargetsForShard().setTestTargets(it)
})
}
uniformSharding = UniformSharding().setNumShards(safeNumUniformShards)
} else {
manualSharding = ManualSharding().setTestTargetsForShard(testShards.map {
TestTargetsForShard().setTestTargets(it)
})
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
val androidTestConfig = args.createAndroidTestConfig(
uploadedApks = uploadedApks,
testShards = testShards,
runGcsPath = runGcsPath
runGcsPath = runGcsPath,
keepTestTargetsEmpty = args.disableSharding && args.testTargets.isEmpty()
)

testMatrices += executeAndroidTestMatrix(runCount = args.repeatTests) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ sealed class AndroidTestConfig {
// sharding
val disableSharding: Boolean,
val testShards: ShardChunks,
val numUniformShards: Int?
val numUniformShards: Int?,
val keepTestTargetsEmpty: Boolean
) : AndroidTestConfig()

data class Robo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@ import ftl.util.FlankFatalError
internal fun AndroidArgs.createAndroidTestConfig(
uploadedApks: UploadedApks,
testShards: ShardChunks? = null,
runGcsPath: String? = null
runGcsPath: String? = null,
keepTestTargetsEmpty: Boolean = false
): AndroidTestConfig = when {
isInstrumentationTest -> createInstrumentationConfig(
uploadedApks = uploadedApks,
keepTestTargetsEmpty = keepTestTargetsEmpty,
testShards = testShards ?: throw FlankFatalError("Arg testShards is required for instrumentation test.")
)
isRoboTest -> createRoboConfig(
isRoboTest
-> createRoboConfig(
uploadedApks = uploadedApks,
runGcsPath = runGcsPath ?: throw FlankFatalError("Arg runGcsPath is required for robo test.")
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import ftl.args.yml.UploadedApks

internal fun AndroidArgs.createInstrumentationConfig(
uploadedApks: UploadedApks,
keepTestTargetsEmpty: Boolean,
testShards: ShardChunks
) = AndroidTestConfig.Instrumentation(
appApkGcsPath = uploadedApks.app,
Expand All @@ -14,5 +15,6 @@ internal fun AndroidArgs.createInstrumentationConfig(
orchestratorOption = "USE_ORCHESTRATOR".takeIf { useOrchestrator },
disableSharding = disableSharding,
numUniformShards = numUniformShards,
testShards = testShards
testShards = testShards,
keepTestTargetsEmpty = keepTestTargetsEmpty
)
2 changes: 1 addition & 1 deletion test_runner/src/test/kotlin/Debug.kt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fun main() {
val projectId = System.getenv("GOOGLE_CLOUD_PROJECT")
?: "YOUR PROJECT ID"
val quantity = "single"
val type = "errorFlaky"
val type = "success-disable-sharding"

// Bugsnag keeps the process alive so we must call exitProcess
// https://github.com/bugsnag/bugsnag-java/issues/151
Expand Down
39 changes: 39 additions & 0 deletions test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package ftl.args

import com.google.api.services.testing.model.TestSpecification
import com.google.common.truth.Truth.assertThat
import ftl.args.yml.AppTestPair
import ftl.args.yml.UploadedApks
import ftl.cli.firebase.test.android.AndroidRunCommand
import ftl.config.Device
import ftl.config.FlankRoboDirective
import ftl.config.FtlConstants.defaultAndroidModel
import ftl.config.FtlConstants.defaultAndroidVersion
import ftl.gc.android.setupAndroidTest
import ftl.run.platform.android.createAndroidTestConfig
import ftl.run.platform.runAndroidTests
import ftl.run.platform.android.getAndroidShardChunks
import ftl.run.status.OutputStyle
Expand All @@ -24,6 +28,7 @@ import org.junit.After
import org.junit.Assert
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.junit.rules.ExpectedException
Expand Down Expand Up @@ -1362,6 +1367,40 @@ AndroidArgs
)
}
}

@Test
fun `should return AndroidTestConfig without testTargets`() {
val yaml = """
gcloud:
app: $appApk
test: $testApk
use-orchestrator: false
flank:
disable-sharding: true
""".trimIndent()
val args = AndroidArgs.load(yaml)
val androidTestConfig = args.createAndroidTestConfig(UploadedApks("", ""), listOf(listOf("test")), null, true)
val testSpecification = TestSpecification().setupAndroidTest(androidTestConfig)
assertTrue(testSpecification.androidInstrumentationTest.testTargets.isEmpty())
}

@Test
fun `should return AndroidTestConfig with testTargets`() {
val yaml = """
gcloud:
app: $appApk
test: $testApk
use-orchestrator: false
test-targets:
- EarlGreyExampleSwiftTests/testWith.*${'$'}
flank:
disable-sharding: true
""".trimIndent()
val args = AndroidArgs.load(yaml)
val androidTestConfig = args.createAndroidTestConfig(UploadedApks("", ""), listOf(listOf("test"), listOf("test")))
val testSpecification = TestSpecification().setupAndroidTest(androidTestConfig)
assertTrue(testSpecification.androidInstrumentationTest.testTargets.isNotEmpty())
}
}

private fun AndroidArgs.Companion.load(yamlData: String, cli: AndroidRunCommand? = null): AndroidArgs = load(StringReader(yamlData), cli)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
gcloud:
app: ./src/test/kotlin/ftl/fixtures/tmp/apk/app-debug.apk
test: ./src/test/kotlin/ftl/fixtures/tmp/apk/app-single-success-debug-androidTest.apk
use-orchestrator: false

flank:
disable-sharding: true
9 changes: 6 additions & 3 deletions test_runner/src/test/kotlin/ftl/gc/GcAndroidTestMatrixTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ class GcAndroidTestMatrixTest {
orchestratorOption = null,
numUniformShards = null,
disableSharding = false,
testRunnerClass = ""
testRunnerClass = "",
keepTestTargetsEmpty = false
),
runGcsPath = "",
otherFiles = emptyMap(),
Expand All @@ -53,7 +54,8 @@ class GcAndroidTestMatrixTest {
orchestratorOption = null,
numUniformShards = null,
disableSharding = false,
testRunnerClass = ""
testRunnerClass = "",
keepTestTargetsEmpty = false
),
runGcsPath = "",
otherFiles = emptyMap(),
Expand All @@ -80,7 +82,8 @@ class GcAndroidTestMatrixTest {
orchestratorOption = null,
numUniformShards = null,
disableSharding = false,
testRunnerClass = ""
testRunnerClass = "",
keepTestTargetsEmpty = false
),
runGcsPath = "",
otherFiles = emptyMap(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ class UtilsKtTest {
.setupTestTargets(
disableSharding = true,
testShards = testShards,
numUniformShards = null
numUniformShards = null,
keepTestTargetsEmpty = false
)
.testTargets

Expand All @@ -39,7 +40,8 @@ class UtilsKtTest {
.setupTestTargets(
disableSharding = false,
testShards = testShards,
numUniformShards = 50
numUniformShards = 50,
keepTestTargetsEmpty = false
)

// then
Expand All @@ -57,7 +59,8 @@ class UtilsKtTest {
.setupTestTargets(
disableSharding = false,
testShards = shardChunks,
numUniformShards = null
numUniformShards = null,
keepTestTargetsEmpty = false
)
.shardingOption
.manualSharding
Expand Down