Skip to content

Commit

Permalink
Add parameter to ignore fully skipped tests
Browse files Browse the repository at this point in the history 
We have some test artifacts that have no non-skipped tests. This CL adds a new
action parameter that allows ignoring them.

When all test cases in an APK are skipped, FTL marks its outcome as FAILED instead
of SKIPPED. In this CL, we fix it back to SKIPPED if requested.

Test: TestRunnerTest
  • Loading branch information
@yigit
yigit committed Jul 31, 2023
1 parent 293a21d commit 49e876d
Show file tree
Hide file tree
Showing 3 changed files with 162 additions and 6 deletions.
11 changes: 10 additions & 1 deletion AndroidXCI/cli/src/main/kotlin/dev/androidx/ci/cli/Main.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,14 @@ private class Cli : CliktCommand() {
envvar = "ANDROIDX_TEST_SUITE_TAGS"
)

val ignoreEmptyTestMatrices by option(
help = """
When set to true (default), the action will not fail when no tests
are run for a TestMatrix.
""".trimIndent(),
envvar = "ANDROIDX_IGNORE_EMPTY_TEST_MATRICES"
)

override fun run() {
logFile?.let(::configureLogger)
val repoParts = githubRepository.split("/")
Expand Down Expand Up @@ -177,7 +185,8 @@ private class Cli : CliktCommand() {
bucketName = gcpBucketName,
bucketPath = gcpBucketPath,
useTestConfigFiles = useTestConfigFiles?.toBoolean() ?: false,
testSuiteTags = testSuiteTags?.split(',')?.map { it.trim() } ?: emptyList()
testSuiteTags = testSuiteTags?.split(',')?.map { it.trim() } ?: emptyList(),
ignoreEmptyTestMatrices = ignoreEmptyTestMatrices?.toBoolean() ?: true,
)
testRunner.runTests()
}
Expand Down
57 changes: 53 additions & 4 deletions AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/testRunner/TestRunner.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class TestRunner internal constructor(
/**
* An optional filter to pick which build artifacts should be downloaded.
*/
private val githubArtifactFilter: ((ArtifactsResponse.Artifact) -> Boolean) = { true },
private val githubArtifactFilter: (ArtifactsResponse.Artifact) -> Boolean = { true },
/**
* The directory where results will be saved locally
*/
Expand All @@ -73,6 +73,11 @@ class TestRunner internal constructor(
* The actual class that will decide which tests to run out of the artifacts
*/
testSchedulerFactory: TestScheduler.Factory,
/**
* When set to true, TestMatrix failures due to empty test suites (e.g. all ignored) will not
* fail the test run.
*/
private val ignoreEmptyTestMatrices: Boolean = true,
) {
private val logger = logger()
private val testMatrixStore = TestMatrixStore(
Expand Down Expand Up @@ -100,6 +105,10 @@ class TestRunner internal constructor(
devicePicker = devicePicker
)

private val testExecutionStore = TestExecutionStore(
toolsResultApi = toolsResultApi
)

/**
* Runs all the test. This never throws, instead, returns an error result if something goes
* wrong.
Expand All @@ -117,10 +126,26 @@ class TestRunner internal constructor(
logger.info { "started all tests for $testMatrices" }
}
logger.info("will wait for test results")
testLabController.collectTestResults(
val collectResult = testLabController.collectTestResults(
matrices = allTestMatrices,
pollIntervalMs = TimeUnit.SECONDS.toMillis(10)
)
if (ignoreEmptyTestMatrices && collectResult is TestResult.CompleteRun) {
// when we skip tests, firebase marks it as failed instead of skipped. we patch fix it here if requested
val updatedTestMatrices = collectResult.matrices.map { testMatrix ->
if (testMatrix.outcomeSummary == TestMatrix.OutcomeSummary.FAILURE &&
testMatrix.areAllTestsSkipped()
) {
// change summary to SKIPPED instead.
testMatrix.copy(outcomeSummary = TestMatrix.OutcomeSummary.SKIPPED)
} else {
testMatrix
}
}
TestResult.CompleteRun(updatedTestMatrices)
} else {
collectResult
}
} catch (th: Throwable) {
logger.error("exception in test run", th)
TestResult.IncompleteRun(th.stackTraceToString())
Expand All @@ -144,6 +169,28 @@ class TestRunner internal constructor(
return result
}

/**
* Returns `true` if all tests in the TestMatrix are skipped.
*
* Firebase marks a TestMatrix as failed if all tests in it are skipped. It is WAI because not running any tests
* is likely an error. In certain cases (e.g. AndroidX) this is OK hence requires special handling.
*
* see: [ignoreEmptyTestMatrices]
*/
private suspend fun TestMatrix.areAllTestsSkipped(): Boolean {
if (outcomeSummary == null) {
// test is not complete yet
return false
}
val steps = testExecutionStore.getTestExecutionSteps(this)
val overviews = steps.flatMap { step ->
step.testExecutionStep?.testSuiteOverviews ?: emptyList()
}
return overviews.isNotEmpty() && overviews.all { overview ->
overview.totalCount != null && overview.totalCount == overview.skippedCount
}
}

companion object {
internal const val RESULT_JSON_FILE_NAME = "result.json"
fun create(
Expand All @@ -160,7 +207,8 @@ class TestRunner internal constructor(
devicePicker: DevicePicker? = null,
artifactNameFilter: (String) -> Boolean = { true },
useTestConfigFiles: Boolean,
testSuiteTags: List<String>
testSuiteTags: List<String>,
ignoreEmptyTestMatrices: Boolean,
): TestRunner {
val credentials = ServiceAccountCredentials.fromStream(
googleCloudCredentials.byteInputStream(Charsets.UTF_8)
Expand Down Expand Up @@ -210,7 +258,8 @@ class TestRunner internal constructor(
targetRunId = targetRunId,
hostRunId = hostRunId,
devicePicker = devicePicker,
testSchedulerFactory = TestScheduler.createFactory(useTestConfigFiles, testSuiteTags)
testSchedulerFactory = TestScheduler.createFactory(useTestConfigFiles, testSuiteTags),
ignoreEmptyTestMatrices = ignoreEmptyTestMatrices,
)
}

Expand Down
100 changes: 99 additions & 1 deletion AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/TestRunnerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,16 @@ package dev.androidx.ci.testRunner

import com.google.common.truth.Truth.assertThat
import dev.androidx.ci.fake.FakeBackend
import dev.androidx.ci.generated.ftl.GoogleCloudStorage
import dev.androidx.ci.generated.ftl.ResultStorage
import dev.androidx.ci.generated.ftl.TestMatrix
import dev.androidx.ci.generated.ftl.TestMatrix.OutcomeSummary.FAILURE
import dev.androidx.ci.generated.ftl.TestMatrix.OutcomeSummary.SKIPPED
import dev.androidx.ci.generated.ftl.TestMatrix.OutcomeSummary.SUCCESS
import dev.androidx.ci.generated.ftl.ToolResultsExecution
import dev.androidx.ci.generated.testResults.Step
import dev.androidx.ci.generated.testResults.TestExecutionStep
import dev.androidx.ci.generated.testResults.TestSuiteOverview
import dev.androidx.ci.github.dto.ArtifactsResponse
import dev.androidx.ci.github.dto.CommitInfo
import dev.androidx.ci.testRunner.TestRunner.Companion.RESULT_JSON_FILE_NAME
Expand Down Expand Up @@ -55,12 +61,12 @@ internal class TestRunnerTest(
TestRunner(
googleCloudApi = fakeBackend.fakeGoogleCloudApi,
githubApi = fakeBackend.fakeGithubApi,
datastoreApi = fakeBackend.datastoreApi,
firebaseTestLabApi = fakeBackend.fakeFirebaseTestLabApi,
toolsResultApi = fakeBackend.fakeToolsResultApi,
projectId = PROJECT_ID,
targetRunId = TARGET_RUN_ID,
hostRunId = HOST_RUN_ID,
datastoreApi = fakeBackend.datastoreApi,
outputFolder = outputFolder,
testSchedulerFactory = TestScheduler.createFactory(
useTestConfigFiles = useTestConfigFiles,
Expand Down Expand Up @@ -247,6 +253,98 @@ internal class TestRunnerTest(
assertOutputFolderContents(result)
}

@Test
fun failedDueToSkippedTests_allSkipped() = failedTestDueToSkippedTestCases(
totalCount = 3,
skippedCount = 3
) { testResult ->
assertThat(testResult.allTestsPassed).isFalse()
assertThat(testResult.hasFailedTest).isFalse()
}

@Test
fun failedDueToSkippedTests_someSkipped() = failedTestDueToSkippedTestCases(
totalCount = 3,
skippedCount = 1
) { testResult ->
assertThat(testResult.allTestsPassed).isFalse()
assertThat(testResult.hasFailedTest).isTrue()
}

@Test
fun failedDueToSkippedTests_missingOverview() = failedTestDueToSkippedTestCases(
totalCount = null,
skippedCount = null
) { testResult ->
assertThat(testResult.allTestsPassed).isFalse()
assertThat(testResult.hasFailedTest).isTrue()
}

private fun failedTestDueToSkippedTestCases(
totalCount: Int?,
skippedCount: Int?,
assertion: (TestResult) -> Unit
) = testScope.runTest {
val artifact1 = fakeBackend.createArchive(
testPairs = listOf(
FakeBackend.TestPair(
testFilePrefix = "bio",
testApk = "biometric-integration-tests-testapp_testapp-debug-androidTest.apk",
appApk = "biometric-integration-tests-testapp_testapp-debug.apk",
)
),
contentNames = listOf(
"biometric-integration-tests-testapp_testapp-debug-androidTest.apk",
"biometric-integration-tests-testapp_testapp-debug.apk",
"biometric-integration-tests-testapp_testapp-release.apk"
)
)
createRuns(
listOf(
artifact1
)
)
val runTests = async {
testRunner.runTests()
}
runCurrent()
assertThat(runTests.isActive).isTrue()
val testMatrices = fakeBackend.fakeFirebaseTestLabApi.getTestMatrices()
assertThat(testMatrices).hasSize(1)
fakeBackend.finishAllTests(FAILURE)
fakeBackend.fakeFirebaseTestLabApi.setTestMatrix(
fakeBackend.fakeFirebaseTestLabApi.getTestMatrices().single().copy(
resultStorage = ResultStorage(
googleCloudStorage = GoogleCloudStorage("gs://empty"),
toolResultsExecution = ToolResultsExecution(
executionId = "e1",
historyId = "h1",
projectId = PROJECT_ID
)
)
)
)
fakeBackend.fakeToolsResultApi.addStep(
projectId = PROJECT_ID,
historyId = "h1",
executionId = "e1",
step = Step(
stepId = "step1",
testExecutionStep = TestExecutionStep(
testSuiteOverviews = listOf(
TestSuiteOverview(
totalCount = totalCount,
skippedCount = skippedCount
)
)
)
)
)
advanceUntilIdle()
val result = runTests.await()
assertion(result)
}

@Test
fun multipleTestsOnMultipleArtifacts_oneFailure() = testScope.runTest {
val artifact1 = fakeBackend.createArchive(
Expand Down

0 comments on commit 49e876d

Please sign in to comment.