Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI-1365: Warn when logs need to be created #1832

Merged
merged 4 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"symfony/yaml": "^6.3",
"thecodingmachine/safe": "^2.4",
"typhonius/acquia-logstream": "^0.0.14",
"typhonius/acquia-php-sdk-v2": "^3.3.2",
"typhonius/acquia-php-sdk-v2": "^3.4.0",
"vlucas/phpdotenv": "^5.5",
"zumba/amplitude-php": "^1.0.4"
},
Expand Down
206 changes: 105 additions & 101 deletions composer.lock

Large diffs are not rendered by default.

6 changes: 4 additions & 2 deletions src/Command/Api/ApiBaseCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ protected function interact(InputInterface $input, OutputInterface $output): voi
/**
* @throws \Acquia\Cli\Exception\AcquiaCliException
* @throws \JsonException
* @throws \AcquiaCloudApi\Exception\ApiErrorException
*/
protected function execute(InputInterface $input, OutputInterface $output): int
{
Expand All @@ -122,8 +123,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$response = $acquiaCloudClient->request($this->method, $path);
$exitCode = 0;
} catch (ApiErrorException $exception) {
// Ignore PhpStorm warning here.
// @see https://youtrack.jetbrains.com/issue/WI-77190/Exception-is-never-thrown-when-thrown-from-submethod
if ($input->isInteractive()) {
throw $exception;
}
$response = $exception->getResponseBody();
$exitCode = 1;
}
Expand Down
8 changes: 4 additions & 4 deletions src/EventListener/ExceptionListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Symfony\Component\Console\Event\ConsoleErrorEvent;
use Symfony\Component\Console\Exception\CommandNotFoundException;
use Symfony\Component\Console\Exception\RuntimeException;
use TypeError;

/**
* Make exceptions warm and cuddly.
Expand Down Expand Up @@ -82,6 +83,9 @@ public function onConsoleError(ConsoleErrorEvent $event): void
}

if ($error instanceof ApiErrorException) {
if (($command = $event->getCommand()) && $error->getResponseBody()->error === 'not_found' && $command->getName() === 'api:environments:log-download') {
$this->helpMessages[] = "You must create logs (api:environments:log-create) prior to downloading them";
}
switch ($errorMessage) {
case "There are no available Cloud IDEs for this application.\n":
$this->helpMessages[] = "Delete an existing IDE via <bg=$this->messagesBgColor;fg=$this->messagesFgColor;options=bold>acli ide:delete</> or contact your Account Manager or Acquia Sales to purchase additional IDEs.";
Expand All @@ -96,10 +100,6 @@ public function onConsoleError(ConsoleErrorEvent $event): void
}
}

if ($error instanceof \TypeError && str_contains($error->getMessage(), 'AcquiaCloudApi\Response')) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type errors should not be tolerated: typhonius/acquia-php-sdk-v2#477

$newErrorMessage = 'Cloud Platform API returned an unexpected data type. This could be due to missing permissions or a problem with your Cloud Platform application.';
}

if (!empty($this->helpMessages)) {
$this->helpMessages[0] = '<options=bold>How to fix it:</> ' . $this->helpMessages[0];
}
Expand Down
4 changes: 2 additions & 2 deletions tests/phpunit/src/CommandTestBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ protected function setCommand(CommandBase $command): void
* An array of strings representing each input passed to the command input
* stream.
*/
protected function executeCommand(array $args = [], array $inputs = [], int $verbosity = Output::VERBOSITY_VERY_VERBOSE): void
protected function executeCommand(array $args = [], array $inputs = [], int $verbosity = OutputInterface::VERBOSITY_VERY_VERBOSE, ?bool $interactive = true): void
{
$cwd = $this->projectDir;
$tester = $this->getCommandTester();
Expand All @@ -96,7 +96,7 @@ protected function executeCommand(array $args = [], array $inputs = [], int $ver
}

try {
$tester->execute($args, ['verbosity' => $verbosity]);
$tester->execute($args, ['verbosity' => $verbosity, 'interactive' => $interactive]);
} catch (Exception $e) {
if (getenv('ACLI_PRINT_COMMAND_OUTPUT')) {
print $this->getDisplay();
Expand Down
33 changes: 31 additions & 2 deletions tests/phpunit/src/Commands/Api/ApiCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Acquia\Cli\Tests\CommandTestBase;
use AcquiaCloudApi\Exception\ApiErrorException;
use Symfony\Component\Console\Exception\MissingInputException;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Filesystem\Path;
use Symfony\Component\Yaml\Yaml;

Expand Down Expand Up @@ -75,6 +76,30 @@ public function testArgumentsInteraction(): void
$this->assertStringContainsString('Select a value for logType', $output);
}

/**
* @throws \AcquiaCloudApi\Exception\ApiErrorException
* @throws \JsonException
* @throws \Exception
*/
public function testInteractiveException(): void
{
$this->command = $this->getApiCommandByName('api:environments:log-download');
$this->clientProphecy->addOption('headers', ['Accept' => 'application/hal+json, version=2'])
->shouldBeCalled();
$mockBody = self::getMockResponseFromSpec($this->command->getPath(), $this->command->getMethod(), '404');
$this->clientProphecy->request('get', '/environments/289576-53785bca-1946-4adc-a022-e50d24686c20/logs/apache-access')
->willThrow(new ApiErrorException($mockBody->{'Not found'}->value))
->shouldBeCalled();
$this->expectException(ApiErrorException::class);
$this->executeCommand([], [
'289576-53785bca-1946-4adc-a022-e50d24686c20',
'apache-access',
]);
}

/**
* @throws \Exception
*/
public function testArgumentsInteractionValidation(): void
{
$this->command = $this->getApiCommandByName('api:environments:variable-update');
Expand Down Expand Up @@ -105,6 +130,10 @@ public function testArgumentsInteractionValidationFormat(): void

/**
* Tests invalid UUID.
*
* @throws \JsonException
* @throws \AcquiaCloudApi\Exception\ApiErrorException
* @throws \Exception
*/
public function testApiCommandErrorResponse(): void
{
Expand All @@ -117,7 +146,7 @@ public function testApiCommandErrorResponse(): void
->willThrow(new ApiErrorException($mockBody))
->shouldBeCalled();

// ApiCommandBase::convertApplicationAliastoUuid() will try to convert the invalid string to a uuid:
// ApiCommandBase::convertApplicationAliasToUuid() will try to convert the invalid string to a UUID:
$this->clientProphecy->addQuery('filter', 'hosting=@*:' . $invalidUuid);
$this->clientProphecy->request('get', '/applications')->willReturn([]);

Expand All @@ -128,7 +157,7 @@ public function testApiCommandErrorResponse(): void
'0',
// Would you like to link the Cloud application Sample application to this repository?
'n',
]);
], \Symfony\Component\Console\Output\OutputInterface::VERBOSITY_VERY_VERBOSE, false);

// Assert.
$output = $this->getDisplay();
Expand Down
3 changes: 0 additions & 3 deletions tests/phpunit/src/Commands/Remote/DrushCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@ public static function providerTestRemoteDrushCommand(): array
return [
[
[
'-vvv' => '',
'drush_command' => 'status --fields=db-status',
],
],
[
[
'-vvv' => '',
'drush_command' => 'status --fields=db-status',
],
],
Expand All @@ -42,7 +40,6 @@ public static function providerTestRemoteDrushCommand(): array

/**
* @dataProvider providerTestRemoteDrushCommand
* @group serial
*/
public function testRemoteDrushCommand(array $args): void
{
Expand Down
11 changes: 11 additions & 0 deletions tests/phpunit/src/Misc/ExceptionListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public function testHelp(Throwable $error, string|array $helpText): void
$applicationProphecy->setHelpMessages($messages)->shouldBeCalled();
$commandProphecy->getApplication()
->willReturn($applicationProphecy->reveal());
$commandProphecy->getName()->willReturn('api:environments:log-download');
$consoleErrorEvent = new ConsoleErrorEvent($this->input, $this->output, $error, $commandProphecy->reveal());
$exceptionListener->onConsoleError($consoleErrorEvent);
$this->prophet->checkPredictions();
Expand Down Expand Up @@ -137,6 +138,16 @@ public static function providerTestHelp(): array
]),
'You can learn more about Cloud Platform API at https://docs.acquia.com/cloud-platform/develop/api/',
],
[
new ApiErrorException((object) [
'error' => 'not_found',
'message' => 'fdsa',
]),
[
'You must create logs (api:environments:log-create) prior to downloading them',
'You can learn more about Cloud Platform API at https://docs.acquia.com/cloud-platform/develop/api/',
],
],
];
}
}
1 change: 0 additions & 1 deletion tests/phpunit/src/Misc/LocalMachineHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public function testExecuteFromCmd(bool $interactive, bool|null $isTty, bool|nul
{
$localMachineHelper = $this->localMachineHelper;
$localMachineHelper->setIsTty($isTty);
$this->input->setInteractive($interactive);
$process = $localMachineHelper->executeFromCmd('echo "hello world"', null, null, $printOutput);
$this->assertTrue($process->isSuccessful());
assert(is_a($this->output, BufferedOutput::class));
Expand Down
4 changes: 4 additions & 0 deletions tests/phpunit/src/TestBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ public static function getMockResponseFromSpec(mixed $path, mixed $method, mixed

/**
* @return array<mixed>
* @throws \Psr\Cache\InvalidArgumentException
*/
protected function getPathMethodCodeFromSpec(string $operationId): array
{
Expand Down Expand Up @@ -473,6 +474,9 @@ protected function createMockAcliConfigFile(mixed $cloudAppUuid = null): void
*
* Auto-completion and return type inferencing is provided by
* .phpstorm.meta.php.
*
* @throws \JsonException
* @throws \Exception
*/
protected function mockRequest(string $operationId, string|array|null $params = null, ?array $body = null, ?string $exampleResponse = null, Closure $tamper = null): object|array
{
Expand Down