MBS-11465 runner execution timeout #1165
Conversation
| @@ -129,7 +129,7 @@ internal class DeviceWorker( | |||
| stateWorker.clearPackages(currentState) | |||
| }.map { intendedState } | |||
|
|
|||
| private fun executeAction(action: InstrumentationTestRunAction): Result<DeviceTestCaseRun> { | |||
| private suspend fun executeAction(action: InstrumentationTestRunAction): Result<DeviceTestCaseRun> { | |||
There was a problem hiding this comment.
Why do we need the suspend there?
| action: InstrumentationTestRunAction, | ||
| outputDir: File | ||
| ): DeviceTestCaseRun { | ||
| // simulating test execution |
There was a problem hiding this comment.
Propose to delete this obvious comment
| @@ -17,7 +17,11 @@ public abstract class InstrumentationConfiguration(public val name: String) { | |||
|
|
|||
| public var kubernetesNamespace: String = "default" | |||
|
|
|||
| public var timeoutInSeconds: Long = TimeUnit.MINUTES.toSeconds(100) | |||
| public var timeoutInSeconds: Long = 120L // TODO: remove after MBS-11465 | |||
| @@ -284,7 +284,8 @@ public class InstrumentationTestsPlugin : Plugin<Project> { | |||
| kubernetesNamespace = configuration.kubernetesNamespace, | |||
| targets = getTargets(configuration, mergedInstrumentationParameters), | |||
| enableDeviceDebug = configuration.enableDeviceDebug, | |||
| timeoutInSeconds = configuration.timeoutInSeconds, | |||
| testRunnerExecutionTimeout = configuration.testRunnerExecutionTimeout, | |||
| gradleTaskTimeout = configuration.gradleTaskTimeout, | |||
There was a problem hiding this comment.
instrumentationTaskTimeout
gradleTask is too general description
| @@ -65,6 +69,8 @@ internal class RunnerIntegrationTest { | |||
| minimumFailedCount = 0 | |||
| ) | |||
| private val loggerFactory = StubLoggerFactory | |||
| private val <E> Channel<E>.isClosedForSendAndReceive: Boolean | |||
There was a problem hiding this comment.
Let's move it to place where it's used
There was a problem hiding this comment.
Moved this extension to :common:coroutines-extension. It seems to be the correct place for this property.
| logger.critical("Test run end with error", e) | ||
| Result.Failure(e) | ||
| } finally { | ||
| logger.debug("Test run finally block called") |
| testSuiteListener.onTestSuiteFinished() | ||
| logger.info("Test run end successfully") | ||
| Result.Success(result) | ||
| } catch (e: Throwable) { |
There was a problem hiding this comment.
Will we get the TimeoutException there? If will maybe make the different log message
There was a problem hiding this comment.
Yes, we will. Changed the message for timeout
There was a problem hiding this comment.
Expect to see multiple catch blocks but you decide do you want to rewrite this code again
No description provided.