Merge branch 'master' into #829-added-outcome-details

Flank · Jun 30, 2020 · 86d1c9f · 86d1c9f
2 parents 4bd6ee8 + 3978611
commit 86d1c9f
Show file tree

Hide file tree

Showing 16 changed files with 199 additions and 107 deletions.
diff --git a/docs/bugs/deviceUsageDuration_always_null.md b/docs/bugs/deviceUsageDuration_always_null.md
@@ -0,0 +1,25 @@
+# deviceUsageDuration always null
+
+## deviceUsageDuration description
+
+> How much the device resource is used to perform the test.
+>  
+> This is the device usage used for billing purpose, which is different from the run_duration,
+> for example, infrastructure failure won't be charged for device usage.
+>  
+> PRECONDITION_FAILED will be returned if one attempts to set a device_usage on a step which
+> already has this field set.
+>  
+> - In response: present if previously set. - In create request: optional - In update request:
+> optional
+> @return value or {@code null} for none
+
+## Problem description
+
+Problem found on pull request: [Flank needs to respect the timeout value as that's a cap for billing purposes. #865](https://github.com/Flank/flank/pull/865)
+
+`deviceUsageDuration` still is null even if we testing on blaze plan with free quota spent
+
+## Next steps
+
+In future we should check problem status and if problem is fixed on testlab we should implement it on Flank
diff --git a/release_notes.md b/release_notes.md
@@ -3,6 +3,7 @@
 - [#837](https://github.com/Flank/flank/pull/837) Added obfuscate option to dump shards. ([piotradamczyk5](https://github.com/piotradamczyk5))
 - [#868](https://github.com/Flank/flank/pull/868) Restored weblinks to all test results, not just failures. ([rainnapper](https://github.com/rainnapper))
 - [#828](https://github.com/Flank/flank/pull/828) Store test results in gcloud bucket. ([adamfilipow92](https://github.com/adamfilipow92))
+- [#865](https://github.com/Flank/flank/pull/865) Flank needs to respect the timeout value as that's a cap for billing purposes. ([adamfilipow92](https://github.com/adamfilipow92), [pawelpasterz](https://github.com/pawelpasterz))
 - [#862](https://github.com/Flank/flank/pull/862) Added printing outcome details. ([piotradamczyk5](https://github.com/piotradamczyk5), [jan-gogo](https://github.com/jan-gogo))
 -
 -

diff --git a/test_runner/docs/ascii/flank.jar_-android-run.adoc b/test_runner/docs/ascii/flank.jar_-android-run.adoc
@@ -13,12 +13,13 @@ flank.jar
 
 *flank.jar
  android run* [*-h*] [*--async*] [*--auto-google-login*]
-                       [*--disable-sharding*] [*--dry*] [*--dump-shards*]
-                       [*--full-junit-result*] [*--ignore-failed-tests*]
-                       [*--keep-file-path*] [*--legacy-junit-result*]
-                       [*--no-auto-google-login*] [*--no-performance-metrics*]
-                       [*--no-record-video*] [*--no-use-orchestrator*]
-                       [*--obfuscate*] [*--performance-metrics*] [*--record-video*]
+                       [*--disable-results-upload*] [*--disable-sharding*] [*--dry*]
+                       [*--dump-shards*] [*--full-junit-result*]
+                       [*--ignore-failed-tests*] [*--keep-file-path*]
+                       [*--legacy-junit-result*] [*--no-auto-google-login*]
+                       [*--no-performance-metrics*] [*--no-record-video*]
+                       [*--no-use-orchestrator*] [*--obfuscate*]
+                       [*--performance-metrics*] [*--record-video*]
                        [*--smart-flank-disable-upload*] [*--use-orchestrator*]
                        [*--app*=_<app>_] [*-c*=_<configPath>_]
                        [*--local-result-dir*=_<localResultsDir>_]
@@ -159,12 +160,12 @@ Configuration is read from flank.yml
 *--output-style*=_<outputStyle>_::
   Output style of execution status. May be one of [verbose, multi, single]. For runs with only one test execution the default value is 'verbose', in other cases 'multi' is used as the default. The output style 'multi' is not displayed correctly on consoles which don't support ansi codes, to avoid corrupted output use `single` or `verbose`.
 
-*--app*=_<app>_::
-  The path to the application binary file. The path may be in the local filesystem or in Google Cloud Storage using gs:// notation.
-
 *--disable-results-upload*::
   Disables flank results upload on gcloud storage.
 
+*--app*=_<app>_::
+  The path to the application binary file. The path may be in the local filesystem or in Google Cloud Storage using gs:// notation.
+
 *--test*=_<test>_::
   The path to the binary file containing instrumentation tests. The given path may be in the local filesystem or in Google Cloud Storage using a URL beginning with gs://.
 
@@ -177,9 +178,6 @@ Configuration is read from flank.yml
 *--no-auto-google-login*::
   Google account not logged in (default behavior). Use --auto-google-login to enable
 
-*--app*=_<app>_::
-  The path to the application binary file. The path may be in the local filesystem or in Google Cloud Storage using gs:// notation.
-
 *--use-orchestrator*::
   Whether each test runs in its own Instrumentation instance with the Android Test Orchestrator (default: Orchestrator is used. To disable, use --no-use-orchestrator). Orchestrator is only compatible with AndroidJUnitRunner v1.0 or higher. See https://developer.android.com/training/testing/junit-runner.html#using-android-test-orchestrator for more information about Android Test Orchestrator.
 

diff --git a/test_runner/docs/ascii/flank.jar_-firebase-test-android-run.adoc b/test_runner/docs/ascii/flank.jar_-firebase-test-android-run.adoc
@@ -172,6 +172,9 @@ Configuration is read from flank.yml
 *--output-style*=_<outputStyle>_::
   Output style of execution status. May be one of [verbose, multi, single]. For runs with only one test execution the default value is 'verbose', in other cases 'multi' is used as the default. The output style 'multi' is not displayed correctly on consoles which don't support ansi codes, to avoid corrupted output use `single` or `verbose`.
 
+*--disable-results-upload*::
+  Disables flank results upload on gcloud storage.
+
 *--app*=_<app>_::
   The path to the application binary file. The path may be in the local filesystem or in Google Cloud Storage using gs:// notation.
 
@@ -187,9 +190,6 @@ Configuration is read from flank.yml
 *--no-auto-google-login*::
   Google account not logged in (default behavior). Use --auto-google-login to enable
 
-*--app*=_<app>_::
-  The path to the application binary file. The path may be in the local filesystem or in Google Cloud Storage using gs:// notation.
-
 *--use-orchestrator*::
   Whether each test runs in its own Instrumentation instance with the Android Test Orchestrator (default: Orchestrator is used. To disable, use --no-use-orchestrator). Orchestrator is only compatible with AndroidJUnitRunner v1.0 or higher. See https://developer.android.com/training/testing/junit-runner.html#using-android-test-orchestrator for more information about Android Test Orchestrator.
 

diff --git a/test_runner/docs/ascii/flank.jar_-firebase-test-ios-run.adoc b/test_runner/docs/ascii/flank.jar_-firebase-test-ios-run.adoc
@@ -12,11 +12,11 @@ flank.jar
 == Synopsis
 
 *flank.jar
- firebase test ios run* [*-h*] [*--async*] [*--disable-sharding*] [*--dry*]
-                                 [*--dump-shards*] [*--full-junit-result*]
-                                 [*--ignore-failed-tests*] [*--keep-file-path*]
-                                 [*--no-record-video*] [*--obfuscate*]
-                                 [*--record-video*]
+ firebase test ios run* [*-h*] [*--async*] [*--disable-results-upload*]
+                                 [*--disable-sharding*] [*--dry*] [*--dump-shards*]
+                                 [*--full-junit-result*] [*--ignore-failed-tests*]
+                                 [*--keep-file-path*] [*--no-record-video*]
+                                 [*--obfuscate*] [*--record-video*]
                                  [*--smart-flank-disable-upload*]
                                  [*-c*=_<configPath>_]
                                  [*--local-result-dir*=_<localResultsDir>_]
@@ -151,6 +151,9 @@ Configuration is read from flank.yml
 *--output-style*=_<outputStyle>_::
   Output style of execution status. May be one of [verbose, multi, single]. For runs with only one test execution the default value is 'verbose', in other cases 'multi' is used as the default. The output style 'multi' is not displayed correctly on consoles which don't support ansi codes, to avoid corrupted output use `single` or `verbose`.
 
+*--disable-results-upload*::
+  Disables flank results upload on gcloud storage.
+
 *--test*=_<test>_::
   The path to the test package (a zip file containing the iOS app and XCTest files). The given path may be in the local filesystem or in Google Cloud Storage using a URL beginning with gs://. Note: any .xctestrun file in this zip file will be ignored if --xctestrun-file is specified.
 

diff --git a/test_runner/docs/ascii/flank.jar_-ios-run.adoc b/test_runner/docs/ascii/flank.jar_-ios-run.adoc
@@ -12,7 +12,8 @@ flank.jar
 == Synopsis
 
 *flank.jar
- ios run* [*-h*] [*--async*] [*--disable-sharding*] [*--dry*] [*--dump-shards*]
+ ios run* [*-h*] [*--async*] [*--disable-results-upload*]
+                   [*--disable-sharding*] [*--dry*] [*--dump-shards*]
                    [*--full-junit-result*] [*--ignore-failed-tests*]
                    [*--keep-file-path*] [*--no-record-video*] [*--obfuscate*]
                    [*--record-video*] [*--smart-flank-disable-upload*]
@@ -142,6 +143,9 @@ Configuration is read from flank.yml
 *--output-style*=_<outputStyle>_::
   Output style of execution status. May be one of [verbose, multi, single]. For runs with only one test execution the default value is 'verbose', in other cases 'multi' is used as the default. The output style 'multi' is not displayed correctly on consoles which don't support ansi codes, to avoid corrupted output use `single` or `verbose`.
 
+*--disable-results-upload*::
+  Disables flank results upload on gcloud storage.
+
 *--test*=_<test>_::
   The path to the test package (a zip file containing the iOS app and XCTest files). The given path may be in the local filesystem or in Google Cloud Storage using a URL beginning with gs://. Note: any .xctestrun file in this zip file will be ignored if --xctestrun-file is specified.
 

diff --git a/test_runner/src/main/kotlin/ftl/json/SavedMatrix.kt b/test_runner/src/main/kotlin/ftl/json/SavedMatrix.kt
@@ -8,14 +8,17 @@ import ftl.reports.api.createTestExecutionDataListAsync
 import ftl.reports.api.createTestSuitOverviewData
 import ftl.reports.api.data.TestSuiteOverviewData
 import ftl.reports.api.prepareForJUnitResult
-import ftl.util.Billing
+import ftl.util.MatrixState.ERROR
 import ftl.util.MatrixState.FINISHED
 import ftl.util.StepOutcome.failure
 import ftl.util.StepOutcome.flaky
 import ftl.util.StepOutcome.inconclusive
 import ftl.util.StepOutcome.skipped
 import ftl.util.StepOutcome.success
+import ftl.util.billableMinutes
+import ftl.util.timeoutToSeconds
 import ftl.util.webLink
+import kotlin.math.min
 
 // execution gcs paths aren't API accessible.
 class SavedMatrix(matrix: TestMatrix) {
@@ -107,8 +110,19 @@ class SavedMatrix(matrix: TestMatrix) {
                         device = it.testExecution.environment.androidDevice,
                         projectId = matrix.projectId.orEmpty()
                     ),
-                    billableMinutes = it.step.testExecutionStep?.testTiming?.testProcessDuration?.seconds
-                        ?.let { testTimeSeconds -> Billing.billableMinutes(testTimeSeconds) }
+                    billableMinutes = it.testExecution.takeIf { exec ->
+                        exec.state != ERROR
+                    }?.run {
+                        // testExecutionStep, testTiming, etc. can all be null.
+                        // sometimes testExecutionStep is present and testTiming is null
+                        val stepResult = GcToolResults.getStepResult(toolResultsStep)
+                        val testTimeSeconds = stepResult.testExecutionStep?.testTiming?.testProcessDuration?.seconds ?: return@run null
+                        val testTimeout = timeoutToSeconds(testSpecification?.testTimeout ?: "0s")
+
+                        // if overall test duration time is higher then testTimeout flank should calculate billable minutes for testTimeout
+                        val timeToBill = min(testTimeSeconds, testTimeout)
+                        billableMinutes(timeToBill)
+                    }
                 )
             }
     }

diff --git a/test_runner/src/main/kotlin/ftl/reports/CostReport.kt b/test_runner/src/main/kotlin/ftl/reports/CostReport.kt
@@ -6,9 +6,8 @@ import ftl.gc.GcStorage
 import ftl.json.MatrixMap
 import ftl.reports.util.IReport
 import ftl.reports.xml.model.JUnitTestResult
-import ftl.util.Billing
+import ftl.util.estimateCosts
 import ftl.util.println
-import ftl.util.write
 import java.io.StringWriter
 
 /** Calculates cost based on the matrix map. Always run. */
@@ -25,7 +24,7 @@ object CostReport : IReport {
             totalBillablePhysicalMinutes += it.billablePhysicalMinutes
         }
 
-        return Billing.estimateCosts(totalBillableVirtualMinutes, totalBillablePhysicalMinutes)
+        return estimateCosts(totalBillableVirtualMinutes, totalBillablePhysicalMinutes)
     }
 
     private fun generate(matrices: MatrixMap): String {

diff --git a/test_runner/src/main/kotlin/ftl/run/platform/common/AfterRunTests.kt b/test_runner/src/main/kotlin/ftl/run/platform/common/AfterRunTests.kt
@@ -34,7 +34,6 @@ internal suspend fun afterRunTests(
             config.resultsBucket + "/" + matrixMap.runPath
     println(FtlConstants.indent + gcsBucket)
     println()
-
     matrixMap.printMatricesWebLinks(config.project)
 }
 

diff --git a/test_runner/src/main/kotlin/ftl/util/Billing.kt b/test_runner/src/main/kotlin/ftl/util/Billing.kt
@@ -4,93 +4,74 @@ import java.math.BigDecimal
 import java.math.RoundingMode
 import java.util.concurrent.TimeUnit
 
-object Billing {
+private val physicalCostPerMinute = divBy60(5) // $5/hr
+private val virtualCostPerMinute = divBy60(1) // $1/hr
 
-    private fun divBy60(value: Long): BigDecimal {
-        return BigDecimal(value).divide(BigDecimal(60), 10, RoundingMode.HALF_UP)
-    }
+private fun divBy60(value: Long) = BigDecimal(value).divide(BigDecimal(60), 10, RoundingMode.HALF_UP)
 
-    private fun divBy60(value: BigDecimal): BigDecimal {
-        return value.divide(BigDecimal(60), 10, RoundingMode.HALF_UP)
-    }
+private fun divBy60(value: BigDecimal) = value.divide(BigDecimal(60), 10, RoundingMode.HALF_UP)
 
-    private var PHYSICAL_COST_PER_MIN = divBy60(5) // $5/hr
-    private var VIRTUAL_COST_PER_MIN = divBy60(1) // $1/hr
+// round decimals up. 0.01 minutes is billable at 1 minute.
+fun billableMinutes(testDurationSeconds: Long) = divBy60(checkForZero(BigDecimal(testDurationSeconds)))
+    .setScale(0, RoundingMode.UP)
+    .longValueExact()
 
-    fun billableMinutes(testDurationSeconds: Long): Long {
-        return billableMinutes(BigDecimal(testDurationSeconds))
-    }
-
-    private fun billableMinutes(testDurationSeconds: BigDecimal): Long {
-        val billableMinutes = divBy60(checkForZero(testDurationSeconds))
-        // round decimals up.  0.01 minutes is billable at 1 minute.
-        return billableMinutes.setScale(0, RoundingMode.UP).longValueExact()
-    }
+// 0s duration => 1s
+private fun checkForZero(testDurationSeconds: BigDecimal) =
+    if (testDurationSeconds == BigDecimal.ZERO) BigDecimal.ONE
+    else testDurationSeconds
 
-    private fun checkForZero(testDurationSeconds: BigDecimal): BigDecimal {
-        // 0s duration => 1s
-        if (testDurationSeconds.compareTo(BigDecimal(0)) == 0) {
-            return BigDecimal(1)
-        }
-
-        return testDurationSeconds
-    }
-
-    fun estimateCosts(billableVirtualMinutes: Long, billablePhysicalMinutes: Long): String {
-        return estimateCosts(BigDecimal(billableVirtualMinutes), BigDecimal(billablePhysicalMinutes))
-    }
+fun estimateCosts(billableVirtualMinutes: Long, billablePhysicalMinutes: Long): String {
+    return estimateCosts(BigDecimal(billableVirtualMinutes), BigDecimal(billablePhysicalMinutes))
+}
 
-    private fun estimateCosts(billableVirtualMinutes: BigDecimal, billablePhysicalMinutes: BigDecimal): String {
-        val virtualCost = billableVirtualMinutes.multiply(VIRTUAL_COST_PER_MIN).setScale(2, RoundingMode.HALF_UP)
-        val physicalCost = billablePhysicalMinutes.multiply(PHYSICAL_COST_PER_MIN).setScale(2, RoundingMode.HALF_UP)
-        val totalCost = (virtualCost + physicalCost).setScale(2, RoundingMode.HALF_UP)
+private fun estimateCosts(billableVirtualMinutes: BigDecimal, billablePhysicalMinutes: BigDecimal): String {
+    val virtualCost = billableVirtualMinutes.multiply(virtualCostPerMinute).setScale(2, RoundingMode.HALF_UP)
+    val physicalCost = billablePhysicalMinutes.multiply(physicalCostPerMinute).setScale(2, RoundingMode.HALF_UP)
+    val totalCost = (virtualCost + physicalCost).setScale(2, RoundingMode.HALF_UP)
 
-        val billableVirtualTime = prettyTime(billableVirtualMinutes)
-        val billablePhysicalTime = prettyTime(billablePhysicalMinutes)
-        val totalTime = prettyTime(billableVirtualMinutes + billablePhysicalMinutes)
+    val billableVirtualTime = prettyTime(billableVirtualMinutes)
+    val billablePhysicalTime = prettyTime(billablePhysicalMinutes)
+    val totalTime = prettyTime(billableVirtualMinutes + billablePhysicalMinutes)
 
-        val displayPhysical = billablePhysicalMinutes.signum() == 1
-        val displayVirtual = billableVirtualMinutes.signum() == 1 // 1 = positive number > 0
-        val displayTotal = displayPhysical && displayVirtual
-        var result = ""
+    val displayPhysical = billablePhysicalMinutes.signum() == 1
+    val displayVirtual = billableVirtualMinutes.signum() == 1 // 1 = positive number > 0
+    val displayTotal = displayPhysical && displayVirtual
+    var result = ""
 
-        if (!displayPhysical && !displayVirtual) {
-            result = "No cost. 0m"
-        }
+    if (!displayPhysical && !displayVirtual) {
+        result = "No cost. 0m"
+    }
 
-        if (displayPhysical) {
-            result += """
+    if (displayPhysical) {
+        result += """
 Physical devices
   $$physicalCost for $billablePhysicalTime
 """
-        }
+    }
 
-        if (displayVirtual) {
-            result += """
+    if (displayVirtual) {
+        result += """
 Virtual devices
   $$virtualCost for $billableVirtualTime
 """
-        }
+    }
 
-        if (displayTotal) {
-            result += """
+    if (displayTotal) {
+        result += """
 Total
   $$totalCost for $totalTime
 """
-        }
-
-        return result.trim()
     }
 
-    private fun prettyTime(billableMinutes: BigDecimal): String {
-        val remainder = billableMinutes.toLong()
-        val hours = TimeUnit.MINUTES.toHours(remainder)
-        val minutes = remainder % 60
+    return result.trim()
+}
+
+private fun prettyTime(billableMinutes: BigDecimal): String {
+    val remainder = billableMinutes.toLong()
+    val hours = TimeUnit.MINUTES.toHours(remainder)
+    val minutes = remainder % 60
 
-        return if (hours > 0) {
-            "${hours}h ${minutes}m"
-        } else {
-            "${minutes}m"
-        }
-    }
+    return if (hours > 0) "${hours}h ${minutes}m"
+    else "${minutes}m"
 }
diff --git a/test_runner/src/test/kotlin/Debug.kt b/test_runner/src/test/kotlin/Debug.kt
@@ -10,8 +10,8 @@ fun main() {
     // run "gradle check" to generate required fixtures
     val projectId = System.getenv("GOOGLE_CLOUD_PROJECT")
         ?: "YOUR PROJECT ID"
-    val quantity = "multiple"
-    val type = "flaky"
+    val quantity = "single"
+    val type = "robo"
 
     // Bugsnag keeps the process alive so we must call exitProcess
     // https://github.com/bugsnag/bugsnag-java/issues/151

diff --git a/test_runner/src/test/kotlin/ftl/fixtures/test_app_cases/flank-single-robo.yml b/test_runner/src/test/kotlin/ftl/fixtures/test_app_cases/flank-single-robo.yml
@@ -5,3 +5,4 @@ gcloud:
     coverage: true
     coverageFilePath: /sdcard/
     clearPackageData: true
+  timeout: 5m
diff --git a/test_runner/src/test/kotlin/ftl/json/MatrixMapTest.kt b/test_runner/src/test/kotlin/ftl/json/MatrixMapTest.kt
@@ -25,7 +25,7 @@ class MatrixMapTest {
 
     private fun matrixForExecution(executionId: Int): SavedMatrix {
         return SavedMatrix(
-            TestMatrix()
+            matrix = TestMatrix()
                 .setResultStorage(createResultsStorage())
                 .setState(MatrixState.FINISHED)
                 .setTestMatrixId("123")