From ea737c9f721434ac218883d6f5f62711a028fe98 Mon Sep 17 00:00:00 2001 From: Bartek Pacia Date: Thu, 22 Aug 2024 20:02:11 +0100 Subject: [PATCH] code discovery (add comments, renames) --- .../java/maestro/cli/command/RecordCommand.kt | 4 +-- .../java/maestro/cli/command/TestCommand.kt | 7 ++-- .../cli/runner/MaestroCommandRunner.kt | 7 +++- .../{TestRunner.kt => MaestroFlowRunner.kt} | 33 ++++++++++--------- .../maestro/cli/runner/TestSuiteInteractor.kt | 2 -- .../main/java/maestro/orchestra/Orchestra.kt | 13 ++++++-- 6 files changed, 40 insertions(+), 26 deletions(-) rename maestro-cli/src/main/java/maestro/cli/runner/{TestRunner.kt => MaestroFlowRunner.kt} (87%) diff --git a/maestro-cli/src/main/java/maestro/cli/command/RecordCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/RecordCommand.kt index 57772b532f..1aabdd029c 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/RecordCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/RecordCommand.kt @@ -25,7 +25,7 @@ import maestro.cli.DisableAnsiMixin import maestro.cli.ShowHelpMixin import maestro.cli.api.ApiClient import maestro.cli.report.TestDebugReporter -import maestro.cli.runner.TestRunner +import maestro.cli.runner.MaestroFlowRunner import maestro.cli.runner.resultview.AnsiResultView import maestro.cli.session.MaestroSessionManager import maestro.cli.view.ProgressBar @@ -104,7 +104,7 @@ class RecordCommand : Callable { val screenRecording = kotlin.io.path.createTempFile(suffix = ".mp4").toFile() val exitCode = screenRecording.sink().use { out -> maestro.startScreenRecording(out).use { - TestRunner.runSingle(maestro, device, flowFile, env, resultView, path) + MaestroFlowRunner.runSingle(maestro, device, flowFile, env, resultView, path) } } diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index 54898d33ff..14eeb56358 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -37,7 +37,7 @@ import maestro.cli.model.TestExecutionSummary import maestro.cli.report.ReportFormat import maestro.cli.report.ReporterFactory import maestro.cli.report.TestDebugReporter -import maestro.cli.runner.TestRunner +import maestro.cli.runner.MaestroFlowRunner import maestro.cli.runner.TestSuiteInteractor import maestro.cli.runner.resultview.AnsiResultView import maestro.cli.runner.resultview.PlainTextResultView @@ -297,12 +297,13 @@ class TestCommand : Callable { if (!flattenDebugOutput) { TestDebugReporter.deleteOldFiles() } - TestRunner.runContinuous(maestro, device, flowFile, env) + MaestroFlowRunner.runContinuous(maestro, device, flowFile, env) + } else { val resultView = if (DisableAnsiMixin.ansiEnabled) AnsiResultView() else PlainTextResultView() - val resultSingle = TestRunner.runSingle( + val resultSingle = MaestroFlowRunner.runSingle( maestro, device, flowFile, diff --git a/maestro-cli/src/main/java/maestro/cli/runner/MaestroCommandRunner.kt b/maestro-cli/src/main/java/maestro/cli/runner/MaestroCommandRunner.kt index 8fac330fbf..cdcda16ee3 100644 --- a/maestro-cli/src/main/java/maestro/cli/runner/MaestroCommandRunner.kt +++ b/maestro-cli/src/main/java/maestro/cli/runner/MaestroCommandRunner.kt @@ -40,6 +40,11 @@ import org.slf4j.LoggerFactory import java.io.File import java.util.IdentityHashMap +/** + * Knows how to run a list of Maestro commands. + * + * Does not know what a "flow" is. + */ object MaestroCommandRunner { private val logger = LoggerFactory.getLogger(MaestroCommandRunner::class.java) @@ -125,7 +130,7 @@ object MaestroCommandRunner { refreshUi() val orchestra = Orchestra( - maestro, + maestro = maestro, onCommandStart = { _, command -> logger.info("${command.description()} RUNNING") commandStatuses[command] = CommandStatus.RUNNING diff --git a/maestro-cli/src/main/java/maestro/cli/runner/TestRunner.kt b/maestro-cli/src/main/java/maestro/cli/runner/MaestroFlowRunner.kt similarity index 87% rename from maestro-cli/src/main/java/maestro/cli/runner/TestRunner.kt rename to maestro-cli/src/main/java/maestro/cli/runner/MaestroFlowRunner.kt index 2aafed0eb2..f89ff8509c 100644 --- a/maestro-cli/src/main/java/maestro/cli/runner/TestRunner.kt +++ b/maestro-cli/src/main/java/maestro/cli/runner/MaestroFlowRunner.kt @@ -26,9 +26,12 @@ import java.io.File import java.nio.file.Path import kotlin.concurrent.thread -object TestRunner { +/** + * Knows how to run a single Maestro flow. + */ +object MaestroFlowRunner { - private val logger = LoggerFactory.getLogger(TestRunner::class.java) + private val logger = LoggerFactory.getLogger(MaestroFlowRunner::class.java) fun runSingle( maestro: Maestro, @@ -53,12 +56,12 @@ object TestRunner { } MaestroCommandRunner.runCommands( - maestro, - device, - resultView, - commands, - debugOutput, - aiOutput, + maestro = maestro, + device = device, + view = resultView, + commands = commands, + debugOutput = debugOutput, + aiOutput = aiOutput, ) } @@ -119,13 +122,13 @@ object TestRunner { previousResult = runCatching(resultView, maestro) { MaestroCommandRunner.runCommands( - maestro, - device, - resultView, - commands, - FlowDebugOutput(), - // TODO: bartekpacia - make AI outputs work in continuous mode - FlowAIOutput( + maestro = maestro, + device = device, + view = resultView, + commands = commands, + debugOutput = FlowDebugOutput(), + // TODO(bartekpacia): make AI outputs work in continuous mode + aiOutput = FlowAIOutput( flowName = "TODO", flowFile = flowFile, ), diff --git a/maestro-cli/src/main/java/maestro/cli/runner/TestSuiteInteractor.kt b/maestro-cli/src/main/java/maestro/cli/runner/TestSuiteInteractor.kt index 07529de30f..e101663b21 100644 --- a/maestro-cli/src/main/java/maestro/cli/runner/TestSuiteInteractor.kt +++ b/maestro-cli/src/main/java/maestro/cli/runner/TestSuiteInteractor.kt @@ -10,7 +10,6 @@ import maestro.cli.report.SingleScreenFlowAIOutput import maestro.cli.report.CommandDebugMetadata import maestro.cli.report.FlowAIOutput import maestro.cli.report.FlowDebugOutput -import maestro.cli.report.HtmlAITestSuiteReporter import maestro.cli.report.TestDebugReporter import maestro.cli.report.TestSuiteReporter import maestro.cli.util.PrintUtils @@ -233,7 +232,6 @@ class TestSuiteInteractor( it.status = CommandStatus.PENDING } }, - // another name idea: onCommandFoundDefects onCommandGeneratedOutput = { command, defects, screenshot -> logger.info("${command.description()} generated output") val screenshotPath = writeAIscreenshot(screenshot) diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/Orchestra.kt b/maestro-orchestra/src/main/java/maestro/orchestra/Orchestra.kt index 4497c18abe..24f7903bb5 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/Orchestra.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/Orchestra.kt @@ -59,6 +59,13 @@ sealed class CommandOutput { data class AIDefects(val defects: List, val screenshot: Buffer) : CommandOutput() } +/** + * Orchestra . It's is one of the core classes in the Maestro codebase.s + * + * Orchestra should not know about: + * - File systems. It should instead write to [Sink]s that it requests from the caller. + * - + */ class Orchestra( private val maestro: Maestro, private val stateDir: File? = null, @@ -223,7 +230,6 @@ class Orchestra( onCommandComplete(index, command) } catch (ignored: CommandSkipped) { // Swallow exception - println("ignored CommandSkipped") onCommandSkipped(index, command) } catch (e: Throwable) { @@ -379,7 +385,7 @@ class Orchestra( aiClient = ai, assertion = command.assertion, screen = imageData.copy().readByteArray(), - previousFalsePositives = listOf(), // TODO: take it from WorkspaceConfig (or MaestroConfig?) + previousFalsePositives = listOf(), // TODO(bartekpacia): take it from WorkspaceConfig (or MaestroConfig?) ) if (defects.isNotEmpty()) { @@ -387,8 +393,9 @@ class Orchestra( if (command.optional) throw CommandSkipped + val word = if (defects.size == 1) "defect" else "defects" throw MaestroException.AssertionFailure( - "Visual AI found possible defects:\n ${defects.joinToString("\n ") { "${it.category}: ${it.reasoning}" }}", + "Visual AI found ${defects.size} possible $word. See the report to learn more.", maestro.viewHierarchy().root, ) }