refactor: Savedmatrix immutability change

test_runner/src/main/kotlin/ftl/json/MatrixMap.kt

@@ -9,12 +9,12 @@ import ftl.util.MatrixCanceledError
 import ftl.util.MatrixState
 
 class MatrixMap(
-    val map: Map<String, SavedMatrix>,
+    val map: MutableMap<String, SavedMatrix>,
     val runPath: String
 ) {
     // determine success by FTL API, not junit xml.
     // FTL sometimes generates empty XML reports (0 failures) for a failed matrix
-    fun allSuccessful(): Boolean = map.values.any(SavedMatrix::failed).not()
+    fun allSuccessful(): Boolean = map.values.any(SavedMatrix::isFailed).not()
 
     /**
      * There are two sources of information for detecting the exit code
@@ -39,11 +39,11 @@ class MatrixMap(
      */
     fun validateMatrices(shouldIgnore: Boolean = false) {
         map.values.run {
-            firstOrNull { it.canceledByUser() }?.let { throw MatrixCanceledError(it.outcomeDetails) }
-            firstOrNull { it.infrastructureFail() }?.let { throw InfrastructureError(it.outcomeDetails) }
-            firstOrNull { it.incompatibleFail() }?.let { throw IncompatibleTestDimensionError(it.outcomeDetails) }
+            firstOrNull { it.canceledByUser() }?.let { throw MatrixCanceledError(it.outcomeDetails.orEmpty()) }
+            firstOrNull { it.infrastructureFail() }?.let { throw InfrastructureError(it.outcomeDetails.orEmpty()) }
+            firstOrNull { it.incompatibleFail() }?.let { throw IncompatibleTestDimensionError(it.outcomeDetails.orEmpty()) }
             firstOrNull { it.state != MatrixState.FINISHED }?.let { throw FTLError(it) }
-            filter { it.failed() }.let {
+            filter { it.isFailed() }.let {
                 if (it.isNotEmpty()) throw FailedMatrixError(
                     matrices = it,
                     ignoreFailed = shouldIgnore
@@ -54,5 +54,7 @@ class MatrixMap(
 }
 
 fun Iterable<TestMatrix>.update(matrixMap: MatrixMap) = forEach { matrix ->
-    matrixMap.map[matrix.testMatrixId]?.update(matrix)
+    matrixMap.map[matrix.testMatrixId]?.updateWithMatrix(matrix)?.let {
+        matrixMap.map[matrix.testMatrixId] = it
+    }
 }

test_runner/src/main/kotlin/ftl/json/SavedMatrix.kt

@@ -1,120 +1,107 @@
 package ftl.json
 
 import com.google.api.services.testing.model.TestMatrix
+import ftl.reports.outcome.BillableMinutes
+import ftl.reports.outcome.TestOutcome
 import ftl.reports.outcome.createMatrixOutcomeSummary
 import ftl.reports.outcome.fetchTestOutcomeContext
-import ftl.util.FlankGeneralError
 import ftl.util.MatrixState.FINISHED
 import ftl.util.MatrixState.INVALID
 import ftl.util.StepOutcome.failure
 import ftl.util.StepOutcome.inconclusive
 import ftl.util.StepOutcome.skipped
-import ftl.util.StepOutcome.success
+import ftl.util.getClientDetails
+import ftl.util.getGcsPath
+import ftl.util.getGcsPathWithoutRootBucket
+import ftl.util.getGcsRootBucket
 import ftl.util.webLink
+import ftl.util.webLinkWithoutExecutionDetails
 
 // execution gcs paths aren't API accessible.
-class SavedMatrix(matrix: TestMatrix) {
-    val matrixId: String = matrix.testMatrixId
-    var state: String = matrix.state
-        private set
-    val gcsPath: String = matrix.resultStorage.googleCloudStorage.gcsPath
-    var webLink: String = matrix.webLink()
-        private set
-    var downloaded = false
-
-    var billableVirtualMinutes: Long = 0
-        private set
-    var billablePhysicalMinutes: Long = 0
-        private set
-    var outcome: String = ""
-        private set
-    var outcomeDetails: String = ""
-        private set
-    val clientDetails = matrix.clientInfo?.clientInfoDetails
-        ?.map { it.key to it.value }
-        ?.toMap()
-
-    init {
-        if (this.state == FINISHED) finished(matrix)
-    }
-
-    // ExitCodeFromRollupOutcome - https://github.com/bootstraponline/gcloud_cli/blob/137d864acd5928baf25434cf59b0225c4d1f9319/google-cloud-sdk/lib/googlecloudsdk/api_lib/firebase/test/exit_code.py#L46
-    fun failed(): Boolean {
-        // outcome of the test execution.
-        // skipped means unsupported environment
-        return when (outcome) {
-            failure -> true
-            skipped -> true
-            inconclusive -> true
-            else -> false
-        }
-    }
+data class SavedMatrix(
+    val matrixId: String = "",
+    val state: String = "",
+    val gcsPath: String = "",
+    val webLink: String = "",
+    val downloaded: Boolean = false,
+    val billableVirtualMinutes: Long = 0,
+    val billablePhysicalMinutes: Long = 0,
+    val outcome: String = "",
+    val outcomeDetails: String = "",
+    val clientDetails: Map<String, String>? = null,
+    val gcsPathWithoutRootBucket: String = "",
+    val gcsRootBucket: String = "",
+    val webLinkWithoutExecutionDetails: String? = "",
+)
+
+fun createSavedMatrix(testMatrix: TestMatrix) = SavedMatrix().updateWithMatrix(testMatrix)
 
-    /** return true if the content changed **/
-    fun update(matrix: TestMatrix): Boolean {
-        val newState = matrix.state
-        val newLink = matrix.webLink()
-        val changedState = state != newState
-        val changedLink = webLink != newLink
+fun SavedMatrix.canceledByUser() = outcomeDetails == ABORTED_BY_USER_MESSAGE
 
-        if (changedState) {
-            updateState(newState, matrix)
-        }
+fun SavedMatrix.infrastructureFail() = outcomeDetails == INFRASTRUCTURE_FAILURE_MESSAGE
 
-        if (changedLink) {
-            this.webLink = newLink
-        }
+fun SavedMatrix.incompatibleFail() = outcomeDetails in arrayOf(
+    INCOMPATIBLE_APP_VERSION_MESSAGE,
+    INCOMPATIBLE_ARCHITECTURE_MESSAGE,
+    INCOMPATIBLE_DEVICE_MESSAGE
+)
+
+fun SavedMatrix.isFailed() = when (outcome) {
+    failure -> true
+    skipped -> true
+    inconclusive -> true
+    else -> false
+}
 
-        return changedState || changedLink
-    }
+fun SavedMatrix.needsUpdate(newMatrix: TestMatrix): Boolean {
+    val newState = newMatrix.state
+    val newLink = newMatrix.webLink()
+    val changedState = state != newState
+    val changedLink = webLink != newLink
+    return (changedState || changedLink)
+}
 
-    private fun updateState(newState: String, testMatrix: TestMatrix) {
-        state = newState
-        when (state) {
-            FINISHED -> finished(testMatrix)
-            INVALID -> {
-                outcomeDetails = "Matrix is invalid"
-                outcome = "---"
-            }
-        }
-    }
+internal fun SavedMatrix.updateWithMatrix(newMatrix: TestMatrix): SavedMatrix =
+    if (needsUpdate(newMatrix)) updatedSavedMatrix(newMatrix)
+    else this
 
-    private fun finished(matrix: TestMatrix) {
-        if (matrix.state != FINISHED) {
-            throw FlankGeneralError("Matrix ${matrix.testMatrixId} ${matrix.state} != $FINISHED")
-        }
-        billableVirtualMinutes = 0
-        billablePhysicalMinutes = 0
-        outcome = success
+private fun SavedMatrix.updatedSavedMatrix(
+    newMatrix: TestMatrix
+): SavedMatrix = when (newMatrix.state) {
+    state -> this
 
-        updateFinishedMatrixData(matrix)
+    FINISHED -> newMatrix.fetchTestOutcomeContext().createMatrixOutcomeSummary().let { (billableMinutes, outcome) ->
+        updateProperties(newMatrix).updateOutcome(outcome).updateBillableMinutes(billableMinutes)
     }
 
-    private fun updateFinishedMatrixData(matrix: TestMatrix) {
-        matrix.fetchTestOutcomeContext().createMatrixOutcomeSummary().let { (billableMinutes, summary) ->
-            outcome = summary.outcome
-            outcomeDetails = summary.testDetails
-            billableVirtualMinutes = billableMinutes.virtual
-            billablePhysicalMinutes = billableMinutes.physical
-        }
-    }
+    INVALID -> updateProperties(newMatrix).updateOutcome(invalidTestOutcome())
 
-    val gcsPathWithoutRootBucket get() = gcsPath.substringAfter('/')
-    val gcsRootBucket get() = gcsPath.substringBefore('/')
-    val webLinkWithoutExecutionDetails: String
-        get() {
-            return if (webLink.isEmpty()) {
-                webLink
-            } else {
-                val executionsRegex = "/executions/.+".toRegex()
-                val foundValue = executionsRegex.find(webLink)?.value.orEmpty()
-                webLink.removeSuffix(foundValue)
-            }
-        }
+    else -> updateProperties(newMatrix)
 }
 
-fun SavedMatrix.canceledByUser() = outcomeDetails == ABORTED_BY_USER_MESSAGE
-
-fun SavedMatrix.infrastructureFail() = outcomeDetails == INFRASTRUCTURE_FAILURE_MESSAGE
-
-fun SavedMatrix.incompatibleFail() = outcomeDetails in arrayOf(INCOMPATIBLE_APP_VERSION_MESSAGE, INCOMPATIBLE_ARCHITECTURE_MESSAGE, INCOMPATIBLE_DEVICE_MESSAGE)
+private fun SavedMatrix.updateProperties(newMatrix: TestMatrix) = copy(
+    matrixId = newMatrix.testMatrixId,
+    state = newMatrix.state,
+    gcsPath = newMatrix.getGcsPath(),
+    webLink = newMatrix.webLink(),
+    downloaded = false,
+    clientDetails = newMatrix.getClientDetails(),
+    gcsPathWithoutRootBucket = newMatrix.getGcsPathWithoutRootBucket(),
+    gcsRootBucket = newMatrix.getGcsRootBucket(),
+    webLinkWithoutExecutionDetails = newMatrix.webLinkWithoutExecutionDetails()
+)
+
+private fun SavedMatrix.updateBillableMinutes(billableMinutes: BillableMinutes) = copy(
+    billablePhysicalMinutes = billableMinutes.physical,
+    billableVirtualMinutes = billableMinutes.virtual,
+)
+
+private fun SavedMatrix.updateOutcome(testOutcome: TestOutcome) = copy(
+    outcome = testOutcome.outcome,
+    outcomeDetails = testOutcome.testDetails
+)
+
+private fun invalidTestOutcome() = TestOutcome(
+    outcome = "---",
+    testDetails = "Matrix is invalid"
+)

test_runner/src/main/kotlin/ftl/reports/MatrixResultsReport.kt

@@ -5,6 +5,7 @@ import ftl.config.FtlConstants.indent
 import ftl.gc.GcStorage
 import ftl.json.MatrixMap
 import ftl.json.SavedMatrix
+import ftl.json.isFailed
 import ftl.reports.util.IReport
 import ftl.reports.xml.model.JUnitTestResult
 import ftl.util.asPrintableTable
@@ -32,7 +33,7 @@ object MatrixResultsReport : IReport {
     private fun generate(matrices: MatrixMap): String {
         val total = matrices.map.size
         // unfinished matrix will not be reported as failed since it's still running
-        val success = matrices.map.values.count { it.failed().not() }
+        val success = matrices.map.values.count { it.isFailed().not() }
         val failed = total - success
         val successDouble: Double = success.toDouble() / total.toDouble() * 100.0
         val successPercent = percentFormat.format(successDouble)
@@ -58,12 +59,12 @@ object MatrixResultsReport : IReport {
         }
     }
 
-private fun Collection<SavedMatrix>.printMatricesLinks(writer: StringWriter) = this
-        .filter { it.failed() }
+    private fun Collection<SavedMatrix>.printMatricesLinks(writer: StringWriter) = this
+        .filter { it.isFailed() }
         .takeIf { it.isNotEmpty() }
         ?.run {
             writer.println("More details are available at:")
-            forEach { writer.println(it.webLinkWithoutExecutionDetails) }
+            forEach { writer.println(it.webLinkWithoutExecutionDetails.orEmpty()) }
             writer.println()
         }
 

test_runner/src/main/kotlin/ftl/reports/outcome/CreateMatrixOutcomeSummary.kt

@@ -5,10 +5,10 @@ import com.google.api.services.toolresults.model.Environment
 fun TestOutcomeContext.createMatrixOutcomeSummary(): Pair<BillableMinutes, TestOutcome> =
     steps.calculateAndroidBillableMinutes(projectId, testTimeout) to
         if (environments.hasOutcome())
-            environments.createMatrixOutcomeSummaryUsingEnvironments(matrixId)
+            environments.createMatrixOutcomeSummaryUsingEnvironments()
         else {
             if (steps.isEmpty()) println("No test results found, something went wrong. Try re-running the tests.")
-            steps.createMatrixOutcomeSummaryUsingSteps(matrixId)
+            steps.createMatrixOutcomeSummaryUsingSteps()
         }
 
 private fun List<Environment>.hasOutcome() = isNotEmpty() && any { env ->

test_runner/src/main/kotlin/ftl/reports/outcome/TestOutcome.kt

@@ -8,17 +8,14 @@ import ftl.util.StepOutcome
 
 data class TestOutcome(
     val outcome: String,
-    val matrixId: String,
     val testDetails: String
 )
 
 fun List<Environment>.createMatrixOutcomeSummaryUsingEnvironments(
-    testMatrixId: String,
     outcome: Outcome? = getOutcomeFromEnvironments(),
     testDetails: String? = outcome?.getDetails(map { it.createTestSuiteOverviewData() }.foldTestSuiteOverviewData())
 ) = TestOutcome(
     outcome = outcome?.summary ?: "Unknown",
-    matrixId = testMatrixId,
     testDetails = testDetails ?: "Unknown outcome"
 )
 
@@ -27,12 +24,10 @@ private fun List<Environment>.getOutcomeFromEnvironments(): Outcome? = maxByOrNu
 }?.environmentResult?.outcome
 
 fun List<Step>.createMatrixOutcomeSummaryUsingSteps(
-    testMatrixId: String,
     outcome: Outcome? = getOutcomeFromSteps(),
     testDetails: String? = outcome?.getDetails(createTestSuiteOverviewData())
 ) = TestOutcome(
     outcome = outcome?.summary ?: "Unknown",
-    matrixId = testMatrixId,
     testDetails = testDetails ?: "Unknown outcome"
 )
 

test_runner/src/main/kotlin/ftl/reports/outcome/TestOutcomeContext.kt

@@ -6,6 +6,7 @@ import com.google.api.services.toolresults.model.Environment
 import com.google.api.services.toolresults.model.Step
 import ftl.gc.GcToolResults
 import ftl.json.SavedMatrix
+import ftl.json.createSavedMatrix
 import ftl.util.FTLError
 import ftl.util.timeoutToSeconds
 
@@ -32,7 +33,7 @@ private fun TestMatrix.getToolResultsIds(): ToolResultsExecution = ToolResultsEx
     .setHistoryId(resultStorage?.toolResultsExecution?.historyId ?: throw badMatrixError())
     .setExecutionId(resultStorage?.toolResultsExecution?.executionId ?: throw badMatrixError())
 
-private fun TestMatrix.badMatrixError() = BadMatrixError(SavedMatrix(this))
+private fun TestMatrix.badMatrixError() = BadMatrixError(createSavedMatrix(this))
 
 class BadMatrixError(matrix: SavedMatrix) : FTLError(matrix)
 

test_runner/src/main/kotlin/ftl/run/RefreshLastRun.kt

@@ -12,7 +12,9 @@ import ftl.run.common.getLastMatrices
 import ftl.run.common.pollMatrices
 import ftl.run.common.updateMatrixFile
 import ftl.args.ShardChunks
+import ftl.json.needsUpdate
 import ftl.json.update
+import ftl.json.updateWithMatrix
 import ftl.util.MatrixState
 import kotlinx.coroutines.Deferred
 import kotlinx.coroutines.Dispatchers
@@ -58,7 +60,12 @@ private suspend fun refreshMatrices(matrixMap: MatrixMap, args: IArgs) = corouti
 
         println(FtlConstants.indent + "${matrix.state} $matrixId")
 
-        if (map[matrixId]?.update(matrix) == true) dirty = true
+        if (map[matrixId]?.needsUpdate(matrix) == true) {
+            map[matrixId]?.updateWithMatrix(matrix)?.let {
+                map[matrixId] = it
+                dirty = true
+            }
+        }
     }
 
     if (dirty) {

test_runner/src/main/kotlin/ftl/run/common/FetchArtifacts.kt

@@ -25,10 +25,10 @@ internal suspend fun fetchArtifacts(matrixMap: MatrixMap, args: IArgs) = corouti
         val finished = it.state == MatrixState.FINISHED
         val notDownloaded = !it.downloaded
         finished && notDownloaded
-    }
+    }.toMutableList()
 
     print(FtlConstants.indent)
-    filtered.flatMap { matrix ->
+    filtered.flatMapIndexed { index, matrix ->
         val prefix = Storage.BlobListOption.prefix(matrix.gcsPathWithoutRootBucket)
         val result = GcStorage.storage.list(matrix.gcsRootBucket, prefix, fields)
         val artifactsList = Artifacts.regexList(args)
@@ -50,7 +50,7 @@ internal suspend fun fetchArtifacts(matrixMap: MatrixMap, args: IArgs) = corouti
         }
 
         dirty = true
-        matrix.downloaded = true
+        filtered[index] = matrix.copy(downloaded = true)
         jobs
     }.joinAll()
     println()

test_runner/src/main/kotlin/ftl/run/common/GetLastMatrices.kt

@@ -1,6 +1,5 @@
 package ftl.run.common
 
-import com.google.gson.reflect.TypeToken
 import ftl.args.IArgs
 import ftl.config.FtlConstants
 import ftl.json.MatrixMap
@@ -24,8 +23,7 @@ internal fun matrixPathToObj(path: String, args: IArgs): MatrixMap {
     }
     val json = filePath.readText()
 
-    val listOfSavedMatrix = object : TypeToken<MutableMap<String, SavedMatrix>>() {}.type
-    val map: MutableMap<String, SavedMatrix> = prettyPrint.fromJson(json, listOfSavedMatrix)
+    val map: MutableMap<String, SavedMatrix> = fromJson(json)
 
     return MatrixMap(map, path)
 }