Skip to content

Commit a1d0e94

Browse files
authored
Create testMatrix to rerun failed tests in FTL (#70)
* Disable caching test matrices for retries * Adding a method to schedule retry tests * add caching back in create test matrix method * update results gcs path * addressing feedback * updating docs * Add a parameterized method to create test matrix * updates * Add testmatrix id parameter to generate test run id method * separating the 2 methods for createId * addressing feedback * adding check for testTargets length * update error msg
1 parent 7386d66 commit a1d0e94

File tree

6 files changed

+342
-61
lines changed

6 files changed

+342
-61
lines changed

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ internal class FirebaseTestLabController(
111111
pullScreenshots: Boolean = false,
112112
cachedTestMatrixFilter: CachedTestMatrixFilter,
113113
testTargets: List<String>? = null,
114-
flakyTestAttempts: Int? = 2
114+
flakyTestAttempts: Int = 2
115115
): List<TestMatrix> {
116116
val devices = (devicePicker ?: defaultDevicePicker).pickDevices()
117117
logger.info {
@@ -134,6 +134,22 @@ internal class FirebaseTestLabController(
134134
}
135135
}
136136

137+
/**
138+
* Enqueues a request to create a [TestMatrix] to run the tests
139+
* specified in the [testTargets] list using the same configuration as [testMatrix]
140+
*/
141+
suspend fun scheduleTests(
142+
testMatrix: TestMatrix,
143+
testTargets: List<String>,
144+
cachedTestMatrixFilter: CachedTestMatrixFilter
145+
): TestMatrix {
146+
return testMatrixStore.getOrCreateTestMatrix(
147+
testMatrix = testMatrix,
148+
testTargets = testTargets,
149+
cachedTestMatrixFilter = cachedTestMatrixFilter
150+
)
151+
}
152+
137153
suspend fun collectTestResultsByTestMatrixIds(
138154
testMatrixIds: List<String>,
139155
pollIntervalMs: Long

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

Lines changed: 190 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ internal class TestMatrixStore(
4949
private val datastoreApi: DatastoreApi,
5050
private val firebaseTestLabApi: FirebaseTestLabApi,
5151
toolsResultApi: ToolsResultApi,
52-
private val resultsGcsPrefix: GcsPath,
52+
private val resultsGcsPrefix: GcsPath
5353
) {
5454
private val logger = logger()
5555
private val toolsResultStore = ToolsResultStore(
@@ -70,7 +70,7 @@ internal class TestMatrixStore(
7070
pullScreenshots: Boolean = false,
7171
cachedTestMatrixFilter: CachedTestMatrixFilter = { true },
7272
testTargets: List<String>? = null,
73-
flakyTestAttempts: Int? = 2
73+
flakyTestAttempts: Int = 2
7474
): TestMatrix {
7575

7676
val testRunId = TestRun.createId(
@@ -86,40 +86,80 @@ internal class TestMatrixStore(
8686
"test run id: $testRunId"
8787
}
8888

89-
getExistingTestMatrix(testRunId)?.let {
90-
logger.info("found existing test matrix: ${it.testMatrixId} with state: ${it.state}")
91-
val state = it.state
92-
// these states are ordered so anything above ERROR is not worth re-using
93-
if (state != null && state >= TestMatrix.State.ERROR) {
94-
logger.warn {
95-
"Skipping cache for ${it.testMatrixId} because its state is $state"
96-
}
97-
} else if (!cachedTestMatrixFilter(it)) {
98-
logger.info {
99-
"Not re-using cached matrix due to filter"
100-
}
101-
} else {
102-
return it
89+
val existingTestMatrix = getCachedTestMatrix(testRunId, cachedTestMatrixFilter)
90+
if (existingTestMatrix != null) {
91+
return existingTestMatrix
92+
}
93+
val newTestMatrix = createNewTestMatrix(
94+
testRunKey = testRunId,
95+
environmentMatrix = environmentMatrix,
96+
clientInfo = clientInfo,
97+
sharding = sharding,
98+
deviceSetup = deviceSetup,
99+
appApk = appApk,
100+
testApk = testApk,
101+
pullScreenshots = pullScreenshots,
102+
testTargets = testTargets,
103+
flakyTestAttempts = flakyTestAttempts
104+
)
105+
logger.info {
106+
"created test matrix: $newTestMatrix"
107+
}
108+
// save it to cache, we don't worry about races here much such that if another instance happens to be creating
109+
// the exact same test, one of them will win but that is OK since they'll each use their own test matrices and
110+
// followup calls will re-use the winner of this race.
111+
val testRun = TestRun(
112+
id = testRunId,
113+
testMatrixId = checkNotNull(newTestMatrix.testMatrixId) {
114+
"newly created test matrix should not have null id $newTestMatrix"
103115
}
116+
)
117+
datastoreApi.put(testRun.toEntity())
118+
logger.info {
119+
"saved test matrix info: $testRun"
104120
}
121+
return newTestMatrix
122+
}
123+
124+
/**
125+
* Creates a TestMatrix for the given configuration or returns an existing one if we've run the same tests
126+
* specified in [testTargets] with the same environment configuration.
127+
*/
128+
suspend fun getOrCreateTestMatrix(
129+
testMatrix: TestMatrix,
130+
cachedTestMatrixFilter: CachedTestMatrixFilter = { true },
131+
testTargets: List<String>,
132+
flakyTestAttempts: Int = 0
133+
): TestMatrix {
134+
checkNotNull(testMatrix.testMatrixId) {
135+
"Test matrix id for the base test matrix should not be null"
136+
}
137+
check(testTargets.isNotEmpty()) {
138+
"The test targets should not be empty"
139+
}
140+
val testRunId = TestRun.createId(
141+
datastoreApi = datastoreApi,
142+
environment = testMatrix.environmentMatrix,
143+
clientInfo = testMatrix.clientInfo,
144+
sharding = testMatrix.testSpecification.androidInstrumentationTest?.shardingOption,
145+
testSetup = testMatrix.testSpecification.testSetup,
146+
testTargets = testTargets,
147+
baseTestMatrixId = testMatrix.testMatrixId
148+
)
105149
logger.trace {
106-
"No test run history for $testRunId or cached TestMatrix is rejected, creating a new one."
150+
"test run id: $testRunId"
107151
}
108-
val newTestMatrix = firebaseTestLabApi.createTestMatrix(
109-
projectId = firebaseProjectId,
110-
requestId = UUID.randomUUID().toString(),
111-
testMatrix = createNewTestMatrix(
112-
testRunKey = testRunId,
113-
environmentMatrix = environmentMatrix,
114-
clientInfo = clientInfo,
115-
sharding = sharding,
116-
deviceSetup = deviceSetup,
117-
appApk = appApk,
118-
testApk = testApk,
119-
pullScreenshots = pullScreenshots,
120-
testTargets = testTargets,
121-
flakyTestAttempts = flakyTestAttempts
122-
)
152+
153+
val existingTestMatrix = getCachedTestMatrix(testRunId, cachedTestMatrixFilter)
154+
if (existingTestMatrix != null) {
155+
return existingTestMatrix
156+
}
157+
158+
val newTestMatrix = createNewTestMatrix(
159+
testRunKey = testRunId,
160+
testMatrix = testMatrix,
161+
testTargets = testTargets,
162+
flakyTestAttempts = flakyTestAttempts
123163
)
124164
logger.info {
125165
"created test matrix: $newTestMatrix"
@@ -140,6 +180,37 @@ internal class TestMatrixStore(
140180
return newTestMatrix
141181
}
142182

183+
/**
184+
* Check if a [TestMatrix] exists for the given [testRunId]
185+
* If it does, ensure it is not in an ERROR state
186+
* and [cachedTestMatrixFilter] allows reusing the testMatrix
187+
*/
188+
private suspend fun getCachedTestMatrix(
189+
testRunId: TestRun.Id,
190+
cachedTestMatrixFilter: CachedTestMatrixFilter,
191+
): TestMatrix? {
192+
getExistingTestMatrix(testRunId)?.let {
193+
logger.info("found existing test matrix: ${it.testMatrixId} with state: ${it.state}")
194+
val state = it.state
195+
// these states are ordered so anything above ERROR is not worth re-using
196+
if (state != null && state >= TestMatrix.State.ERROR) {
197+
logger.warn {
198+
"Skipping cache for ${it.testMatrixId} because its state is $state"
199+
}
200+
} else if (!cachedTestMatrixFilter(it)) {
201+
logger.info {
202+
"Not re-using cached matrix due to filter"
203+
}
204+
} else {
205+
return it
206+
}
207+
}
208+
logger.trace {
209+
"No test run history for $testRunId or cached TestMatrix is rejected."
210+
}
211+
return null
212+
}
213+
143214
/**
144215
* Returns an existing TestMatrix for the given [testRunId].
145216
*
@@ -183,7 +254,7 @@ internal class TestMatrixStore(
183254
testApk: UploadedApk,
184255
pullScreenshots: Boolean = false,
185256
testTargets: List<String>? = null,
186-
flakyTestAttempts: Int? = 2
257+
flakyTestAttempts: Int = 2
187258
): TestMatrix {
188259
val packageName = firebaseTestLabApi.getApkDetails(
189260
FileReference(
@@ -210,35 +281,96 @@ internal class TestMatrixStore(
210281
else null
211282
)
212283
}
213-
return TestMatrix(
214-
projectId = firebaseProjectId,
215-
flakyTestAttempts = flakyTestAttempts,
216-
testSpecification = TestSpecification(
217-
testTimeout = "2700s", // Limit for physical devices.
218-
disableVideoRecording = false,
219-
disablePerformanceMetrics = true, // Not a useful feature for androidx
220-
androidInstrumentationTest = AndroidInstrumentationTest(
221-
appApk = FileReference(
222-
gcsPath = appApk.gcsPath.path
223-
),
224-
testApk = FileReference(
225-
gcsPath = testApk.gcsPath.path
226-
),
227-
shardingOption = sharding,
228-
testTargets = testTargets
284+
val testSpecification = TestSpecification(
285+
testTimeout = "2700s", // Limit for physical devices.
286+
disableVideoRecording = false,
287+
disablePerformanceMetrics = true, // Not a useful feature for androidx
288+
androidInstrumentationTest = AndroidInstrumentationTest(
289+
appApk = FileReference(
290+
gcsPath = appApk.gcsPath.path
291+
),
292+
testApk = FileReference(
293+
gcsPath = testApk.gcsPath.path
229294
),
230-
testSetup = testSetup
295+
shardingOption = sharding,
296+
testTargets = testTargets
297+
),
298+
testSetup = testSetup
299+
)
300+
val resultStorage = ResultStorage(
301+
googleCloudStorage = GoogleCloudStorage(
302+
gcsPath = testRunKey.resultGcsPath().path
231303
),
304+
toolResultsHistory = ToolResultsHistory(
305+
projectId = firebaseProjectId,
306+
historyId = historyId
307+
)
308+
)
309+
return createTestMatrix(
310+
flakyTestAttempts = flakyTestAttempts,
311+
testSpecification = testSpecification,
232312
clientInfo = clientInfo,
233313
environmentMatrix = environmentMatrix,
234-
resultStorage = ResultStorage(
235-
googleCloudStorage = GoogleCloudStorage(
236-
gcsPath = testRunKey.resultGcsPath().path
237-
),
238-
toolResultsHistory = ToolResultsHistory(
239-
projectId = firebaseProjectId,
240-
historyId = historyId
241-
)
314+
resultStorage = resultStorage
315+
)
316+
}
317+
318+
/**
319+
* Creates a [TestMatrix] to run the tests specified in [testTargets] list
320+
* using the same configuration as [testMatrix]
321+
*/
322+
private suspend fun createNewTestMatrix(
323+
testRunKey: TestRun.Id,
324+
testMatrix: TestMatrix,
325+
testTargets: List<String>? = null,
326+
flakyTestAttempts: Int = 0
327+
): TestMatrix {
328+
logger.trace {
329+
"test matrix id: ${testMatrix.testMatrixId}"
330+
}
331+
332+
val testSpecification = testMatrix.testSpecification.copy(
333+
androidInstrumentationTest = testMatrix.testSpecification.androidInstrumentationTest?.copy(
334+
testTargets = testTargets
335+
)
336+
)
337+
338+
val resultStorage = ResultStorage(
339+
googleCloudStorage = GoogleCloudStorage(
340+
gcsPath = testRunKey.resultGcsPath().path
341+
),
342+
toolResultsHistory = testMatrix.resultStorage.toolResultsHistory
343+
)
344+
345+
return createTestMatrix(
346+
clientInfo = testMatrix.clientInfo,
347+
environmentMatrix = testMatrix.environmentMatrix,
348+
testSpecification = testSpecification,
349+
resultStorage = resultStorage,
350+
flakyTestAttempts = flakyTestAttempts
351+
)
352+
}
353+
354+
/**
355+
* Creates a [TestMatrix] using the specified parameters
356+
*/
357+
suspend fun createTestMatrix(
358+
clientInfo: ClientInfo?,
359+
environmentMatrix: EnvironmentMatrix,
360+
testSpecification: TestSpecification,
361+
resultStorage: ResultStorage,
362+
flakyTestAttempts: Int
363+
): TestMatrix {
364+
return firebaseTestLabApi.createTestMatrix(
365+
projectId = firebaseProjectId,
366+
requestId = UUID.randomUUID().toString(),
367+
testMatrix = TestMatrix(
368+
projectId = firebaseProjectId,
369+
flakyTestAttempts = flakyTestAttempts,
370+
testSpecification = testSpecification,
371+
clientInfo = clientInfo,
372+
environmentMatrix = environmentMatrix,
373+
resultStorage = resultStorage
242374
)
243375
)
244376
}

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,19 @@ interface TestRunnerService {
8181
pullScreenshots: Boolean = false,
8282
cachedTestMatrixFilter: CachedTestMatrixFilter = { true },
8383
testTargets: List<String>? = null,
84-
flakyTestAttempts: Int? = 2
84+
flakyTestAttempts: Int = 2
8585
): ScheduleTestsResponse
8686

87+
/**
88+
* Schedules a task to create a [TestMatrix] to run tests
89+
* specified in the [testTargets] list using the same configuration as [testMatrix]
90+
*/
91+
suspend fun scheduleTests(
92+
testMatrix: TestMatrix,
93+
testTargets: List<String>,
94+
cachedTestMatrixFilter: CachedTestMatrixFilter = { true }
95+
): TestMatrix
96+
8797
/**
8898
* Queries the Firebase for the given [testMatrixId] and returns it if it exists.
8999
*/

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ internal class TestRunnerServiceImpl internal constructor(
101101
pullScreenshots: Boolean,
102102
cachedTestMatrixFilter: CachedTestMatrixFilter,
103103
testTargets: List<String>?,
104-
flakyTestAttempts: Int?
104+
flakyTestAttempts: Int
105105
): TestRunnerService.ScheduleTestsResponse {
106106
val testMatrices = testLabController.submitTests(
107107
appApk = appApk ?: apkStore.getPlaceholderApk(),
@@ -120,6 +120,18 @@ internal class TestRunnerServiceImpl internal constructor(
120120
)
121121
}
122122

123+
override suspend fun scheduleTests(
124+
testMatrix: TestMatrix,
125+
testTargets: List<String>,
126+
cachedTestMatrixFilter: CachedTestMatrixFilter
127+
): TestMatrix {
128+
return testLabController.scheduleTests(
129+
testMatrix = testMatrix,
130+
testTargets = testTargets,
131+
cachedTestMatrixFilter = cachedTestMatrixFilter
132+
)
133+
}
134+
123135
override suspend fun getTestMatrix(
124136
testMatrixId: String
125137
): TestMatrix? {

0 commit comments

Comments
 (0)