From 76f520acbbb112e08b5a40a58ba91e95363d904d Mon Sep 17 00:00:00 2001 From: Jerry Radwick Date: Fri, 27 Dec 2024 17:27:14 -0500 Subject: [PATCH 1/5] Fix: $pCommand written as $sCommand --- src/Command/ExecCommand.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Command/ExecCommand.php b/src/Command/ExecCommand.php index 757c59f..ac96800 100644 --- a/src/Command/ExecCommand.php +++ b/src/Command/ExecCommand.php @@ -212,11 +212,11 @@ function ($value) use ($command, $placeholder, $output, $progressBar, &$exitCode return; } - $sCommand = Placeholder::replace([$placeholder->value => $value], $command); - $process = new Process("($sCommand) 2>&1", getcwd()); + $pCommand = Placeholder::replace([$placeholder->value => $value], $command); + $process = new Process("($pCommand) 2>&1", getcwd()); yield $process->start(); - $this->logger->debug('Running: {command}', ['command' => $sCommand]); + $this->logger->debug('Running: {command}', ['command' => $pCommand]); // @todo Improve formatting of headings. $pOutput = yield ByteStream\buffer($process->getStdout()); From acbc1170f764d80b9655d039ecc2cd1f949a0fcd Mon Sep 17 00:00:00 2001 From: Jerry Radwick Date: Sat, 28 Dec 2024 08:57:21 -0500 Subject: [PATCH 2/5] Fix: 'shell command' written as 'drush command' --- src/Command/ExecCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Command/ExecCommand.php b/src/Command/ExecCommand.php index ac96800..20e86d1 100644 --- a/src/Command/ExecCommand.php +++ b/src/Command/ExecCommand.php @@ -63,7 +63,7 @@ protected function configure() { $this->addArgument( 'cmd', InputArgument::REQUIRED | InputArgument::IS_ARRAY, - 'A drush command.' + 'A shell command.' ); $this->addOption( From 874fdfa9c1c9183a8e54eb52a4ee31bc732f9e76 Mon Sep 17 00:00:00 2001 From: Jerry Radwick Date: Sat, 28 Dec 2024 09:12:28 -0500 Subject: [PATCH 3/5] Add option --interval; improve validation; refs #83 --- README.md | 39 ++--- src/Command/ExecCommand.php | 141 +++++++++++++------ test/Integration/Command/ExecCommandTest.php | 68 ++++++++- 3 files changed, 183 insertions(+), 65 deletions(-) diff --git a/README.md b/README.md index 2e4a5c4..9a19b74 100644 --- a/README.md +++ b/README.md @@ -108,7 +108,7 @@ interrupt signal is received, then Drall stops immediately. In this method, the `--uri` option is sent to `drush`. - drall exec drush --uri=@@dir core:status + drall exec -- drush --uri=@@dir core:status If it is a Drush command and no valid `@@placeholder` are present, then `--uri=@@dir` is automatically added after each occurrence of `drush`. @@ -117,7 +117,7 @@ If it is a Drush command and no valid `@@placeholder` are present, then # Raw drush command (no placeholders) drall exec drush core:status # Command that is executed (placeholders injected) -drall exec drush --uri=@@dir core:status +drall exec -- drush --uri=@@dir core:status ``` ##### Example @@ -174,17 +174,16 @@ cat web/sites/ralph/settings.local.php #### Options For the `drall exec` command, all Drall options must be set right after -`drall exec`. For example, +`drall exec`. Additionally, `--` must be used before the command to be +executed. Following are some examples of running `drush` with options. ```shell -# Correct: Drall gets --verbose. -drall exec --verbose drush core:status -# Incorrect: --verbose is ignored. -drall exec drush core:status --verbose -# Correct: Drall and Drush, both --verbose. +# Drall is --verbose +drall exec --verbose -- drush core:status +# Drush is verbose +drall exec -- drush --verbose core:status +# Both Drall and Drush are --verbose drall exec --verbose -- drush --verbose core:status -# Incorrect: Only Drall gets --verbose. -drall exec --verbose drush core:status ``` In summary, the syntax is as follows: @@ -195,6 +194,14 @@ drall exec [DRALL-OPTIONS] -- drush [DRUSH-OPTIONS] Besides the global options, the `exec` command supports the following options. +#### --interval + +This option makes Drall wait for `n` seconds after processing each item. + + drall exec --interval=3 -- drush core:rebuild + +Such an interval cannot be used when using a multiple workers. + #### --workers Say you have 100 sites in a Drupal installation. By default, Drall runs @@ -213,7 +220,7 @@ conflict between the Drall workers. The command below launches 3 instances of Drall to run `core:rebuild` command. - drall exec drush core:rebuild --workers=3 + drall exec --workers=3 -- drush core:rebuild When a worker runs out of work, it terminates automatically. @@ -229,7 +236,7 @@ bar can be disabled using the `--no-progress` option. ##### Example: Hide progress bar - drall exec --no-progress drush core:rebuild + drall exec --no-progress -- drush core:rebuild #### --dry-run @@ -239,7 +246,7 @@ executing them. ##### Example: Dry run ```shell -$ drall exec --dry-run --group=bluish core:status +$ drall exec --dry-run --group=bluish -- drush core:status drush --uri=donnie core:status drush --uri=leo core:status ``` @@ -333,7 +340,7 @@ This section covers some options that are supported by all `drall` commands. Specify the target site group. See the section *site groups* for more information on site groups. - drall exec --group=GROUP core:status --field=site + drall exec --group=GROUP -- drush core:status --field=site If `--group` is not set, then the Drall uses the environment variable `DRALL_GROUP`, if it is set. @@ -345,9 +352,9 @@ commands on specific sites. ```shell # Run only on the "leo" site. -drall exec --filter=leo core:status +drall exec --filter=leo -- drush core:status # Run only on "leo" and "ralph" sites. -drall exec --filter="leo||ralph" core:status +drall exec --filter="leo||ralph" -- drush core:status ``` For more on using filter expressions, refer to the documentation on diff --git a/src/Command/ExecCommand.php b/src/Command/ExecCommand.php index 20e86d1..ce4d04a 100644 --- a/src/Command/ExecCommand.php +++ b/src/Command/ExecCommand.php @@ -74,6 +74,14 @@ protected function configure() { 1, ); + $this->addOption( + 'interval', + NULL, + InputOption::VALUE_OPTIONAL, + 'Number of seconds to wait between commands.', + 0, + ); + $this->addOption( 'dry-run', 'X', @@ -92,26 +100,22 @@ protected function configure() { } protected function initialize(InputInterface $input, OutputInterface $output): void { - if (!method_exists($input, 'getRawTokens')) { - parent::initialize($input, $output); - return; - } + $this->checkObsoleteOptions($input, $output); + $this->checkOptionsSeparator($input, $output); + $this->checkIntervalOption($input, $output); + $this->checkWorkersOption($input, $output); + $this->checkInterOptionCompatibility($input, $output); - // Parts of the command after "exec". - $rawTokens = $input->getRawTokens(TRUE); + parent::initialize($input, $output); + } - // If obsolete --drall-* options are present, then abort. - foreach ($rawTokens as $token) { - if (str_starts_with($token, '--drall-')) { - $output->writeln(<<getRawTokens(TRUE); if (!in_array('--', $rawTokens)) { foreach ($rawTokens as $token) { if (str_starts_with($token, '-')) { @@ -127,8 +131,71 @@ protected function initialize(InputInterface $input, OutputInterface $output): v } } } + } - parent::initialize($input, $output); + private function checkObsoleteOptions(InputInterface $input, OutputInterface $output): void { + if (!method_exists($input, 'getRawTokens')) { + return; + } + + // If obsolete --drall-* options are present, then abort. + foreach ($input->getRawTokens(TRUE) as $token) { + if (str_starts_with($token, '--drall-')) { + $output->writeln(<<--drall-* options have been renamed. +See https://github.com/jigarius/drall/issues/99 +EOT); + throw new \RuntimeException('Obsolete options detected'); + } + } + } + + private function checkIntervalOption(InputInterface $input, OutputInterface $output): void { + $interval = $input->getOption('interval'); + + if ($interval < 0) { + $output->writeln(<<--interval must be a positive integer. +EOT); + throw new \RuntimeException('Invalid options detected'); + } + } + + private function checkWorkersOption(InputInterface $input, OutputInterface $output): void { + $workers = $input->getOption('workers'); + + if ($workers > self::WORKER_LIMIT) { + $limit = self::WORKER_LIMIT; + $output->writeln(<<--workers must be less than or equal to $limit. +EOT); + throw new \RuntimeException('Invalid options detected'); + } + } + + private function checkInterOptionCompatibility(InputInterface $input, OutputInterface $output): void { + if ( + $input->getOption('workers') > 1 && + $input->getOption('interval') > 0 + ) { + $output->writeln(<<--interval and --workers cannot be used together. +EOT); + throw new \RuntimeException('Incompatible options detected'); + } + } + + protected function preExecute(InputInterface $input, OutputInterface $output): void { + parent::preExecute($input, $output); + + $workers = $input->getOption('workers'); + if ($workers > 1) { + $this->logger->notice("Using {count} workers.", ['count' => $workers]); + } + + if ($interval = $input->getOption('interval')) { + $this->logger->notice("Using a $interval-second interval between commands.", ['interval' => $interval]); + } } protected function execute(InputInterface $input, OutputInterface $output): int { @@ -159,7 +226,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 0; } - $workers = $this->getWorkerCount($input); + $workers = $workers = $input->getOption('workers'); // Display commands without executing them. if ($input->getOption('dry-run')) { @@ -198,6 +265,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $values, $command, $placeholder, + $input, $output, $progressBar, $workers, @@ -207,7 +275,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int yield ConcurrentIterator\each( Iterator\fromIterable($values), new LocalSemaphore($workers), - function ($value) use ($command, $placeholder, $output, $progressBar, &$exitCode, &$isStopping) { + function ($value) use ( + $command, + $placeholder, + $input, + $output, + $progressBar, + &$exitCode, + &$isStopping, + ) { if ($isStopping) { return; } @@ -237,6 +313,11 @@ function ($value) use ($command, $placeholder, $output, $progressBar, &$exitCode $progressBar->advance(); $progressBar->display(); + + // Wait between commands if --interval is specified. + if ($interval = $input->getOption('interval')) { + sleep($interval); + } } ); }); @@ -291,30 +372,6 @@ private function getCommand(InputInterface $input, OutputInterface $output): ?st return $command; } - /** - * Gets the number of workers that should be used. - * - * @param \Symfony\Component\Console\Input\InputInterface $input - * The input. - * - * @return int - * Number of workers to be used. - */ - protected function getWorkerCount(InputInterface $input): int { - $result = $input->getOption('workers'); - - if ($result > self::WORKER_LIMIT) { - $this->logger->warning('Limiting workers to {count}, which is the maximum.', ['count' => self::WORKER_LIMIT]); - $result = self::WORKER_LIMIT; - } - - if ($result > 1) { - $this->logger->notice("Using {count} workers.", ['count' => $result]); - } - - return $result; - } - /** * Get unique placeholder from a command. */ diff --git a/test/Integration/Command/ExecCommandTest.php b/test/Integration/Command/ExecCommandTest.php index 4c9ecf4..922ffa8 100644 --- a/test/Integration/Command/ExecCommandTest.php +++ b/test/Integration/Command/ExecCommandTest.php @@ -668,6 +668,43 @@ public function testWithDryRunQuiet(): void { EOF, $process->getOutput()); } + /** + * @testdox Shows error when --interval is negative. + */ + public function testNegativeInterval(): void { + $process = Process::fromShellCommandline( + 'drall ex --interval=-3 -- drush st --fields=site', + static::PATH_DRUPAL, + ); + $process->run(); + $this->assertOutputEquals(<<getOutput()); + $this->assertOutputContainsString('Invalid options detected', $process->getErrorOutput()); + $this->assertEquals(1, $process->getExitCode()); + } + + /** + * @testdox With --interval. + */ + public function testWithInterval(): void { + $process = Process::fromShellCommandline( + 'drall ex --interval=2 --verbose -- ./vendor/bin/drush st --fields=site', + static::PATH_DRUPAL, + ); + $process->run(); + $this->assertOutputContainsString( + '[notice] Using a 2-second interval between commands.' . PHP_EOL, + $process->getOutput(), + ); + + // The command must take 2 * count($sites) seconds. + // This confirms that the sleep(2) command is actually executed. + $timeTaken = microtime(TRUE) - $process->getStartTime(); + $this->assertGreaterThan(10, $timeTaken); + } + /** * @testdox With --workers=2. */ @@ -697,20 +734,37 @@ public function testWithWorkers(): void { } /** - * @testdox With --workers=17. - * - * Drall caps the maximum workers to a pre-determined limit. + * @testdox Shows error when --worker limit exceeds the maximum. */ public function testWorkerLimit(): void { $process = Process::fromShellCommandline( - 'drall ex --workers=17 --verbose -- drush --uri=@@dir st --fields=site', + 'drall ex --workers=17 -- drush st --fields=site', static::PATH_DRUPAL, ); $process->run(); - $this->assertStringStartsWith( - '[warning] Limiting workers to 16, which is the maximum.' . PHP_EOL, - $process->getOutput(), + $this->assertOutputEquals(<<getOutput()); + $this->assertOutputContainsString('Invalid options detected', $process->getErrorOutput()); + $this->assertEquals(1, $process->getExitCode()); + } + + /** + * @testdox Shows error when --workers and --interval are used together. + */ + public function testIntervalWithWorkers(): void { + $process = Process::fromShellCommandline( + 'drall ex --workers=2 --interval=2 -- drush st --fields=site', + static::PATH_DRUPAL, ); + $process->run(); + $this->assertOutputEquals(<<getOutput()); + $this->assertOutputContainsString('Incompatible options detected', $process->getErrorOutput()); + $this->assertEquals(1, $process->getExitCode()); } /** From 760b81483db83c81890969c124a1aecc7eab3572 Mon Sep 17 00:00:00 2001 From: Jerry Radwick Date: Sat, 28 Dec 2024 09:31:32 -0500 Subject: [PATCH 4/5] Create EnvironmentId::isActive() --- src/Command/ExecCommand.php | 3 +-- src/Drall.php | 18 ------------------ src/Model/EnvironmentId.php | 15 +++++++++++++++ test/Integration/Command/ExecCommandTest.php | 5 +++-- test/Unit/Model/EnvironmentIdTest.php | 19 +++++++++++++++++++ 5 files changed, 38 insertions(+), 22 deletions(-) create mode 100644 test/Unit/Model/EnvironmentIdTest.php diff --git a/src/Command/ExecCommand.php b/src/Command/ExecCommand.php index ce4d04a..b31487e 100644 --- a/src/Command/ExecCommand.php +++ b/src/Command/ExecCommand.php @@ -8,7 +8,6 @@ use Amp\Process\Process; use Amp\Sync\ConcurrentIterator; use Amp\Sync\LocalSemaphore; -use Drall\Drall; use Drall\Model\EnvironmentId; use Drall\Model\Placeholder; use Drall\Trait\SignalAwareTrait; @@ -401,7 +400,7 @@ private function getUniquePlaceholder(string $command): ?Placeholder { */ private function isProgressBarHidden(InputInterface $input): bool { if ( - Drall::isEnvironment(EnvironmentId::Test) || + EnvironmentId::Test->isActive() || $input->getOption('no-progress') ) { return TRUE; diff --git a/src/Drall.php b/src/Drall.php index c6182ba..ef796f0 100644 --- a/src/Drall.php +++ b/src/Drall.php @@ -7,7 +7,6 @@ use Drall\Command\SiteAliasesCommand; use Drall\Command\SiteDirectoriesCommand; use Drall\Command\SiteKeysCommand; -use Drall\Model\EnvironmentId; use Drall\Trait\SiteDetectorAwareTrait; use Symfony\Component\Console\Application; use Symfony\Component\Console\Command\Command; @@ -96,21 +95,4 @@ public function find(string $name): Command { } } - /** - * Whether Drall is running in a specific environment. - * - * This helps with development and testing. For example, during tests, - * Drall progress bars can pollute the output. Thus, we hide them for - * the "test" environment. - * - * @param \Drall\Model\EnvironmentId $id - * Environment ID. - * - * @return bool - * True or False. - */ - public static function isEnvironment(EnvironmentId $id): bool { - return getenv('DRALL_ENVIRONMENT') === $id->value; - } - } diff --git a/src/Model/EnvironmentId.php b/src/Model/EnvironmentId.php index 43e07f5..9aff0f7 100644 --- a/src/Model/EnvironmentId.php +++ b/src/Model/EnvironmentId.php @@ -8,4 +8,19 @@ enum EnvironmentId: string { case Test = 'test'; + case Unknown = 'unknown'; + + /** + * Whether the environment is currently active. + * + * For example, progress bars are automatically hidden for the "test" + * environment to prevent them from polluting the output. + * + * @return bool + * True or False. + */ + public function isActive(): bool { + return getenv('DRALL_ENVIRONMENT') === $this->value; + } + } diff --git a/test/Integration/Command/ExecCommandTest.php b/test/Integration/Command/ExecCommandTest.php index 922ffa8..f84f5d9 100644 --- a/test/Integration/Command/ExecCommandTest.php +++ b/test/Integration/Command/ExecCommandTest.php @@ -2,6 +2,7 @@ namespace Drall\Test\Integration\Command; +use Drall\Model\EnvironmentId; use Drall\TestCase; use Symfony\Component\Process\Process; @@ -523,7 +524,7 @@ public function testWithProgressBar(): void { $process = Process::fromShellCommandline( 'drall exec -- ./vendor/bin/drush st --field=site 2>&1', static::PATH_DRUPAL, - ['DRALL_ENVIRONMENT' => 'unknown'], + ['DRALL_ENVIRONMENT' => EnvironmentId::Unknown->value], ); $process->run(); $this->assertOutputEquals(<< 'unknown'], + ['DRALL_ENVIRONMENT' => EnvironmentId::Unknown->value], ); $process->run(); $this->assertOutputEquals(<<assertFalse(EnvironmentId::Unknown->isActive()); + $this->assertEquals('test', getenv('DRALL_ENVIRONMENT')); + $this->assertTrue(EnvironmentId::Test->isActive()); + } + +} From 2aade277d2482b586542ff69067820da1c7afd51 Mon Sep 17 00:00:00 2001 From: Jerry Radwick Date: Sat, 28 Dec 2024 09:43:59 -0500 Subject: [PATCH 5/5] Add tests for custom assertions --- test/Unit/TestCaseTest.php | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/test/Unit/TestCaseTest.php b/test/Unit/TestCaseTest.php index 9c4201c..ce3675e 100644 --- a/test/Unit/TestCaseTest.php +++ b/test/Unit/TestCaseTest.php @@ -22,16 +22,32 @@ public function testCreateTempFile() { } public function testAssertOutputEquals() { - $expected = <<assertOutputEquals(<<assertOutputStartsWith( + '[notice] Hakuna matata.' . PHP_EOL, + "[notice] Hakuna matata. \n - bunny \n - wabbit \n", + ); + } - $this->assertOutputEquals($expected, $actual); + public function testAssertOutputContainsString() { + // For some reason, drush's output ($actual) has spaces before EOL. + // This assertion respects leading spaces and ignores trailing spaces. + $this->assertOutputContainsString( + ' - bunny' . PHP_EOL, + "[notice] Hakuna matata. \n - bunny \n - wabbit \n", + ); } }