Skip to content

Commit ad31f58

Browse files
yigityigit
authored
Add parameter to ignore fully skipped tests (#66)
* Add parameter to ignore fully skipped tests 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 * update action yml
1 parent 293a21d commit ad31f58

File tree

4 files changed

+165
-6
lines changed

4 files changed

+165
-6
lines changed

‎AndroidXCI/cli/src/main/kotlin/dev/androidx/ci/cli/Main.kt‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,14 @@ private class Cli : CliktCommand() {
148148
envvar = "ANDROIDX_TEST_SUITE_TAGS"
149149
)
150150

151+
val ignoreEmptyTestMatrices by option(
152+
help = """
153+
When set to true (default), the action will not fail when no tests
154+
are run for a TestMatrix.
155+
""".trimIndent(),
156+
envvar = "ANDROIDX_IGNORE_EMPTY_TEST_MATRICES"
157+
)
158+
151159
override fun run() {
152160
logFile?.let(::configureLogger)
153161
val repoParts = githubRepository.split("/")
@@ -177,7 +185,8 @@ private class Cli : CliktCommand() {
177185
bucketName = gcpBucketName,
178186
bucketPath = gcpBucketPath,
179187
useTestConfigFiles = useTestConfigFiles?.toBoolean() ?: false,
180-
testSuiteTags = testSuiteTags?.split(',')?.map { it.trim() } ?: emptyList()
188+
testSuiteTags = testSuiteTags?.split(',')?.map { it.trim() } ?: emptyList(),
189+
ignoreEmptyTestMatrices = ignoreEmptyTestMatrices?.toBoolean() ?: true,
181190
)
182191
testRunner.runTests()
183192
}

‎AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/testRunner/TestRunner.kt‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class TestRunner internal constructor(
6060
/**
6161
* An optional filter to pick which build artifacts should be downloaded.
6262
*/
63-
private val githubArtifactFilter: ((ArtifactsResponse.Artifact) -> Boolean) = { true },
63+
private val githubArtifactFilter: (ArtifactsResponse.Artifact) -> Boolean = { true },
6464
/**
6565
* The directory where results will be saved locally
6666
*/
@@ -73,6 +73,11 @@ class TestRunner internal constructor(
7373
* The actual class that will decide which tests to run out of the artifacts
7474
*/
7575
testSchedulerFactory: TestScheduler.Factory,
76+
/**
77+
* When set to true, TestMatrix failures due to empty test suites (e.g. all ignored) will not
78+
* fail the test run.
79+
*/
80+
private val ignoreEmptyTestMatrices: Boolean = true,
7681
) {
7782
private val logger = logger()
7883
private val testMatrixStore = TestMatrixStore(
@@ -100,6 +105,10 @@ class TestRunner internal constructor(
100105
devicePicker = devicePicker
101106
)
102107

108+
private val testExecutionStore = TestExecutionStore(
109+
toolsResultApi = toolsResultApi
110+
)
111+
103112
/**
104113
* Runs all the test. This never throws, instead, returns an error result if something goes
105114
* wrong.
@@ -117,10 +126,26 @@ class TestRunner internal constructor(
117126
logger.info { "started all tests for $testMatrices" }
118127
}
119128
logger.info("will wait for test results")
120-
testLabController.collectTestResults(
129+
val collectResult = testLabController.collectTestResults(
121130
matrices = allTestMatrices,
122131
pollIntervalMs = TimeUnit.SECONDS.toMillis(10)
123132
)
133+
if (ignoreEmptyTestMatrices && collectResult is TestResult.CompleteRun) {
134+
// when we skip tests, firebase marks it as failed instead of skipped. we patch fix it here if requested
135+
val updatedTestMatrices = collectResult.matrices.map { testMatrix ->
136+
if (testMatrix.outcomeSummary == TestMatrix.OutcomeSummary.FAILURE &&
137+
testMatrix.areAllTestsSkipped()
138+
) {
139+
// change summary to SKIPPED instead.
140+
testMatrix.copy(outcomeSummary = TestMatrix.OutcomeSummary.SKIPPED)
141+
} else {
142+
testMatrix
143+
}
144+
}
145+
TestResult.CompleteRun(updatedTestMatrices)
146+
} else {
147+
collectResult
148+
}
124149
} catch (th: Throwable) {
125150
logger.error("exception in test run", th)
126151
TestResult.IncompleteRun(th.stackTraceToString())
@@ -144,6 +169,28 @@ class TestRunner internal constructor(
144169
return result
145170
}
146171

172+
/**
173+
* Returns `true` if all tests in the TestMatrix are skipped.
174+
*
175+
* Firebase marks a TestMatrix as failed if all tests in it are skipped. It is WAI because not running any tests
176+
* is likely an error. In certain cases (e.g. AndroidX) this is OK hence requires special handling.
177+
*
178+
* see: [ignoreEmptyTestMatrices]
179+
*/
180+
private suspend fun TestMatrix.areAllTestsSkipped(): Boolean {
181+
if (outcomeSummary == null) {
182+
// test is not complete yet
183+
return false
184+
}
185+
val steps = testExecutionStore.getTestExecutionSteps(this)
186+
val overviews = steps.flatMap { step ->
187+
step.testExecutionStep?.testSuiteOverviews ?: emptyList()
188+
}
189+
return overviews.isNotEmpty() && overviews.all { overview ->
190+
overview.totalCount != null && overview.totalCount == overview.skippedCount
191+
}
192+
}
193+
147194
companion object {
148195
internal const val RESULT_JSON_FILE_NAME = "result.json"
149196
fun create(
@@ -160,7 +207,8 @@ class TestRunner internal constructor(
160207
devicePicker: DevicePicker? = null,
161208
artifactNameFilter: (String) -> Boolean = { true },
162209
useTestConfigFiles: Boolean,
163-
testSuiteTags: List<String>
210+
testSuiteTags: List<String>,
211+
ignoreEmptyTestMatrices: Boolean,
164212
): TestRunner {
165213
val credentials = ServiceAccountCredentials.fromStream(
166214
googleCloudCredentials.byteInputStream(Charsets.UTF_8)
@@ -210,7 +258,8 @@ class TestRunner internal constructor(
210258
targetRunId = targetRunId,
211259
hostRunId = hostRunId,
212260
devicePicker = devicePicker,
213-
testSchedulerFactory = TestScheduler.createFactory(useTestConfigFiles, testSuiteTags)
261+
testSchedulerFactory = TestScheduler.createFactory(useTestConfigFiles, testSuiteTags),
262+
ignoreEmptyTestMatrices = ignoreEmptyTestMatrices,
214263
)
215264
}
216265

‎AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/TestRunnerTest.kt‎

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,16 @@ package dev.androidx.ci.testRunner
1818

1919
import com.google.common.truth.Truth.assertThat
2020
import dev.androidx.ci.fake.FakeBackend
21+
import dev.androidx.ci.generated.ftl.GoogleCloudStorage
22+
import dev.androidx.ci.generated.ftl.ResultStorage
2123
import dev.androidx.ci.generated.ftl.TestMatrix
2224
import dev.androidx.ci.generated.ftl.TestMatrix.OutcomeSummary.FAILURE
2325
import dev.androidx.ci.generated.ftl.TestMatrix.OutcomeSummary.SKIPPED
2426
import dev.androidx.ci.generated.ftl.TestMatrix.OutcomeSummary.SUCCESS
27+
import dev.androidx.ci.generated.ftl.ToolResultsExecution
28+
import dev.androidx.ci.generated.testResults.Step
29+
import dev.androidx.ci.generated.testResults.TestExecutionStep
30+
import dev.androidx.ci.generated.testResults.TestSuiteOverview
2531
import dev.androidx.ci.github.dto.ArtifactsResponse
2632
import dev.androidx.ci.github.dto.CommitInfo
2733
import dev.androidx.ci.testRunner.TestRunner.Companion.RESULT_JSON_FILE_NAME
@@ -55,12 +61,12 @@ internal class TestRunnerTest(
5561
TestRunner(
5662
googleCloudApi = fakeBackend.fakeGoogleCloudApi,
5763
githubApi = fakeBackend.fakeGithubApi,
64+
datastoreApi = fakeBackend.datastoreApi,
5865
firebaseTestLabApi = fakeBackend.fakeFirebaseTestLabApi,
5966
toolsResultApi = fakeBackend.fakeToolsResultApi,
6067
projectId = PROJECT_ID,
6168
targetRunId = TARGET_RUN_ID,
6269
hostRunId = HOST_RUN_ID,
63-
datastoreApi = fakeBackend.datastoreApi,
6470
outputFolder = outputFolder,
6571
testSchedulerFactory = TestScheduler.createFactory(
6672
useTestConfigFiles = useTestConfigFiles,
@@ -247,6 +253,98 @@ internal class TestRunnerTest(
247253
assertOutputFolderContents(result)
248254
}
249255

256+
@Test
257+
fun failedDueToSkippedTests_allSkipped() = failedTestDueToSkippedTestCases(
258+
totalCount = 3,
259+
skippedCount = 3
260+
) { testResult ->
261+
assertThat(testResult.allTestsPassed).isFalse()
262+
assertThat(testResult.hasFailedTest).isFalse()
263+
}
264+
265+
@Test
266+
fun failedDueToSkippedTests_someSkipped() = failedTestDueToSkippedTestCases(
267+
totalCount = 3,
268+
skippedCount = 1
269+
) { testResult ->
270+
assertThat(testResult.allTestsPassed).isFalse()
271+
assertThat(testResult.hasFailedTest).isTrue()
272+
}
273+
274+
@Test
275+
fun failedDueToSkippedTests_missingOverview() = failedTestDueToSkippedTestCases(
276+
totalCount = null,
277+
skippedCount = null
278+
) { testResult ->
279+
assertThat(testResult.allTestsPassed).isFalse()
280+
assertThat(testResult.hasFailedTest).isTrue()
281+
}
282+
283+
private fun failedTestDueToSkippedTestCases(
284+
totalCount: Int?,
285+
skippedCount: Int?,
286+
assertion: (TestResult) -> Unit
287+
) = testScope.runTest {
288+
val artifact1 = fakeBackend.createArchive(
289+
testPairs = listOf(
290+
FakeBackend.TestPair(
291+
testFilePrefix = "bio",
292+
testApk = "biometric-integration-tests-testapp_testapp-debug-androidTest.apk",
293+
appApk = "biometric-integration-tests-testapp_testapp-debug.apk",
294+
)
295+
),
296+
contentNames = listOf(
297+
"biometric-integration-tests-testapp_testapp-debug-androidTest.apk",
298+
"biometric-integration-tests-testapp_testapp-debug.apk",
299+
"biometric-integration-tests-testapp_testapp-release.apk"
300+
)
301+
)
302+
createRuns(
303+
listOf(
304+
artifact1
305+
)
306+
)
307+
val runTests = async {
308+
testRunner.runTests()
309+
}
310+
runCurrent()
311+
assertThat(runTests.isActive).isTrue()
312+
val testMatrices = fakeBackend.fakeFirebaseTestLabApi.getTestMatrices()
313+
assertThat(testMatrices).hasSize(1)
314+
fakeBackend.finishAllTests(FAILURE)
315+
fakeBackend.fakeFirebaseTestLabApi.setTestMatrix(
316+
fakeBackend.fakeFirebaseTestLabApi.getTestMatrices().single().copy(
317+
resultStorage = ResultStorage(
318+
googleCloudStorage = GoogleCloudStorage("gs://empty"),
319+
toolResultsExecution = ToolResultsExecution(
320+
executionId = "e1",
321+
historyId = "h1",
322+
projectId = PROJECT_ID
323+
)
324+
)
325+
)
326+
)
327+
fakeBackend.fakeToolsResultApi.addStep(
328+
projectId = PROJECT_ID,
329+
historyId = "h1",
330+
executionId = "e1",
331+
step = Step(
332+
stepId = "step1",
333+
testExecutionStep = TestExecutionStep(
334+
testSuiteOverviews = listOf(
335+
TestSuiteOverview(
336+
totalCount = totalCount,
337+
skippedCount = skippedCount
338+
)
339+
)
340+
)
341+
)
342+
)
343+
advanceUntilIdle()
344+
val result = runTests.await()
345+
assertion(result)
346+
}
347+
250348
@Test
251349
fun multipleTestsOnMultipleArtifacts_oneFailure() = testScope.runTest {
252350
val artifact1 = fakeBackend.createArchive(

‎action.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ inputs:
3838
test-suite-tags:
3939
description: 'A comma separated list of test suite tags to be run. Only used if use-test-config-files is true'
4040
required: false
41+
ignore-empty-test-matrices:
42+
description: 'When set to true (default), test matrices which have 0 non-skipped tests are considered SKIPPED instead of FAILED (FTL default).'
43+
required: false
4144

4245
runs:
4346
using: "composite"

0 commit comments

Comments
 (0)