From 212f5f8596c6e655846a5bf200271494ea478124 Mon Sep 17 00:00:00 2001 From: Veronika Tolkachova Date: Tue, 11 Aug 2026 13:12:39 +0200 Subject: [PATCH 1/2] fix: error nicely when task not found --- integration-tests/task_test.go | 123 ++++++++++++++++++++ legacy/src/Command/Task/TaskListCommand.php | 14 +-- legacy/src/Command/Task/TaskRunCommand.php | 28 ++++- legacy/src/Service/Api.php | 22 ++++ 4 files changed, 174 insertions(+), 13 deletions(-) create mode 100644 integration-tests/task_test.go diff --git a/integration-tests/task_test.go b/integration-tests/task_test.go new file mode 100644 index 00000000..a2eefbb7 --- /dev/null +++ b/integration-tests/task_test.go @@ -0,0 +1,123 @@ +package tests + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" +) + +func TestTaskRun(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + + projectID := mockapi.ProjectID() + envPath := "/projects/" + projectID + "/environments/main" + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + ), + DefaultBranch: "main", + }}) + + apiHandler.SetEnvironments([]*mockapi.Environment{ + makeEnv(projectID, "main", "staging", "active", nil), + }) + + apiHandler.Get(envPath+"/tasks", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode([]any{ + map[string]any{ + "name": "migrate", + "type": "app", + "run": map[string]any{ + "command": "php migrate.php", + "timeout": 300, + }, + }, + }) + }) + + var runCalled bool + apiHandler.Post(envPath+"/tasks/migrate/run", func(w http.ResponseWriter, _ *http.Request) { + runCalled = true + _ = json.NewEncoder(w).Encode(map[string]any{ + "_embedded": map[string]any{"activities": []any{}}, + }) + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + t.Run("list", func(t *testing.T) { + stdout, stderr, err := f.RunCombinedOutput("task:list", "-p", projectID, "-e", "main", "--format", "plain") + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + assert.Contains(t, stdout, "migrate") + assert.Contains(t, stdout, "php migrate.php") + }) + + t.Run("run", func(t *testing.T) { + stdout, stderr, err := f.RunCombinedOutput("task:run", "migrate", "-p", projectID, "-e", "main", "--yes") + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + assert.Contains(t, stderr, "The task has been triggered.") + assert.True(t, runCalled, "the task run endpoint was not called") + }) + + t.Run("not_found", func(t *testing.T) { + stdout, stderr, err := f.RunCombinedOutput("task:run", "does-not-exist", "-p", projectID, "-e", "main", "--yes") + assert.Error(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + assert.Contains(t, stderr, "The task does-not-exist was not found on the environment") + assert.Contains(t, stderr, "To list tasks, run") + assert.NotContains(t, stderr, "RequestException") + assert.NotContains(t, stderr, "resulted in a") + }) +} + +// A project with no tasks defined at all should say so, rather than reporting +// the requested task as missing. +func TestTaskRunNoTasks(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + + projectID := mockapi.ProjectID() + envPath := "/projects/" + projectID + "/environments/main" + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + ), + DefaultBranch: "main", + }}) + + apiHandler.SetEnvironments([]*mockapi.Environment{ + makeEnv(projectID, "main", "staging", "active", nil), + }) + + apiHandler.Get(envPath+"/tasks", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode([]any{}) + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + stdout, stderr, err := f.RunCombinedOutput("task:run", "migrate", "-p", projectID, "-e", "main", "--yes") + assert.Error(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + assert.Contains(t, stderr, "No tasks were found on the environment") +} diff --git a/legacy/src/Command/Task/TaskListCommand.php b/legacy/src/Command/Task/TaskListCommand.php index 40c94303..70afbb21 100644 --- a/legacy/src/Command/Task/TaskListCommand.php +++ b/legacy/src/Command/Task/TaskListCommand.php @@ -4,13 +4,10 @@ namespace Platformsh\Cli\Command\Task; -use GuzzleHttp\Exception\BadResponseException; -use GuzzleHttp\Utils; use Platformsh\Cli\Command\CommandBase; use Platformsh\Cli\Selector\Selector; use Platformsh\Cli\Service\Api; use Platformsh\Cli\Service\Table; -use Platformsh\Client\Exception\ApiResponseException; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -44,12 +41,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $selection = $this->selector->getSelection($input); $environment = $selection->getEnvironment(); - try { - $response = $this->api->getHttpClient()->request('GET', $environment->getUri() . '/tasks'); - } catch (BadResponseException $e) { - throw ApiResponseException::create($e->getRequest(), $e->getResponse(), $e); - } - $tasks = (array) Utils::jsonDecode((string) $response->getBody(), true); + $tasks = $this->api->getEnvironmentTasks($environment); if ($tasks === []) { $this->stdErr->writeln(sprintf( @@ -61,9 +53,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $rows = []; - foreach ($tasks as $task) { + foreach ($tasks as $name => $task) { $rows[] = [ - 'name' => $task['name'] ?? '', + 'name' => $name, 'type' => $task['type'] ?? '', 'command' => isset($task['run']['command']) ? trim((string) $task['run']['command']) : '', 'timeout' => $task['run']['timeout'] ?? '', diff --git a/legacy/src/Command/Task/TaskRunCommand.php b/legacy/src/Command/Task/TaskRunCommand.php index bd7a203a..0650a9e2 100644 --- a/legacy/src/Command/Task/TaskRunCommand.php +++ b/legacy/src/Command/Task/TaskRunCommand.php @@ -5,7 +5,6 @@ namespace Platformsh\Cli\Command\Task; use GuzzleHttp\Exception\BadResponseException; -use GuzzleHttp\Utils; use Platformsh\Cli\Command\CommandBase; use Platformsh\Cli\Model\Variable; use Platformsh\Cli\Selector\Selector; @@ -54,6 +53,31 @@ protected function execute(InputInterface $input, OutputInterface $output): int $taskName = $input->getArgument('task'); $variables = (new Variable())->parseMultiple($input->getOption('variable')); + $tasks = $this->api->getEnvironmentTasks($environment); + if (!isset($tasks[$taskName])) { + if ($tasks === []) { + $this->stdErr->writeln(sprintf( + 'No tasks were found on the environment %s.', + $this->api->getEnvironmentLabel($environment, 'comment'), + )); + + return 1; + } + + $this->stdErr->writeln(sprintf( + 'The task %s was not found on the environment %s.', + $taskName, + $this->api->getEnvironmentLabel($environment, 'comment'), + )); + $this->stdErr->writeln(''); + $this->stdErr->writeln(sprintf( + 'To list tasks, run: %s tasks', + $this->config->getStr('application.executable'), + )); + + return 1; + } + if ($environment->type === 'production' && !$this->questionHelper->confirm(sprintf( 'Are you sure you want to run the task %s on the production environment %s?', $taskName, @@ -76,7 +100,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $result = new Result( - (array) Utils::jsonDecode((string) $response->getBody(), true), + (array) json_decode((string) $response->getBody(), true, 512, JSON_THROW_ON_ERROR), $environment->getUri(), $this->api->getHttpClient(), Activity::class, diff --git a/legacy/src/Service/Api.php b/legacy/src/Service/Api.php index 1bd15d9a..b5858f9e 100644 --- a/legacy/src/Service/Api.php +++ b/legacy/src/Service/Api.php @@ -1825,6 +1825,28 @@ public function setAutoscalingSettings(Environment $environment, array $settings } } + /** + * Returns the tasks defined on an environment, keyed by task name. + * + * @return array> + */ + public function getEnvironmentTasks(Environment $environment): array + { + try { + $response = $this->getHttpClient()->request('GET', $environment->getUri() . '/tasks'); + } catch (BadResponseException $e) { + throw ApiResponseException::create($e->getRequest(), $e->getResponse(), $e); + } + $tasks = (array) json_decode((string) $response->getBody(), true, 512, JSON_THROW_ON_ERROR); + + $byName = []; + foreach ($tasks as $key => $task) { + $byName[(string) ($task['name'] ?? $key)] = $task; + } + + return $byName; + } + /** * Warn the user if a project is suspended. * From 9a218aedc9afdda5dd93307ff7748b6e43bcc596 Mon Sep 17 00:00:00 2001 From: Veronika Tolkachova Date: Tue, 11 Aug 2026 13:22:46 +0200 Subject: [PATCH 2/2] feat: allow to choose from a list of tasks --- integration-tests/task_test.go | 73 ++++++++++++++++++---- legacy/src/Command/Task/TaskRunCommand.php | 41 +++++++++--- 2 files changed, 93 insertions(+), 21 deletions(-) diff --git a/integration-tests/task_test.go b/integration-tests/task_test.go index a2eefbb7..66599e0b 100644 --- a/integration-tests/task_test.go +++ b/integration-tests/task_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "sync/atomic" "testing" "github.com/stretchr/testify/assert" @@ -40,20 +41,31 @@ func TestTaskRun(t *testing.T) { "name": "migrate", "type": "app", "run": map[string]any{ - "command": "php migrate.php", + // A multi-line command, to check that the chooser stays on one line. + "command": "set -e\nphp migrate.php\n", "timeout": 300, }, }, + map[string]any{ + "name": "cache-clear", + "type": "app", + "run": map[string]any{ + "command": "php cache-clear.php", + "timeout": 60, + }, + }, }) }) - var runCalled bool - apiHandler.Post(envPath+"/tasks/migrate/run", func(w http.ResponseWriter, _ *http.Request) { - runCalled = true - _ = json.NewEncoder(w).Encode(map[string]any{ - "_embedded": map[string]any{"activities": []any{}}, + var ranTask atomic.Value // string + for _, name := range []string{"migrate", "cache-clear"} { + apiHandler.Post(envPath+"/tasks/"+name+"/run", func(w http.ResponseWriter, _ *http.Request) { + ranTask.Store(name) + _ = json.NewEncoder(w).Encode(map[string]any{ + "_embedded": map[string]any{"activities": []any{}}, + }) }) - }) + } apiServer := httptest.NewServer(apiHandler) defer apiServer.Close() @@ -64,28 +76,58 @@ func TestTaskRun(t *testing.T) { stdout, stderr, err := f.RunCombinedOutput("task:list", "-p", projectID, "-e", "main", "--format", "plain") require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) assert.Contains(t, stdout, "migrate") - assert.Contains(t, stdout, "php migrate.php") + assert.Contains(t, stdout, "cache-clear") }) t.Run("run", func(t *testing.T) { + ranTask.Store("") stdout, stderr, err := f.RunCombinedOutput("task:run", "migrate", "-p", projectID, "-e", "main", "--yes") require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) assert.Contains(t, stderr, "The task has been triggered.") - assert.True(t, runCalled, "the task run endpoint was not called") + assert.Equal(t, "migrate", ranTask.Load()) + }) + + // With no task argument, the tasks are offered as a numbered list. They are + // sorted by name and numbered from 0, so choice 1 is "migrate". + t.Run("choose", func(t *testing.T) { + ranTask.Store("") + stdout, stderr, err := f.RunInteractive("1\n", "task:run", "-p", projectID, "-e", "main") + + combined := stdout + "\n---\n" + stderr + assert.NotContains(t, combined, "TypeError") + assert.NotContains(t, combined, "must be of type") + assert.NotContains(t, combined, "Fatal error") + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + + assert.Contains(t, stderr, "Enter a number to choose a task to run:") + assert.Contains(t, stderr, "cache-clear (php cache-clear.php)") + assert.Contains(t, stderr, "migrate (set -e …)") + assert.Contains(t, stderr, "The task has been triggered.") + assert.Equal(t, "migrate", ranTask.Load()) + }) + + t.Run("no_argument_non_interactive", func(t *testing.T) { + ranTask.Store("") + stdout, stderr, err := f.RunCombinedOutput("task:run", "-p", projectID, "-e", "main", "--yes") + assert.Error(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + assert.Contains(t, stderr, "The task argument is required in non-interactive mode.") + assert.Equal(t, "", ranTask.Load()) }) t.Run("not_found", func(t *testing.T) { + ranTask.Store("") stdout, stderr, err := f.RunCombinedOutput("task:run", "does-not-exist", "-p", projectID, "-e", "main", "--yes") assert.Error(t, err, "stdout: %s\nstderr: %s", stdout, stderr) assert.Contains(t, stderr, "The task does-not-exist was not found on the environment") assert.Contains(t, stderr, "To list tasks, run") assert.NotContains(t, stderr, "RequestException") assert.NotContains(t, stderr, "resulted in a") + assert.Equal(t, "", ranTask.Load()) }) } // A project with no tasks defined at all should say so, rather than reporting -// the requested task as missing. +// the requested task as missing or offering an empty list of choices. func TestTaskRunNoTasks(t *testing.T) { authServer := mockapi.NewAuthServer(t) defer authServer.Close() @@ -117,7 +159,12 @@ func TestTaskRunNoTasks(t *testing.T) { f := newCommandFactory(t, apiServer.URL, authServer.URL) - stdout, stderr, err := f.RunCombinedOutput("task:run", "migrate", "-p", projectID, "-e", "main", "--yes") - assert.Error(t, err, "stdout: %s\nstderr: %s", stdout, stderr) - assert.Contains(t, stderr, "No tasks were found on the environment") + for _, args := range [][]string{ + {"task:run", "migrate", "-p", projectID, "-e", "main", "--yes"}, + {"task:run", "-p", projectID, "-e", "main", "--yes"}, + } { + stdout, stderr, err := f.RunCombinedOutput(args...) + assert.Error(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + assert.Contains(t, stderr, "No tasks were found on the environment") + } } diff --git a/legacy/src/Command/Task/TaskRunCommand.php b/legacy/src/Command/Task/TaskRunCommand.php index 0650a9e2..2bc83511 100644 --- a/legacy/src/Command/Task/TaskRunCommand.php +++ b/legacy/src/Command/Task/TaskRunCommand.php @@ -32,7 +32,7 @@ public function __construct(private readonly ActivityMonitor $activityMonitor, p protected function configure(): void { $this - ->addArgument('task', InputArgument::REQUIRED, 'The name of the task to execute') + ->addArgument('task', InputArgument::OPTIONAL, 'The name of the task to execute') ->addOption('variable', null, InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'A variable to set when running the task, in the format type:name=value') // Tasks can run for a long time, so waiting is opt-in rather than the default. ->addOption('wait', null, InputOption::VALUE_NONE, 'Wait for the task to complete'); @@ -50,20 +50,35 @@ protected function execute(InputInterface $input, OutputInterface $output): int $selection = $this->selector->getSelection($input); $environment = $selection->getEnvironment(); - $taskName = $input->getArgument('task'); $variables = (new Variable())->parseMultiple($input->getOption('variable')); $tasks = $this->api->getEnvironmentTasks($environment); - if (!isset($tasks[$taskName])) { - if ($tasks === []) { - $this->stdErr->writeln(sprintf( - 'No tasks were found on the environment %s.', - $this->api->getEnvironmentLabel($environment, 'comment'), - )); + if ($tasks === []) { + $this->stdErr->writeln(sprintf( + 'No tasks were found on the environment %s.', + $this->api->getEnvironmentLabel($environment, 'comment'), + )); + + return 1; + } + + $taskName = $input->getArgument('task'); + if ($taskName === null) { + if (!$input->isInteractive()) { + $this->stdErr->writeln('The task argument is required in non-interactive mode.'); return 1; } + $choices = []; + foreach ($tasks as $name => $task) { + $choices[$name] = isset($task['run']['command']) + ? sprintf('%s (%s)', $name, $this->summarizeCommand((string) $task['run']['command'])) + : $name; + } + ksort($choices, SORT_NATURAL); + $taskName = $this->questionHelper->choose($choices, 'Enter a number to choose a task to run:', null, false); + } elseif (!isset($tasks[$taskName])) { $this->stdErr->writeln(sprintf( 'The task %s was not found on the environment %s.', $taskName, @@ -136,4 +151,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 0; } + + /** + * Reduces a task command to a single line, for use in a list of choices. + */ + private function summarizeCommand(string $command): string + { + $lines = (array) preg_split('/\r?\n/', trim($command)); + + return trim((string) $lines[0]) . (count($lines) > 1 ? ' …' : ''); + } }