From be3bc0a3485ca8f29c622ebcef54030226639539 Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Wed, 16 Sep 2020 17:37:17 -0500 Subject: [PATCH 01/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available --- config/services.xml | 10 +++ src/Patch/Applier.php | 68 +++++------------- src/Patch/GitPatchCommand.php | 108 ++++++++++++++++++++++++++++ src/Patch/PatchCommand.php | 108 ++++++++++++++++++++++++++++ src/Patch/PatchCommandInterface.php | 17 +++++ src/Patch/PatchCommandNotFound.php | 18 +++++ src/Patch/PatchCommandSelector.php | 79 ++++++++++++++++++++ 7 files changed, 358 insertions(+), 50 deletions(-) create mode 100644 src/Patch/GitPatchCommand.php create mode 100644 src/Patch/PatchCommand.php create mode 100644 src/Patch/PatchCommandInterface.php create mode 100644 src/Patch/PatchCommandNotFound.php create mode 100644 src/Patch/PatchCommandSelector.php diff --git a/config/services.xml b/config/services.xml index 52ca97c..0272fb6 100644 --- a/config/services.xml +++ b/config/services.xml @@ -19,6 +19,7 @@ + @@ -76,5 +77,14 @@ + + + + + + + + + diff --git a/src/Patch/Applier.php b/src/Patch/Applier.php index e5f2d1e..fa0becf 100644 --- a/src/Patch/Applier.php +++ b/src/Patch/Applier.php @@ -10,7 +10,6 @@ use Magento\CloudPatches\Composer\MagentoVersion; use Magento\CloudPatches\Filesystem\Filesystem; use Magento\CloudPatches\Patch\Status\StatusPool; -use Magento\CloudPatches\Shell\ProcessFactory; use Symfony\Component\Process\Exception\ProcessFailedException; /** @@ -18,11 +17,6 @@ */ class Applier { - /** - * @var ProcessFactory - */ - private $processFactory; - /** * @var GitConverter */ @@ -37,23 +31,27 @@ class Applier * @var Filesystem */ private $filesystem; + /** + * @var PatchCommandInterface + */ + private $patchCommand; /** - * @param ProcessFactory $processFactory * @param GitConverter $gitConverter * @param MagentoVersion $magentoVersion * @param Filesystem $filesystem + * @param PatchCommandInterface $patchCommand */ public function __construct( - ProcessFactory $processFactory, GitConverter $gitConverter, MagentoVersion $magentoVersion, - Filesystem $filesystem + Filesystem $filesystem, + PatchCommandInterface $patchCommand ) { - $this->processFactory = $processFactory; $this->gitConverter = $gitConverter; $this->magentoVersion = $magentoVersion; $this->filesystem = $filesystem; + $this->patchCommand = $patchCommand; } /** @@ -69,20 +67,12 @@ public function apply(string $path, string $id): string { $content = $this->readContent($path); try { - $this->processFactory->create(['git', 'apply'], $content) - ->mustRun(); + $result = $this->patchCommand->apply($content); } catch (ProcessFailedException $exception) { - try { - $this->processFactory->create(['git', 'apply', '--check', '--reverse'], $content) - ->mustRun(); - } catch (ProcessFailedException $reverseException) { - throw new ApplierException($exception->getMessage(), $exception->getCode()); - } - - return sprintf('Patch %s was already applied', $id); + throw new ApplierException($exception->getMessage(), $exception->getCode()); } - return sprintf('Patch %s has been applied', $id); + return $result ? sprintf('Patch %s has been applied', $id) : sprintf('Patch %s was already applied', $id); } /** @@ -98,20 +88,12 @@ public function revert(string $path, string $id): string { $content = $this->readContent($path); try { - $this->processFactory->create(['git', 'apply', '--reverse'], $content) - ->mustRun(); + $result = $this->patchCommand->revert($content); } catch (ProcessFailedException $exception) { - try { - $this->processFactory->create(['git', 'apply', '--check'], $content) - ->mustRun(); - } catch (ProcessFailedException $applyException) { - throw new ApplierException($exception->getMessage(), $exception->getCode()); - } - - return sprintf('Patch %s wasn\'t applied', $id); + throw new ApplierException($exception->getMessage(), $exception->getCode()); } - return sprintf('Patch %s has been reverted', $id); + return $result ? sprintf('Patch %s has been reverted', $id) : sprintf('Patch %s wasn\'t applied', $id); } /** @@ -124,20 +106,12 @@ public function status(string $patchContent): string { $patchContent = $this->prepareContent($patchContent); try { - $this->processFactory->create(['git', 'apply', '--check'], $patchContent) - ->mustRun(); + $result = $this->patchCommand->status($patchContent); } catch (ProcessFailedException $exception) { - try { - $this->processFactory->create(['git', 'apply', '--check', '--reverse'], $patchContent) - ->mustRun(); - } catch (ProcessFailedException $reverseException) { - return StatusPool::NA; - } - - return StatusPool::APPLIED; + return StatusPool::NA; } - return StatusPool::NOT_APPLIED; + return $result ? StatusPool::NOT_APPLIED : StatusPool::APPLIED; } /** @@ -149,14 +123,8 @@ public function status(string $patchContent): string public function checkApply(string $patchContent): bool { $patchContent = $this->prepareContent($patchContent); - try { - $this->processFactory->create(['git', 'apply', '--check'], $patchContent) - ->mustRun(); - } catch (ProcessFailedException $exception) { - return false; - } - return true; + return $this->patchCommand->check($patchContent); } /** diff --git a/src/Patch/GitPatchCommand.php b/src/Patch/GitPatchCommand.php new file mode 100644 index 0000000..7695644 --- /dev/null +++ b/src/Patch/GitPatchCommand.php @@ -0,0 +1,108 @@ +processFactory = $processFactory; + } + + /** + * @inheritDoc + */ + public function apply(string $patch): bool + { + try { + $this->processFactory->create(['git', 'apply'], $patch) + ->mustRun(); + $result = true; + } catch (ProcessFailedException $exception) { + $this->processFactory->create(['git', 'apply', '--check', '--reverse'], $patch) + ->mustRun(); + $result = false; + } + return $result; + } + + /** + * @inheritDoc + */ + public function revert(string $patch): bool + { + try { + $this->processFactory->create(['git', 'apply', '--reverse'], $patch) + ->mustRun(); + $result = true; + } catch (ProcessFailedException $exception) { + $this->processFactory->create(['git', 'apply', '--check'], $patch) + ->mustRun(); + $result = false; + } + return $result; + } + + /** + * @inheritDoc + */ + public function check(string $patch): bool + { + try { + $this->processFactory->create(['git', 'apply', '--check'], $patch) + ->mustRun(); + $result = true; + } catch (ProcessFailedException $exception) { + $result = false; + } + + return $result; + } + + /** + * @inheritDoc + */ + public function status(string $patch): bool + { + try { + $this->processFactory->create(['git', 'apply', '--check'], $patch) + ->mustRun(); + $result = true; + } catch (ProcessFailedException $exception) { + $this->processFactory->create(['git', 'apply', '--check', '--reverse'], $patch) + ->mustRun(); + $result = false; + } + + return $result; + } + + /** + * @inheritDoc + */ + public function isInstalled(): bool + { + try { + $this->processFactory->create(['git', '--version'])->mustRun(); + $result = true; + } catch (ProcessFailedException $exception) { + $result = false; + } + + return $result; + } +} diff --git a/src/Patch/PatchCommand.php b/src/Patch/PatchCommand.php new file mode 100644 index 0000000..d460114 --- /dev/null +++ b/src/Patch/PatchCommand.php @@ -0,0 +1,108 @@ +processFactory = $processFactory; + } + + /** + * @inheritDoc + */ + public function apply(string $patch): bool + { + try { + $this->processFactory->create(['patch', '--silent', '-p1'], $patch) + ->mustRun(); + $result = true; + } catch (ProcessFailedException $exception) { + $this->processFactory->create(['patch', '--dry-run', '--silent', '-R', '-p1'], $patch) + ->mustRun(); + $result = false; + } + return $result; + } + + /** + * @inheritDoc + */ + public function revert(string $patch): bool + { + try { + $this->processFactory->create(['patch', '--silent', '-R', '-p1'], $patch) + ->mustRun(); + $result = true; + } catch (ProcessFailedException $exception) { + $this->processFactory->create(['patch', '--dry-run', '--silent', '-p1'], $patch) + ->mustRun(); + $result = false; + } + return $result; + } + + /** + * @inheritDoc + */ + public function check(string $patch): bool + { + try { + $this->processFactory->create(['patch', '--dry-run', '--silent', '-p1'], $patch) + ->mustRun(); + $result = true; + } catch (ProcessFailedException $exception) { + $result = false; + } + + return $result; + } + + /** + * @inheritDoc + */ + public function status(string $patch): bool + { + try { + $this->processFactory->create(['patch', '--dry-run', '--silent', '-p1'], $patch) + ->mustRun(); + $result = true; + } catch (ProcessFailedException $exception) { + $this->processFactory->create(['patch', '--dry-run', '--silent', '-R', '-p1'], $patch) + ->mustRun(); + $result = false; + } + + return $result; + } + + /** + * @inheritDoc + */ + public function isInstalled(): bool + { + try { + $this->processFactory->create(['patch', '--version'])->mustRun(); + $result = true; + } catch (ProcessFailedException $exception) { + $result = false; + } + + return $result; + } +} diff --git a/src/Patch/PatchCommandInterface.php b/src/Patch/PatchCommandInterface.php new file mode 100644 index 0000000..cbd4b93 --- /dev/null +++ b/src/Patch/PatchCommandInterface.php @@ -0,0 +1,17 @@ +commands = $commands; + } + + /** + * @inheritDoc + */ + public function apply(string $patch): bool + { + return $this->getCommand()->apply($patch); + } + + /** + * @inheritDoc + */ + public function revert(string $patch): bool + { + return $this->getCommand()->revert($patch); + } + + /** + * @inheritDoc + */ + public function check(string $patch): bool + { + return $this->getCommand()->check($patch); + } + + /** + * @inheritDoc + */ + public function status(string $patch): bool + { + return $this->getCommand()->status($patch); + } + + /** + * @inheritDoc + */ + public function isInstalled(): bool + { + return $this->getCommand()->isInstalled(); + } + + /** + * Return first available command + */ + private function getCommand() + { + foreach ($this->commands as $command) { + if ($command->isInstalled()) { + return $command; + } + } + throw new PatchCommandNotFound(); + } +} From e2633c4dfa3e47aa24da1aca9a1a16f3726ee1d0 Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Mon, 21 Sep 2020 09:19:04 -0500 Subject: [PATCH 02/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available - Move exception handling from patch command implementations --- src/Patch/Applier.php | 60 +++++++++++++++++++--------- src/Patch/GitPatchCommand.php | 59 +++++++--------------------- src/Patch/PatchCommand.php | 61 ++++++++--------------------- src/Patch/PatchCommandInterface.php | 46 +++++++++++++++++++--- src/Patch/PatchCommandNotFound.php | 2 +- src/Patch/PatchCommandSelector.php | 20 +++++----- 6 files changed, 127 insertions(+), 121 deletions(-) diff --git a/src/Patch/Applier.php b/src/Patch/Applier.php index fa0becf..b381e0b 100644 --- a/src/Patch/Applier.php +++ b/src/Patch/Applier.php @@ -17,6 +17,11 @@ */ class Applier { + /** + * @var PatchCommandInterface + */ + private $patchCommand; + /** * @var GitConverter */ @@ -31,27 +36,23 @@ class Applier * @var Filesystem */ private $filesystem; - /** - * @var PatchCommandInterface - */ - private $patchCommand; /** + * @param PatchCommandInterface $patchCommand * @param GitConverter $gitConverter * @param MagentoVersion $magentoVersion * @param Filesystem $filesystem - * @param PatchCommandInterface $patchCommand */ public function __construct( + PatchCommandInterface $patchCommand, GitConverter $gitConverter, MagentoVersion $magentoVersion, - Filesystem $filesystem, - PatchCommandInterface $patchCommand + Filesystem $filesystem ) { + $this->patchCommand = $patchCommand; $this->gitConverter = $gitConverter; $this->magentoVersion = $magentoVersion; $this->filesystem = $filesystem; - $this->patchCommand = $patchCommand; } /** @@ -67,12 +68,18 @@ public function apply(string $path, string $id): string { $content = $this->readContent($path); try { - $result = $this->patchCommand->apply($content); + $this->patchCommand->applyCheck($content); } catch (ProcessFailedException $exception) { - throw new ApplierException($exception->getMessage(), $exception->getCode()); + try { + $this->patchCommand->reverseCheck($content); + } catch (ProcessFailedException $reverseException) { + throw new ApplierException($exception->getMessage(), $exception->getCode()); + } + + return sprintf('Patch %s was already applied', $id); } - return $result ? sprintf('Patch %s has been applied', $id) : sprintf('Patch %s was already applied', $id); + return sprintf('Patch %s has been applied', $id); } /** @@ -88,12 +95,18 @@ public function revert(string $path, string $id): string { $content = $this->readContent($path); try { - $result = $this->patchCommand->revert($content); + $this->patchCommand->revert($content); } catch (ProcessFailedException $exception) { - throw new ApplierException($exception->getMessage(), $exception->getCode()); + try { + $this->patchCommand->applyCheck($content); + } catch (ProcessFailedException $applyException) { + throw new ApplierException($exception->getMessage(), $exception->getCode()); + } + + return sprintf('Patch %s wasn\'t applied', $id); } - return $result ? sprintf('Patch %s has been reverted', $id) : sprintf('Patch %s wasn\'t applied', $id); + return sprintf('Patch %s has been reverted', $id); } /** @@ -106,12 +119,18 @@ public function status(string $patchContent): string { $patchContent = $this->prepareContent($patchContent); try { - $result = $this->patchCommand->status($patchContent); + $this->patchCommand->applyCheck($patchContent); } catch (ProcessFailedException $exception) { - return StatusPool::NA; + try { + $this->patchCommand->reverseCheck($patchContent); + } catch (ProcessFailedException $reverseException) { + return StatusPool::NA; + } + + return StatusPool::APPLIED; } - return $result ? StatusPool::NOT_APPLIED : StatusPool::APPLIED; + return StatusPool::NOT_APPLIED; } /** @@ -123,8 +142,13 @@ public function status(string $patchContent): string public function checkApply(string $patchContent): bool { $patchContent = $this->prepareContent($patchContent); + try { + $this->patchCommand->applyCheck($patchContent); + } catch (ProcessFailedException $exception) { + return false; + } - return $this->patchCommand->check($patchContent); + return true; } /** diff --git a/src/Patch/GitPatchCommand.php b/src/Patch/GitPatchCommand.php index 7695644..3d4108a 100644 --- a/src/Patch/GitPatchCommand.php +++ b/src/Patch/GitPatchCommand.php @@ -10,6 +10,9 @@ use Magento\CloudPatches\Shell\ProcessFactory; use Symfony\Component\Process\Exception\ProcessFailedException; +/** + * Patch command for GIT + */ class GitPatchCommand implements PatchCommandInterface { /** @@ -26,69 +29,37 @@ public function __construct( /** * @inheritDoc */ - public function apply(string $patch): bool + public function apply(string $patch) { - try { - $this->processFactory->create(['git', 'apply'], $patch) - ->mustRun(); - $result = true; - } catch (ProcessFailedException $exception) { - $this->processFactory->create(['git', 'apply', '--check', '--reverse'], $patch) - ->mustRun(); - $result = false; - } - return $result; + $this->processFactory->create(['git', 'apply'], $patch) + ->mustRun(); } /** * @inheritDoc */ - public function revert(string $patch): bool + public function revert(string $patch) { - try { - $this->processFactory->create(['git', 'apply', '--reverse'], $patch) - ->mustRun(); - $result = true; - } catch (ProcessFailedException $exception) { - $this->processFactory->create(['git', 'apply', '--check'], $patch) - ->mustRun(); - $result = false; - } - return $result; + $this->processFactory->create(['git', 'apply', '--reverse'], $patch) + ->mustRun(); } /** * @inheritDoc */ - public function check(string $patch): bool + public function applyCheck(string $patch) { - try { - $this->processFactory->create(['git', 'apply', '--check'], $patch) - ->mustRun(); - $result = true; - } catch (ProcessFailedException $exception) { - $result = false; - } - - return $result; + $this->processFactory->create(['git', 'apply', '--check'], $patch) + ->mustRun(); } /** * @inheritDoc */ - public function status(string $patch): bool + public function reverseCheck(string $patch) { - try { - $this->processFactory->create(['git', 'apply', '--check'], $patch) - ->mustRun(); - $result = true; - } catch (ProcessFailedException $exception) { - $this->processFactory->create(['git', 'apply', '--check', '--reverse'], $patch) - ->mustRun(); - $result = false; - } - - return $result; + $this->processFactory->create(['git', 'apply', '--reverse', '--check'], $patch) + ->mustRun(); } /** diff --git a/src/Patch/PatchCommand.php b/src/Patch/PatchCommand.php index d460114..29a1d38 100644 --- a/src/Patch/PatchCommand.php +++ b/src/Patch/PatchCommand.php @@ -10,6 +10,9 @@ use Magento\CloudPatches\Shell\ProcessFactory; use Symfony\Component\Process\Exception\ProcessFailedException; +/** + * Patch command for unix patch + */ class PatchCommand implements PatchCommandInterface { /** @@ -26,69 +29,39 @@ public function __construct( /** * @inheritDoc */ - public function apply(string $patch): bool + public function apply(string $patch) { - try { - $this->processFactory->create(['patch', '--silent', '-p1'], $patch) - ->mustRun(); - $result = true; - } catch (ProcessFailedException $exception) { - $this->processFactory->create(['patch', '--dry-run', '--silent', '-R', '-p1'], $patch) - ->mustRun(); - $result = false; - } - return $result; + $this->applyCheck($patch); + $this->processFactory->create(['patch', '--silent', '-f', '-p1'], $patch) + ->mustRun(); } /** * @inheritDoc */ - public function revert(string $patch): bool + public function revert(string $patch) { - try { - $this->processFactory->create(['patch', '--silent', '-R', '-p1'], $patch) - ->mustRun(); - $result = true; - } catch (ProcessFailedException $exception) { - $this->processFactory->create(['patch', '--dry-run', '--silent', '-p1'], $patch) - ->mustRun(); - $result = false; - } - return $result; + $this->reverseCheck($patch); + $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--reverse'], $patch) + ->mustRun(); } /** * @inheritDoc */ - public function check(string $patch): bool + public function applyCheck(string $patch) { - try { - $this->processFactory->create(['patch', '--dry-run', '--silent', '-p1'], $patch) - ->mustRun(); - $result = true; - } catch (ProcessFailedException $exception) { - $result = false; - } - - return $result; + $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--dry-run'], $patch) + ->mustRun(); } /** * @inheritDoc */ - public function status(string $patch): bool + public function reverseCheck(string $patch) { - try { - $this->processFactory->create(['patch', '--dry-run', '--silent', '-p1'], $patch) - ->mustRun(); - $result = true; - } catch (ProcessFailedException $exception) { - $this->processFactory->create(['patch', '--dry-run', '--silent', '-R', '-p1'], $patch) - ->mustRun(); - $result = false; - } - - return $result; + $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--reverse', '--dry-run'], $patch) + ->mustRun(); } /** diff --git a/src/Patch/PatchCommandInterface.php b/src/Patch/PatchCommandInterface.php index cbd4b93..f135a09 100644 --- a/src/Patch/PatchCommandInterface.php +++ b/src/Patch/PatchCommandInterface.php @@ -7,11 +7,47 @@ namespace Magento\CloudPatches\Patch; +/** + * Patch command interface + */ interface PatchCommandInterface { - public function apply(string $patch): bool; - public function revert(string $patch): bool; - public function check(string $patch): bool; - public function status(string $patch): bool; + /** + * Applies patch + * + * @param string $patch + * @return void + */ + public function apply(string $patch); + + /** + * Reverts patch + * + * @param string $patch + * @return void + */ + public function revert(string $patch); + + /** + * Checks if the patch can be applied. + * + * @param string $patch + * @return void + */ + public function applyCheck(string $patch); + + /** + * Checks if the patch can be reversed + * + * @param string $patch + * @return void + */ + public function reverseCheck(string $patch); + + /** + * Checks if the command is installed + * + * @return bool + */ public function isInstalled(): bool; -} \ No newline at end of file +} diff --git a/src/Patch/PatchCommandNotFound.php b/src/Patch/PatchCommandNotFound.php index 870fd81..698a70a 100644 --- a/src/Patch/PatchCommandNotFound.php +++ b/src/Patch/PatchCommandNotFound.php @@ -15,4 +15,4 @@ public function __construct() { parent::__construct('git or patch is required to perform this operation.'); } -} \ No newline at end of file +} diff --git a/src/Patch/PatchCommandSelector.php b/src/Patch/PatchCommandSelector.php index b753aa4..e93d3c9 100644 --- a/src/Patch/PatchCommandSelector.php +++ b/src/Patch/PatchCommandSelector.php @@ -7,6 +7,9 @@ namespace Magento\CloudPatches\Patch; +/** + * Patch command selector + */ class PatchCommandSelector implements PatchCommandInterface { /** @@ -15,7 +18,6 @@ class PatchCommandSelector implements PatchCommandInterface private $commands; /** - * PatchCommandSelector constructor. * @param PatchCommandInterface[] $commands */ public function __construct( @@ -27,33 +29,33 @@ public function __construct( /** * @inheritDoc */ - public function apply(string $patch): bool + public function apply(string $patch) { - return $this->getCommand()->apply($patch); + $this->getCommand()->apply($patch); } /** * @inheritDoc */ - public function revert(string $patch): bool + public function revert(string $patch) { - return $this->getCommand()->revert($patch); + $this->getCommand()->revert($patch); } /** * @inheritDoc */ - public function check(string $patch): bool + public function applyCheck(string $patch) { - return $this->getCommand()->check($patch); + $this->getCommand()->applyCheck($patch); } /** * @inheritDoc */ - public function status(string $patch): bool + public function reverseCheck(string $patch) { - return $this->getCommand()->status($patch); + $this->getCommand()->reverseCheck($patch); } /** From 87f774e6ee3bf0df3168281a4c6824ae188a9bcc Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Mon, 21 Sep 2020 10:06:34 -0500 Subject: [PATCH 03/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available - Fix unit test --- src/Patch/Applier.php | 6 +- src/Patch/GitPatchCommand.php | 2 +- src/Patch/PatchCommand.php | 4 +- src/Patch/PatchCommandInterface.php | 2 +- src/Patch/PatchCommandSelector.php | 4 +- src/Test/Unit/Patch/ApplierTest.php | 184 +++++++++------------------- 6 files changed, 67 insertions(+), 135 deletions(-) diff --git a/src/Patch/Applier.php b/src/Patch/Applier.php index b381e0b..7ad39ff 100644 --- a/src/Patch/Applier.php +++ b/src/Patch/Applier.php @@ -68,10 +68,10 @@ public function apply(string $path, string $id): string { $content = $this->readContent($path); try { - $this->patchCommand->applyCheck($content); + $this->patchCommand->apply($content); } catch (ProcessFailedException $exception) { try { - $this->patchCommand->reverseCheck($content); + $this->patchCommand->revertCheck($content); } catch (ProcessFailedException $reverseException) { throw new ApplierException($exception->getMessage(), $exception->getCode()); } @@ -122,7 +122,7 @@ public function status(string $patchContent): string $this->patchCommand->applyCheck($patchContent); } catch (ProcessFailedException $exception) { try { - $this->patchCommand->reverseCheck($patchContent); + $this->patchCommand->revertCheck($patchContent); } catch (ProcessFailedException $reverseException) { return StatusPool::NA; } diff --git a/src/Patch/GitPatchCommand.php b/src/Patch/GitPatchCommand.php index 3d4108a..ce02bd4 100644 --- a/src/Patch/GitPatchCommand.php +++ b/src/Patch/GitPatchCommand.php @@ -56,7 +56,7 @@ public function applyCheck(string $patch) /** * @inheritDoc */ - public function reverseCheck(string $patch) + public function revertCheck(string $patch) { $this->processFactory->create(['git', 'apply', '--reverse', '--check'], $patch) ->mustRun(); diff --git a/src/Patch/PatchCommand.php b/src/Patch/PatchCommand.php index 29a1d38..a38b4a4 100644 --- a/src/Patch/PatchCommand.php +++ b/src/Patch/PatchCommand.php @@ -41,7 +41,7 @@ public function apply(string $patch) */ public function revert(string $patch) { - $this->reverseCheck($patch); + $this->revertCheck($patch); $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--reverse'], $patch) ->mustRun(); } @@ -58,7 +58,7 @@ public function applyCheck(string $patch) /** * @inheritDoc */ - public function reverseCheck(string $patch) + public function revertCheck(string $patch) { $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--reverse', '--dry-run'], $patch) ->mustRun(); diff --git a/src/Patch/PatchCommandInterface.php b/src/Patch/PatchCommandInterface.php index f135a09..6aee769 100644 --- a/src/Patch/PatchCommandInterface.php +++ b/src/Patch/PatchCommandInterface.php @@ -42,7 +42,7 @@ public function applyCheck(string $patch); * @param string $patch * @return void */ - public function reverseCheck(string $patch); + public function revertCheck(string $patch); /** * Checks if the command is installed diff --git a/src/Patch/PatchCommandSelector.php b/src/Patch/PatchCommandSelector.php index e93d3c9..48b6afc 100644 --- a/src/Patch/PatchCommandSelector.php +++ b/src/Patch/PatchCommandSelector.php @@ -53,9 +53,9 @@ public function applyCheck(string $patch) /** * @inheritDoc */ - public function reverseCheck(string $patch) + public function revertCheck(string $patch) { - $this->getCommand()->reverseCheck($patch); + $this->getCommand()->revertCheck($patch); } /** diff --git a/src/Test/Unit/Patch/ApplierTest.php b/src/Test/Unit/Patch/ApplierTest.php index d62d0c0..91a3197 100644 --- a/src/Test/Unit/Patch/ApplierTest.php +++ b/src/Test/Unit/Patch/ApplierTest.php @@ -12,8 +12,8 @@ use Magento\CloudPatches\Patch\Applier; use Magento\CloudPatches\Patch\ApplierException; use Magento\CloudPatches\Patch\GitConverter; +use Magento\CloudPatches\Patch\PatchCommandInterface; use Magento\CloudPatches\Patch\Status\StatusPool; -use Magento\CloudPatches\Shell\ProcessFactory; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Symfony\Component\Process\Exception\ProcessFailedException; @@ -30,9 +30,9 @@ class ApplierTest extends TestCase private $applier; /** - * @var ProcessFactory|MockObject + * @var PatchCommandInterface|MockObject */ - private $processFactory; + private $patchCommand; /** * @var GitConverter|MockObject @@ -54,13 +54,13 @@ class ApplierTest extends TestCase */ protected function setUp() { - $this->processFactory = $this->createMock(ProcessFactory::class); + $this->patchCommand = $this->createMock(PatchCommandInterface::class); $this->gitConverter = $this->createMock(GitConverter::class); $this->magentoVersion = $this->createMock(MagentoVersion::class); $this->filesystem = $this->createMock(Filesystem::class); $this->applier = new Applier( - $this->processFactory, + $this->patchCommand, $this->gitConverter, $this->magentoVersion, $this->filesystem @@ -86,14 +86,10 @@ public function testApply() $this->gitConverter->expects($this->once()) ->method('convert') ->willReturn('gitContent'); - $processMock = $this->createMock(Process::class); - $this->processFactory->expects($this->once()) - ->method('create') - ->withConsecutive([['git', 'apply'], 'gitContent']) - ->willReturn($processMock); - $processMock->expects($this->once()) - ->method('mustRun'); + $this->patchCommand->expects($this->once()) + ->method('apply') + ->with('gitContent'); $this->assertSame($expectedMessage, $this->applier->apply($path, $patchId)); } @@ -106,14 +102,13 @@ public function testApplyFailed() $path = 'path/to/patch'; $patchId = 'MC-11111'; - /** @var Process|MockObject $result */ - $processMock = $this->createMock(Process::class); - $processMock->method('mustRun') - ->willThrowException(new ProcessFailedException($processMock)); + $this->patchCommand->expects($this->once()) + ->method('apply') + ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); - $this->processFactory->expects($this->exactly(2)) - ->method('create') - ->willReturn($processMock); + $this->patchCommand->expects($this->once()) + ->method('revertCheck') + ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); $this->expectException(ApplierException::class); $this->applier->apply($path, $patchId); @@ -139,45 +134,16 @@ public function testApplyPatchAlreadyApplied() $this->gitConverter->expects($this->never()) ->method('convert'); - $this->processFactory->expects($this->exactly(2)) - ->method('create') - ->willReturnMap([ - [['git', 'apply'], 'patchContent'], - [['git', 'apply', '--check', '--reverse'], 'patchContent'] - ])->willReturnCallback([$this, 'shellApplyRevertCallback']); + $this->patchCommand->expects($this->once()) + ->method('apply') + ->with('patchContent') + ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); - $this->assertSame($expectedMessage, $this->applier->apply($path, $patchId)); - } + $this->patchCommand->expects($this->once()) + ->method('revertCheck') + ->with('patchContent'); - /** - * Callback for 'apply' and 'revert' operations. - * - * @param array $command - * @return Process - * - * @throws ProcessFailedException when the command isn't a reverse - */ - public function shellApplyRevertCallback(array $command): Process - { - if (in_array('--reverse', $command, true) && in_array('--check', $command, true) || - !in_array('--reverse', $command, true) && in_array('--check', $command, true) - ) { - // Command was the reverse check, it's all good. - /** @var Process|MockObject $result */ - $result = $this->createMock(Process::class); - $result->expects($this->once()) - ->method('mustRun'); - - return $result; - } - - /** @var Process|MockObject $result */ - $result = $this->createMock(Process::class); - $result->expects($this->once()) - ->method('mustRun') - ->willThrowException(new ProcessFailedException($result)); - - return $result; + $this->assertSame($expectedMessage, $this->applier->apply($path, $patchId)); } /** @@ -201,14 +167,9 @@ public function testRevert() ->method('convert') ->willReturn('gitContent'); - $processMock = $this->createMock(Process::class); - - $this->processFactory->expects($this->once()) - ->method('create') - ->withConsecutive([['git', 'apply', '--reverse'], 'gitContent']) - ->willReturn($processMock); - $processMock->expects($this->once()) - ->method('mustRun'); + $this->patchCommand->expects($this->once()) + ->method('revert') + ->with('gitContent'); $this->assertSame($expectedMessage, $this->applier->revert($path, $patchId)); } @@ -221,14 +182,13 @@ public function testRevertFailed() $path = 'path/to/patch'; $patchId = 'MC-11111'; - /** @var Process|MockObject $result */ - $processMock = $this->createMock(Process::class); - $processMock->method('mustRun') - ->willThrowException(new ProcessFailedException($processMock)); + $this->patchCommand->expects($this->once()) + ->method('revert') + ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); - $this->processFactory->expects($this->exactly(2)) - ->method('create') - ->willReturn($processMock); + $this->patchCommand->expects($this->once()) + ->method('applyCheck') + ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); $this->expectException(ApplierException::class); $this->applier->revert($path, $patchId); @@ -255,12 +215,14 @@ public function testRevertPatchWasntApplied() $this->gitConverter->expects($this->never()) ->method('convert'); - $this->processFactory->expects($this->exactly(2)) - ->method('create') - ->willReturnMap([ - [['git', 'apply'], $patchContent], - [['git', 'apply', '--check'], $patchContent] - ])->willReturnCallback([$this, 'shellApplyRevertCallback']); + $this->patchCommand->expects($this->once()) + ->method('revert') + ->with($patchContent) + ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); + + $this->patchCommand->expects($this->once()) + ->method('applyCheck') + ->with($patchContent); $this->assertSame($expectedMessage, $this->applier->revert($path, $patchId)); } @@ -271,14 +233,10 @@ public function testRevertPatchWasntApplied() public function testStatusNotApplied() { $patchContent = 'patch content'; - $processMock = $this->createMock(Process::class); - $this->processFactory->expects($this->once()) - ->method('create') - ->withConsecutive([['git', 'apply', '--check'], $patchContent]) - ->willReturn($processMock); - $processMock->expects($this->once()) - ->method('mustRun'); + $this->patchCommand->expects($this->once()) + ->method('applyCheck') + ->with($patchContent); $this->assertSame(StatusPool::NOT_APPLIED, $this->applier->status($patchContent)); } @@ -290,14 +248,15 @@ public function testStatusNotAvailable() { $patchContent = 'patch content'; - /** @var Process|MockObject $result */ - $processMock = $this->createMock(Process::class); - $processMock->method('mustRun') - ->willThrowException(new ProcessFailedException($processMock)); + $this->patchCommand->expects($this->once()) + ->method('applyCheck') + ->with($patchContent) + ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); - $this->processFactory->expects($this->exactly(2)) - ->method('create') - ->willReturn($processMock); + $this->patchCommand->expects($this->once()) + ->method('revertCheck') + ->with($patchContent) + ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); $this->assertSame(StatusPool::NA, $this->applier->status($patchContent)); } @@ -309,42 +268,15 @@ public function testStatusApplied() { $patchContent = 'patch content'; - $this->processFactory->expects($this->exactly(2)) - ->method('create') - ->willReturnMap([ - [['git', 'apply', '--check']], - [['git', 'apply', '--check', '--reverse']] - ])->willReturnCallback([$this, 'shellStatusCallback']); + $this->patchCommand->expects($this->once()) + ->method('applyCheck') + ->with($patchContent) + ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); - $this->assertSame(StatusPool::APPLIED, $this->applier->status($patchContent)); - } + $this->patchCommand->expects($this->once()) + ->method('revertCheck') + ->with($patchContent); - /** - * Callback for 'status' operations. - * - * @param array $command - * @return Process - * - * @throws ProcessFailedException when the command isn't a reverse - */ - public function shellStatusCallback(array $command): Process - { - if (in_array('--reverse', $command, true) && in_array('--check', $command, true)) { - // Command was the reverse check, it's all good. - /** @var Process|MockObject $result */ - $result = $this->createMock(Process::class); - $result->expects($this->once()) - ->method('mustRun'); - - return $result; - } - - /** @var Process|MockObject $result */ - $result = $this->createMock(Process::class); - $result->expects($this->once()) - ->method('mustRun') - ->willThrowException(new ProcessFailedException($result)); - - return $result; + $this->assertSame(StatusPool::APPLIED, $this->applier->status($patchContent)); } } From 678e65078289632ca1b925e48ab454a1380bdc27 Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Mon, 21 Sep 2020 15:11:55 -0500 Subject: [PATCH 04/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available - Add unit test --- config/services.xml | 8 +- src/Patch/Applier.php | 15 +- src/Patch/GitPatchCommand.php | 79 -------- src/Patch/PatchCommand.php | 64 +++--- src/Patch/PatchCommandException.php | 17 ++ src/Patch/PatchCommandInterface.php | 11 +- src/Patch/PatchCommandNotFound.php | 2 +- src/Patch/PatchCommandSelector.php | 81 -------- src/Shell/Command/DriverInterface.php | 23 +++ src/Shell/Command/GitDriver.php | 96 +++++++++ src/Shell/Command/PatchDriver.php | 98 +++++++++ src/Test/Unit/Patch/ApplierTest.php | 21 +- .../Unit/Shell/Command/PatchDriverTest.php | 187 ++++++++++++++++++ tests/unit/_data/files/file1.md | 1 + tests/unit/_data/files/file1.patch | 9 + tests/unit/_data/files/file1_and_file2.patch | 18 ++ tests/unit/_data/files/file1_applied_patch.md | 3 + tests/unit/_data/files/file2.md | 1 + tests/unit/_data/files/file2_applied_patch.md | 3 + tests/unit/var/.gitignore | 2 + 20 files changed, 523 insertions(+), 216 deletions(-) delete mode 100644 src/Patch/GitPatchCommand.php create mode 100644 src/Patch/PatchCommandException.php delete mode 100644 src/Patch/PatchCommandSelector.php create mode 100644 src/Shell/Command/DriverInterface.php create mode 100644 src/Shell/Command/GitDriver.php create mode 100644 src/Shell/Command/PatchDriver.php create mode 100644 src/Test/Unit/Shell/Command/PatchDriverTest.php create mode 100644 tests/unit/_data/files/file1.md create mode 100644 tests/unit/_data/files/file1.patch create mode 100644 tests/unit/_data/files/file1_and_file2.patch create mode 100644 tests/unit/_data/files/file1_applied_patch.md create mode 100644 tests/unit/_data/files/file2.md create mode 100644 tests/unit/_data/files/file2_applied_patch.md create mode 100644 tests/unit/var/.gitignore diff --git a/config/services.xml b/config/services.xml index 0272fb6..d635b9a 100644 --- a/config/services.xml +++ b/config/services.xml @@ -77,14 +77,14 @@ - + - - + + - + diff --git a/src/Patch/Applier.php b/src/Patch/Applier.php index 7ad39ff..93ecfb6 100644 --- a/src/Patch/Applier.php +++ b/src/Patch/Applier.php @@ -10,7 +10,6 @@ use Magento\CloudPatches\Composer\MagentoVersion; use Magento\CloudPatches\Filesystem\Filesystem; use Magento\CloudPatches\Patch\Status\StatusPool; -use Symfony\Component\Process\Exception\ProcessFailedException; /** * Applies and reverts patches. @@ -69,10 +68,10 @@ public function apply(string $path, string $id): string $content = $this->readContent($path); try { $this->patchCommand->apply($content); - } catch (ProcessFailedException $exception) { + } catch (PatchCommandException $exception) { try { $this->patchCommand->revertCheck($content); - } catch (ProcessFailedException $reverseException) { + } catch (PatchCommandException $reverseException) { throw new ApplierException($exception->getMessage(), $exception->getCode()); } @@ -96,10 +95,10 @@ public function revert(string $path, string $id): string $content = $this->readContent($path); try { $this->patchCommand->revert($content); - } catch (ProcessFailedException $exception) { + } catch (PatchCommandException $exception) { try { $this->patchCommand->applyCheck($content); - } catch (ProcessFailedException $applyException) { + } catch (PatchCommandException $applyException) { throw new ApplierException($exception->getMessage(), $exception->getCode()); } @@ -120,10 +119,10 @@ public function status(string $patchContent): string $patchContent = $this->prepareContent($patchContent); try { $this->patchCommand->applyCheck($patchContent); - } catch (ProcessFailedException $exception) { + } catch (PatchCommandException $exception) { try { $this->patchCommand->revertCheck($patchContent); - } catch (ProcessFailedException $reverseException) { + } catch (PatchCommandException $reverseException) { return StatusPool::NA; } @@ -144,7 +143,7 @@ public function checkApply(string $patchContent): bool $patchContent = $this->prepareContent($patchContent); try { $this->patchCommand->applyCheck($patchContent); - } catch (ProcessFailedException $exception) { + } catch (PatchCommandException $exception) { return false; } diff --git a/src/Patch/GitPatchCommand.php b/src/Patch/GitPatchCommand.php deleted file mode 100644 index ce02bd4..0000000 --- a/src/Patch/GitPatchCommand.php +++ /dev/null @@ -1,79 +0,0 @@ -processFactory = $processFactory; - } - - /** - * @inheritDoc - */ - public function apply(string $patch) - { - $this->processFactory->create(['git', 'apply'], $patch) - ->mustRun(); - } - - /** - * @inheritDoc - */ - public function revert(string $patch) - { - $this->processFactory->create(['git', 'apply', '--reverse'], $patch) - ->mustRun(); - } - - /** - * @inheritDoc - */ - public function applyCheck(string $patch) - { - $this->processFactory->create(['git', 'apply', '--check'], $patch) - ->mustRun(); - } - - /** - * @inheritDoc - */ - public function revertCheck(string $patch) - { - $this->processFactory->create(['git', 'apply', '--reverse', '--check'], $patch) - ->mustRun(); - } - - /** - * @inheritDoc - */ - public function isInstalled(): bool - { - try { - $this->processFactory->create(['git', '--version'])->mustRun(); - $result = true; - } catch (ProcessFailedException $exception) { - $result = false; - } - - return $result; - } -} diff --git a/src/Patch/PatchCommand.php b/src/Patch/PatchCommand.php index a38b4a4..2097ba6 100644 --- a/src/Patch/PatchCommand.php +++ b/src/Patch/PatchCommand.php @@ -7,23 +7,28 @@ namespace Magento\CloudPatches\Patch; -use Magento\CloudPatches\Shell\ProcessFactory; -use Symfony\Component\Process\Exception\ProcessFailedException; - /** - * Patch command for unix patch + * Patch command selector */ class PatchCommand implements PatchCommandInterface { /** - * @var ProcessFactory + * @var PatchCommandInterface[] + */ + private $commands; + + /** + * @var PatchCommandInterface */ - private $processFactory; + private $command; + /** + * @param PatchCommandInterface[] $commands + */ public function __construct( - ProcessFactory $processFactory + array $commands ) { - $this->processFactory = $processFactory; + $this->commands = $commands; } /** @@ -31,9 +36,7 @@ public function __construct( */ public function apply(string $patch) { - $this->applyCheck($patch); - $this->processFactory->create(['patch', '--silent', '-f', '-p1'], $patch) - ->mustRun(); + $this->getCommand()->apply($patch); } /** @@ -41,9 +44,7 @@ public function apply(string $patch) */ public function revert(string $patch) { - $this->revertCheck($patch); - $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--reverse'], $patch) - ->mustRun(); + $this->getCommand()->revert($patch); } /** @@ -51,8 +52,7 @@ public function revert(string $patch) */ public function applyCheck(string $patch) { - $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--dry-run'], $patch) - ->mustRun(); + $this->getCommand()->applyCheck($patch); } /** @@ -60,8 +60,7 @@ public function applyCheck(string $patch) */ public function revertCheck(string $patch) { - $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--reverse', '--dry-run'], $patch) - ->mustRun(); + $this->getCommand()->revertCheck($patch); } /** @@ -69,13 +68,28 @@ public function revertCheck(string $patch) */ public function isInstalled(): bool { - try { - $this->processFactory->create(['patch', '--version'])->mustRun(); - $result = true; - } catch (ProcessFailedException $exception) { - $result = false; - } + return $this->getCommand()->isInstalled(); + } - return $result; + /** + * Return first available command + * + * @return PatchCommandInterface + * @throws PatchCommandNotFound + */ + private function getCommand(): PatchCommandInterface + { + if ($this->command === null) { + foreach ($this->commands as $command) { + if ($command->isInstalled()) { + $this->command = $command; + break; + } + } + if ($this->command === null) { + throw new PatchCommandNotFound(); + } + } + return $this->command; } } diff --git a/src/Patch/PatchCommandException.php b/src/Patch/PatchCommandException.php new file mode 100644 index 0000000..3bcda3d --- /dev/null +++ b/src/Patch/PatchCommandException.php @@ -0,0 +1,17 @@ +commands = $commands; - } - - /** - * @inheritDoc - */ - public function apply(string $patch) - { - $this->getCommand()->apply($patch); - } - - /** - * @inheritDoc - */ - public function revert(string $patch) - { - $this->getCommand()->revert($patch); - } - - /** - * @inheritDoc - */ - public function applyCheck(string $patch) - { - $this->getCommand()->applyCheck($patch); - } - - /** - * @inheritDoc - */ - public function revertCheck(string $patch) - { - $this->getCommand()->revertCheck($patch); - } - - /** - * @inheritDoc - */ - public function isInstalled(): bool - { - return $this->getCommand()->isInstalled(); - } - - /** - * Return first available command - */ - private function getCommand() - { - foreach ($this->commands as $command) { - if ($command->isInstalled()) { - return $command; - } - } - throw new PatchCommandNotFound(); - } -} diff --git a/src/Shell/Command/DriverInterface.php b/src/Shell/Command/DriverInterface.php new file mode 100644 index 0000000..25f44ea --- /dev/null +++ b/src/Shell/Command/DriverInterface.php @@ -0,0 +1,23 @@ +processFactory = $processFactory; + } + + /** + * @inheritDoc + */ + public function apply(string $patch) + { + try { + $this->processFactory->create(['git', 'apply'], $patch) + ->mustRun(); + } catch (ProcessFailedException $exception) { + throw new PatchCommandException('Failed to apply patch', $exception->getCode(), $exception); + } + } + + /** + * @inheritDoc + */ + public function revert(string $patch) + { + try { + $this->processFactory->create(['git', 'apply', '--reverse'], $patch) + ->mustRun(); + } catch (ProcessFailedException $exception) { + throw new PatchCommandException('Failed to revert patch', $exception->getCode(), $exception); + } + } + + /** + * @inheritDoc + */ + public function applyCheck(string $patch) + { + try { + $this->processFactory->create(['git', 'apply', '--check'], $patch) + ->mustRun(); + } catch (ProcessFailedException $exception) { + throw new PatchCommandException('Patch cannot be applied', $exception->getCode(), $exception); + } + } + + /** + * @inheritDoc + */ + public function revertCheck(string $patch) + { + try { + $this->processFactory->create(['git', 'apply', '--reverse', '--check'], $patch) + ->mustRun(); + } catch (ProcessFailedException $exception) { + throw new PatchCommandException('Patch cannot be reverted', $exception->getCode(), $exception); + } + } + + /** + * @inheritDoc + */ + public function isInstalled(): bool + { + try { + $this->processFactory->create(['git', '--version'])->mustRun(); + $result = true; + } catch (ProcessFailedException $exception) { + $result = false; + } + + return $result; + } +} diff --git a/src/Shell/Command/PatchDriver.php b/src/Shell/Command/PatchDriver.php new file mode 100644 index 0000000..2daa62d --- /dev/null +++ b/src/Shell/Command/PatchDriver.php @@ -0,0 +1,98 @@ +processFactory = $processFactory; + } + + /** + * @inheritDoc + */ + public function apply(string $patch) + { + try { + $this->applyCheck($patch); + $this->processFactory->create(['patch', '--silent', '-f', '-p1'], $patch) + ->mustRun(); + } catch (ProcessFailedException $exception) { + throw new PatchCommandException('Failed to apply patch', $exception->getCode(), $exception); + } + } + + /** + * @inheritDoc + */ + public function revert(string $patch) + { + try { + $this->revertCheck($patch); + $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--reverse'], $patch) + ->mustRun(); + } catch (ProcessFailedException $exception) { + throw new PatchCommandException('Failed to revert patch', $exception->getCode(), $exception); + } + } + + /** + * @inheritDoc + */ + public function applyCheck(string $patch) + { + try { + $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--dry-run'], $patch) + ->mustRun(); + } catch (ProcessFailedException $exception) { + throw new PatchCommandException('Patch cannot be applied', $exception->getCode(), $exception); + } + } + + /** + * @inheritDoc + */ + public function revertCheck(string $patch) + { + try { + $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--reverse', '--dry-run'], $patch) + ->mustRun(); + } catch (ProcessFailedException $exception) { + throw new PatchCommandException('Patch cannot be reverted', $exception->getCode(), $exception); + } + } + + /** + * @inheritDoc + */ + public function isInstalled(): bool + { + try { + $this->processFactory->create(['patch', '--version'])->mustRun(); + $result = true; + } catch (ProcessFailedException $exception) { + $result = false; + } + + return $result; + } +} diff --git a/src/Test/Unit/Patch/ApplierTest.php b/src/Test/Unit/Patch/ApplierTest.php index 91a3197..cd08b37 100644 --- a/src/Test/Unit/Patch/ApplierTest.php +++ b/src/Test/Unit/Patch/ApplierTest.php @@ -12,12 +12,11 @@ use Magento\CloudPatches\Patch\Applier; use Magento\CloudPatches\Patch\ApplierException; use Magento\CloudPatches\Patch\GitConverter; +use Magento\CloudPatches\Patch\PatchCommandException; use Magento\CloudPatches\Patch\PatchCommandInterface; use Magento\CloudPatches\Patch\Status\StatusPool; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Symfony\Component\Process\Exception\ProcessFailedException; -use Symfony\Component\Process\Process; /** * @inheritDoc @@ -104,11 +103,11 @@ public function testApplyFailed() $this->patchCommand->expects($this->once()) ->method('apply') - ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); + ->willThrowException(new PatchCommandException('Patch cannot be applied')); $this->patchCommand->expects($this->once()) ->method('revertCheck') - ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); + ->willThrowException(new PatchCommandException('Patch cannot be reverted')); $this->expectException(ApplierException::class); $this->applier->apply($path, $patchId); @@ -137,7 +136,7 @@ public function testApplyPatchAlreadyApplied() $this->patchCommand->expects($this->once()) ->method('apply') ->with('patchContent') - ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); + ->willThrowException(new PatchCommandException('Patch cannot be applied')); $this->patchCommand->expects($this->once()) ->method('revertCheck') @@ -184,11 +183,11 @@ public function testRevertFailed() $this->patchCommand->expects($this->once()) ->method('revert') - ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); + ->willThrowException(new PatchCommandException('Patch cannot be reverted')); $this->patchCommand->expects($this->once()) ->method('applyCheck') - ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); + ->willThrowException(new PatchCommandException('Patch cannot be applied')); $this->expectException(ApplierException::class); $this->applier->revert($path, $patchId); @@ -218,7 +217,7 @@ public function testRevertPatchWasntApplied() $this->patchCommand->expects($this->once()) ->method('revert') ->with($patchContent) - ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); + ->willThrowException(new PatchCommandException('Patch cannot be reverted')); $this->patchCommand->expects($this->once()) ->method('applyCheck') @@ -251,12 +250,12 @@ public function testStatusNotAvailable() $this->patchCommand->expects($this->once()) ->method('applyCheck') ->with($patchContent) - ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); + ->willThrowException(new PatchCommandException('Patch cannot be applied')); $this->patchCommand->expects($this->once()) ->method('revertCheck') ->with($patchContent) - ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); + ->willThrowException(new PatchCommandException('Patch cannot be reverted')); $this->assertSame(StatusPool::NA, $this->applier->status($patchContent)); } @@ -271,7 +270,7 @@ public function testStatusApplied() $this->patchCommand->expects($this->once()) ->method('applyCheck') ->with($patchContent) - ->willThrowException(new ProcessFailedException($this->createMock(Process::class))); + ->willThrowException(new PatchCommandException('Patch cannot be applied')); $this->patchCommand->expects($this->once()) ->method('revertCheck') diff --git a/src/Test/Unit/Shell/Command/PatchDriverTest.php b/src/Test/Unit/Shell/Command/PatchDriverTest.php new file mode 100644 index 0000000..d1462d5 --- /dev/null +++ b/src/Test/Unit/Shell/Command/PatchDriverTest.php @@ -0,0 +1,187 @@ +baseDir = dirname(__DIR__, 5) . '/tests/unit/'; + $this->cwd = $this->baseDir . 'var/'; + $processFactory = $this->createMock(ProcessFactory::class); + $processFactory->method('create') + ->willReturnCallback( + function (array $cmd, string $input = null) { + return new Process( + $cmd, + $this->cwd, + null, + $input + ); + } + ); + $this->command = new PatchDriver( + $processFactory + ); + } + + /** + * @inheritDoc + */ + protected function tearDown() + { + foreach (glob($this->cwd . '*') as $file) { + if (is_file($file)) { + unlink($file); + } + } + parent::tearDown(); + } + + /** + * Tests that patch is applied + */ + public function testApply() + { + $this->copyFileToWorkingDir($this->getFixtureFile('file1.md')); + $patchContent = $this->getFileContent($this->getFixtureFile('file1.patch')); + $this->command->apply($patchContent); + $expected = $this->getFileContent($this->getFixtureFile('file1_applied_patch.md')); + $actual = $this->getFileContent($this->getVarFile('file1.md')); + $this->assertEquals($expected, $actual); + } + + /** + * Tests that patch is not applied to any target files if an error occurs + */ + public function testApplyFailure() + { + $this->copyFileToWorkingDir($this->getFixtureFile('file1.md')); + $this->copyFileToWorkingDir($this->getFixtureFile('file2_applied_patch.md'), 'file2.md'); + $patchContent = $this->getFileContent($this->getFixtureFile('file1_and_file2.patch')); + $exception = null; + try { + $this->command->apply($patchContent); + } catch (PatchCommandException $e) { + $exception = $e; + } + $this->assertNotNull($exception); + $expected = $this->getFileContent($this->getFixtureFile('file1.md')); + $actual = $this->getFileContent($this->getVarFile('file1.md')); + $this->assertEquals($expected, $actual); + $expected = $this->getFileContent($this->getFixtureFile('file2_applied_patch.md')); + $actual = $this->getFileContent($this->getVarFile('file2.md')); + $this->assertEquals($expected, $actual); + } + + /** + * Tests that patch is reverted + */ + public function testRevert() + { + $this->copyFileToWorkingDir($this->getFixtureFile('file1_applied_patch.md'), 'file1.md'); + $patchContent = $this->getFileContent($this->getFixtureFile('file1.patch')); + $this->command->revert($patchContent); + $expected = $this->getFileContent($this->getFixtureFile('file1.md')); + $actual = $this->getFileContent($this->getVarFile('file1.md')); + $this->assertEquals($expected, $actual); + } + + /** + * Tests that patch is not reverted in any target files if an error occurs + */ + public function testRevertFailure() + { + $this->copyFileToWorkingDir($this->getFixtureFile('file1_applied_patch.md'), 'file1.md'); + $this->copyFileToWorkingDir($this->getFixtureFile('file2.md')); + $patchContent = $this->getFileContent($this->getFixtureFile('file1_and_file2.patch')); + $exception = null; + try { + $this->command->revert($patchContent); + } catch (PatchCommandException $e) { + $exception = $e; + } + $this->assertNotNull($exception); + $expected = $this->getFileContent($this->getFixtureFile('file1_applied_patch.md')); + $actual = $this->getFileContent($this->getVarFile('file1.md')); + $this->assertEquals($expected, $actual); + $expected = $this->getFileContent($this->getFixtureFile('file2.md')); + $actual = $this->getFileContent($this->getVarFile('file2.md')); + $this->assertEquals($expected, $actual); + } + + /** + * Get file path in var directory + * + * @param string $name + * @return string + */ + private function getVarFile(string $name): string + { + return $this->cwd . $name; + } + + /** + * Get file path in files directory + * + * @param string $name + * @return string + */ + private function getFixtureFile(string $name): string + { + return $this->baseDir . '_data/files/' . $name; + } + + /** + * Get the file content + * + * @param string $path + * @return string + */ + private function getFileContent(string $path): string + { + return file_get_contents($path); + } + + /** + * Copy file to working directory + * + * @param string $path + * @param string|null $name + */ + private function copyFileToWorkingDir(string $path, string $name = null) + { + $name = $name ?? basename($path); + copy($path, $this->getVarFile($name)); + } +} diff --git a/tests/unit/_data/files/file1.md b/tests/unit/_data/files/file1.md new file mode 100644 index 0000000..abe4d18 --- /dev/null +++ b/tests/unit/_data/files/file1.md @@ -0,0 +1 @@ +## File 1 diff --git a/tests/unit/_data/files/file1.patch b/tests/unit/_data/files/file1.patch new file mode 100644 index 0000000..9c259fe --- /dev/null +++ b/tests/unit/_data/files/file1.patch @@ -0,0 +1,9 @@ +diff --git a/file1.md b/file1.md +index abe4d18..2f8298f 100644 +--- a/file1.md ++++ b/file1.md +@@ -1 +1,3 @@ +-## File 1 ++# File One ++ ++## Description diff --git a/tests/unit/_data/files/file1_and_file2.patch b/tests/unit/_data/files/file1_and_file2.patch new file mode 100644 index 0000000..f2dcb41 --- /dev/null +++ b/tests/unit/_data/files/file1_and_file2.patch @@ -0,0 +1,18 @@ +diff --git a/file1.md b/file1.md +index abe4d18..2f8298f 100644 +--- a/file1.md ++++ b/file1.md +@@ -1 +1,3 @@ +-## File 1 ++# File One ++ ++## Description +diff --git a/file2.md b/file2.md +index 37ae4d5..c49d210 100644 +--- a/file2.md ++++ b/file2.md +@@ -1 +1,3 @@ +-## File 2 ++# File Two ++ ++## Description diff --git a/tests/unit/_data/files/file1_applied_patch.md b/tests/unit/_data/files/file1_applied_patch.md new file mode 100644 index 0000000..2f8298f --- /dev/null +++ b/tests/unit/_data/files/file1_applied_patch.md @@ -0,0 +1,3 @@ +# File One + +## Description diff --git a/tests/unit/_data/files/file2.md b/tests/unit/_data/files/file2.md new file mode 100644 index 0000000..37ae4d5 --- /dev/null +++ b/tests/unit/_data/files/file2.md @@ -0,0 +1 @@ +## File 2 diff --git a/tests/unit/_data/files/file2_applied_patch.md b/tests/unit/_data/files/file2_applied_patch.md new file mode 100644 index 0000000..c49d210 --- /dev/null +++ b/tests/unit/_data/files/file2_applied_patch.md @@ -0,0 +1,3 @@ +# File Two + +## Description diff --git a/tests/unit/var/.gitignore b/tests/unit/var/.gitignore new file mode 100644 index 0000000..d6b7ef3 --- /dev/null +++ b/tests/unit/var/.gitignore @@ -0,0 +1,2 @@ +* +!.gitignore From e4a801d307d6dfbad3aa7f07e517eb31a593e0c6 Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Mon, 21 Sep 2020 17:00:46 -0500 Subject: [PATCH 05/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available --- config/services.xml | 1 + src/Patch/PatchCommand.php | 8 -------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/config/services.xml b/config/services.xml index d635b9a..04e0098 100644 --- a/config/services.xml +++ b/config/services.xml @@ -20,6 +20,7 @@ + diff --git a/src/Patch/PatchCommand.php b/src/Patch/PatchCommand.php index 2097ba6..7f64689 100644 --- a/src/Patch/PatchCommand.php +++ b/src/Patch/PatchCommand.php @@ -63,14 +63,6 @@ public function revertCheck(string $patch) $this->getCommand()->revertCheck($patch); } - /** - * @inheritDoc - */ - public function isInstalled(): bool - { - return $this->getCommand()->isInstalled(); - } - /** * Return first available command * From 68a4f8c4779b16c109c7c3b506e59cc624724575 Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Mon, 21 Sep 2020 17:23:02 -0500 Subject: [PATCH 06/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available --- src/Patch/PatchCommand.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Patch/PatchCommand.php b/src/Patch/PatchCommand.php index 7f64689..4241768 100644 --- a/src/Patch/PatchCommand.php +++ b/src/Patch/PatchCommand.php @@ -7,23 +7,25 @@ namespace Magento\CloudPatches\Patch; +use Magento\CloudPatches\Shell\Command\DriverInterface; + /** * Patch command selector */ class PatchCommand implements PatchCommandInterface { /** - * @var PatchCommandInterface[] + * @var DriverInterface[] */ private $commands; /** - * @var PatchCommandInterface + * @var DriverInterface */ private $command; /** - * @param PatchCommandInterface[] $commands + * @param DriverInterface[] $commands */ public function __construct( array $commands @@ -66,10 +68,10 @@ public function revertCheck(string $patch) /** * Return first available command * - * @return PatchCommandInterface + * @return DriverInterface * @throws PatchCommandNotFound */ - private function getCommand(): PatchCommandInterface + private function getCommand(): DriverInterface { if ($this->command === null) { foreach ($this->commands as $command) { From 661c46429ff78fa61d9905479ffa8e93a709a3ed Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Mon, 21 Sep 2020 17:35:40 -0500 Subject: [PATCH 07/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available --- config/services.xml | 2 +- src/Patch/PatchCommand.php | 34 +++++++++++++-------------- src/Patch/PatchCommandInterface.php | 4 ++-- src/Patch/PatchCommandNotFound.php | 3 +++ src/Shell/Command/DriverInterface.php | 2 +- src/Shell/Command/GitDriver.php | 3 +++ src/Shell/Command/PatchDriver.php | 3 +++ 7 files changed, 30 insertions(+), 21 deletions(-) diff --git a/config/services.xml b/config/services.xml index 04e0098..7264d41 100644 --- a/config/services.xml +++ b/config/services.xml @@ -79,7 +79,7 @@ - + diff --git a/src/Patch/PatchCommand.php b/src/Patch/PatchCommand.php index 4241768..a96baf4 100644 --- a/src/Patch/PatchCommand.php +++ b/src/Patch/PatchCommand.php @@ -17,20 +17,20 @@ class PatchCommand implements PatchCommandInterface /** * @var DriverInterface[] */ - private $commands; + private $drivers; /** * @var DriverInterface */ - private $command; + private $driver; /** - * @param DriverInterface[] $commands + * @param DriverInterface[] $drivers */ public function __construct( - array $commands + array $drivers ) { - $this->commands = $commands; + $this->drivers = $drivers; } /** @@ -38,7 +38,7 @@ public function __construct( */ public function apply(string $patch) { - $this->getCommand()->apply($patch); + $this->getDriver()->apply($patch); } /** @@ -46,7 +46,7 @@ public function apply(string $patch) */ public function revert(string $patch) { - $this->getCommand()->revert($patch); + $this->getDriver()->revert($patch); } /** @@ -54,7 +54,7 @@ public function revert(string $patch) */ public function applyCheck(string $patch) { - $this->getCommand()->applyCheck($patch); + $this->getDriver()->applyCheck($patch); } /** @@ -62,28 +62,28 @@ public function applyCheck(string $patch) */ public function revertCheck(string $patch) { - $this->getCommand()->revertCheck($patch); + $this->getDriver()->revertCheck($patch); } /** - * Return first available command + * Returns first available driver * * @return DriverInterface * @throws PatchCommandNotFound */ - private function getCommand(): DriverInterface + private function getDriver(): DriverInterface { - if ($this->command === null) { - foreach ($this->commands as $command) { - if ($command->isInstalled()) { - $this->command = $command; + if ($this->driver === null) { + foreach ($this->drivers as $driver) { + if ($driver->isInstalled()) { + $this->driver = $driver; break; } } - if ($this->command === null) { + if ($this->driver === null) { throw new PatchCommandNotFound(); } } - return $this->command; + return $this->driver; } } diff --git a/src/Patch/PatchCommandInterface.php b/src/Patch/PatchCommandInterface.php index 04e80ae..4b59757 100644 --- a/src/Patch/PatchCommandInterface.php +++ b/src/Patch/PatchCommandInterface.php @@ -31,7 +31,7 @@ public function apply(string $patch); public function revert(string $patch); /** - * Checks if the patch can be applied. + * Checks if patch can be applied. * * @param string $patch * @return void @@ -40,7 +40,7 @@ public function revert(string $patch); public function applyCheck(string $patch); /** - * Checks if the patch can be reversed + * Checks if patch can be reverted * * @param string $patch * @return void diff --git a/src/Patch/PatchCommandNotFound.php b/src/Patch/PatchCommandNotFound.php index b316417..70a23e4 100644 --- a/src/Patch/PatchCommandNotFound.php +++ b/src/Patch/PatchCommandNotFound.php @@ -9,6 +9,9 @@ use Magento\CloudPatches\App\GenericException; +/** + * Exception thrown if none of defined patch drivers is available + */ class PatchCommandNotFound extends GenericException { public function __construct() diff --git a/src/Shell/Command/DriverInterface.php b/src/Shell/Command/DriverInterface.php index 25f44ea..9ac8d86 100644 --- a/src/Shell/Command/DriverInterface.php +++ b/src/Shell/Command/DriverInterface.php @@ -10,7 +10,7 @@ use Magento\CloudPatches\Patch\PatchCommandInterface; /** - * Patch driver interface + * Patch command driver interface */ interface DriverInterface extends PatchCommandInterface { diff --git a/src/Shell/Command/GitDriver.php b/src/Shell/Command/GitDriver.php index 385168e..ec005a9 100644 --- a/src/Shell/Command/GitDriver.php +++ b/src/Shell/Command/GitDriver.php @@ -21,6 +21,9 @@ class GitDriver implements DriverInterface */ private $processFactory; + /** + * @param ProcessFactory $processFactory + */ public function __construct( ProcessFactory $processFactory ) { diff --git a/src/Shell/Command/PatchDriver.php b/src/Shell/Command/PatchDriver.php index 2daa62d..6461b7b 100644 --- a/src/Shell/Command/PatchDriver.php +++ b/src/Shell/Command/PatchDriver.php @@ -21,6 +21,9 @@ class PatchDriver implements DriverInterface */ private $processFactory; + /** + * @param ProcessFactory $processFactory + */ public function __construct( ProcessFactory $processFactory ) { From 7643add5c5485d978be0b63db4f6ce4b514b2837 Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Tue, 22 Sep 2020 13:13:21 -0500 Subject: [PATCH 08/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available - Remove -f flag in patch command - Fix services configuration --- config/services.xml | 5 ++--- src/Shell/Command/PatchDriver.php | 8 ++++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/config/services.xml b/config/services.xml index 7264d41..92357b1 100644 --- a/config/services.xml +++ b/config/services.xml @@ -29,6 +29,8 @@ + + @@ -84,8 +86,5 @@ - - - diff --git a/src/Shell/Command/PatchDriver.php b/src/Shell/Command/PatchDriver.php index 6461b7b..8bf9076 100644 --- a/src/Shell/Command/PatchDriver.php +++ b/src/Shell/Command/PatchDriver.php @@ -37,7 +37,7 @@ public function apply(string $patch) { try { $this->applyCheck($patch); - $this->processFactory->create(['patch', '--silent', '-f', '-p1'], $patch) + $this->processFactory->create(['patch', '--silent', '-p1', '--no-backup-if-mismatch'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { throw new PatchCommandException('Failed to apply patch', $exception->getCode(), $exception); @@ -51,7 +51,7 @@ public function revert(string $patch) { try { $this->revertCheck($patch); - $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--reverse'], $patch) + $this->processFactory->create(['patch', '--silent', '-p1', '--no-backup-if-mismatch', '--reverse'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { throw new PatchCommandException('Failed to revert patch', $exception->getCode(), $exception); @@ -64,7 +64,7 @@ public function revert(string $patch) public function applyCheck(string $patch) { try { - $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--dry-run'], $patch) + $this->processFactory->create(['patch', '--silent', '-p1', '--dry-run'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { throw new PatchCommandException('Patch cannot be applied', $exception->getCode(), $exception); @@ -77,7 +77,7 @@ public function applyCheck(string $patch) public function revertCheck(string $patch) { try { - $this->processFactory->create(['patch', '--silent', '-f', '-p1', '--reverse', '--dry-run'], $patch) + $this->processFactory->create(['patch', '--silent', '-p1', '--reverse', '--dry-run'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { throw new PatchCommandException('Patch cannot be reverted', $exception->getCode(), $exception); From 5754d25b74d8d091d2efec52b592bf443d38a0c4 Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Tue, 22 Sep 2020 13:15:08 -0500 Subject: [PATCH 09/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available - Fix services configuration --- config/services.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/services.xml b/config/services.xml index 92357b1..1bc514d 100644 --- a/config/services.xml +++ b/config/services.xml @@ -80,7 +80,7 @@ - + From 0060375d3ea85cd63790d8bcfb00f85275a9a3b6 Mon Sep 17 00:00:00 2001 From: Viktor Tymchynskyi Date: Wed, 23 Sep 2020 20:55:00 -0500 Subject: [PATCH 10/15] - Update information string --- src/Command/Process/ShowStatus.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Command/Process/ShowStatus.php b/src/Command/Process/ShowStatus.php index 181366e..0ae5294 100644 --- a/src/Command/Process/ShowStatus.php +++ b/src/Command/Process/ShowStatus.php @@ -111,9 +111,14 @@ function ($patch) { */ private function printDetailsInfo(OutputInterface $output) { + $supportUrl = 'https://support.magento.com'; + $releaseNotesUrl = 'https://devdocs.magento.com/quality-patches/release-notes.html'; + $output->writeln( - 'More detailed information about patches you can find on ' . - 'https://support.magento.com' + 'Patch details you can find on ' . + sprintf('%1$s (search for patch id, ex. MDVA-30265)', $supportUrl) . + PHP_EOL . + sprintf('Release notes %1$s', $releaseNotesUrl) ); } From 6cbccf8352720a3a84680fcbd273f63a2e9fd449 Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Thu, 24 Sep 2020 15:21:56 -0500 Subject: [PATCH 11/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available - Fix error message formatting --- config/services.xml | 7 +- src/Shell/Command/DriverException.php | 55 +++++++ src/Shell/Command/GitDriver.php | 9 +- src/Shell/Command/GitDriverException.php | 29 ++++ src/Shell/Command/PatchDriver.php | 9 +- src/Shell/Command/PatchDriverException.php | 29 ++++ .../Shell/Command/DriverExceptionTest.php | 138 ++++++++++++++++++ 7 files changed, 264 insertions(+), 12 deletions(-) create mode 100644 src/Shell/Command/DriverException.php create mode 100644 src/Shell/Command/GitDriverException.php create mode 100644 src/Shell/Command/PatchDriverException.php create mode 100644 src/Test/Unit/Shell/Command/DriverExceptionTest.php diff --git a/config/services.xml b/config/services.xml index 1bc514d..d9ce683 100644 --- a/config/services.xml +++ b/config/services.xml @@ -19,8 +19,6 @@ - - @@ -29,6 +27,11 @@ + + + + + diff --git a/src/Shell/Command/DriverException.php b/src/Shell/Command/DriverException.php new file mode 100644 index 0000000..ea5b01a --- /dev/null +++ b/src/Shell/Command/DriverException.php @@ -0,0 +1,55 @@ +formatMessage($message), $code, $previous); + } + + /** + * Format error message + * + * @param string $message + * @return string + */ + private function formatMessage(string $message): string + { + $result = $message; + $errorMsg = null; + $generalMsg = null; + if (preg_match('#^.*?Error Output:(?.*?)$#is', $result, $matches)) { + $errorMsg = PHP_EOL .'Error Output:' . $matches['errors']; + $result = str_replace($errorMsg, '', $result); + if (!trim(str_replace('=', '', $matches['errors']))) { + $errorMsg = null; + } + } + if (empty($errorMsg) && preg_match('#^.*?Output:(?.*?)$#is', $result, $matches)) { + $generalMsg = PHP_EOL .'Output:' . $matches['errors']; + if (!trim(str_replace('=', '', $matches['errors']))) { + $generalMsg = null; + } + } + + return $errorMsg ?? $generalMsg ?? $message; + } +} diff --git a/src/Shell/Command/GitDriver.php b/src/Shell/Command/GitDriver.php index ec005a9..16b6a56 100644 --- a/src/Shell/Command/GitDriver.php +++ b/src/Shell/Command/GitDriver.php @@ -7,7 +7,6 @@ namespace Magento\CloudPatches\Shell\Command; -use Magento\CloudPatches\Patch\PatchCommandException; use Magento\CloudPatches\Shell\ProcessFactory; use Symfony\Component\Process\Exception\ProcessFailedException; @@ -39,7 +38,7 @@ public function apply(string $patch) $this->processFactory->create(['git', 'apply'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new PatchCommandException('Failed to apply patch', $exception->getCode(), $exception); + throw new GitDriverException($exception); } } @@ -52,7 +51,7 @@ public function revert(string $patch) $this->processFactory->create(['git', 'apply', '--reverse'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new PatchCommandException('Failed to revert patch', $exception->getCode(), $exception); + throw new GitDriverException($exception); } } @@ -65,7 +64,7 @@ public function applyCheck(string $patch) $this->processFactory->create(['git', 'apply', '--check'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new PatchCommandException('Patch cannot be applied', $exception->getCode(), $exception); + throw new GitDriverException($exception); } } @@ -78,7 +77,7 @@ public function revertCheck(string $patch) $this->processFactory->create(['git', 'apply', '--reverse', '--check'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new PatchCommandException('Patch cannot be reverted', $exception->getCode(), $exception); + throw new GitDriverException($exception); } } diff --git a/src/Shell/Command/GitDriverException.php b/src/Shell/Command/GitDriverException.php new file mode 100644 index 0000000..490af43 --- /dev/null +++ b/src/Shell/Command/GitDriverException.php @@ -0,0 +1,29 @@ +getMessage(); + if ($previous instanceof ProcessFailedException) { + $message = $previous->getProcess()->getErrorOutput() ?: ($previous->getProcess()->getOutput() ?: $message); + } + parent::__construct($message, $previous->getCode(), $previous); + } +} diff --git a/src/Shell/Command/PatchDriver.php b/src/Shell/Command/PatchDriver.php index 8bf9076..11b5388 100644 --- a/src/Shell/Command/PatchDriver.php +++ b/src/Shell/Command/PatchDriver.php @@ -7,7 +7,6 @@ namespace Magento\CloudPatches\Shell\Command; -use Magento\CloudPatches\Patch\PatchCommandException; use Magento\CloudPatches\Shell\ProcessFactory; use Symfony\Component\Process\Exception\ProcessFailedException; @@ -40,7 +39,7 @@ public function apply(string $patch) $this->processFactory->create(['patch', '--silent', '-p1', '--no-backup-if-mismatch'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new PatchCommandException('Failed to apply patch', $exception->getCode(), $exception); + throw new PatchDriverException($exception); } } @@ -54,7 +53,7 @@ public function revert(string $patch) $this->processFactory->create(['patch', '--silent', '-p1', '--no-backup-if-mismatch', '--reverse'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new PatchCommandException('Failed to revert patch', $exception->getCode(), $exception); + throw new PatchDriverException($exception); } } @@ -67,7 +66,7 @@ public function applyCheck(string $patch) $this->processFactory->create(['patch', '--silent', '-p1', '--dry-run'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new PatchCommandException('Patch cannot be applied', $exception->getCode(), $exception); + throw new PatchDriverException($exception); } } @@ -80,7 +79,7 @@ public function revertCheck(string $patch) $this->processFactory->create(['patch', '--silent', '-p1', '--reverse', '--dry-run'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new PatchCommandException('Patch cannot be reverted', $exception->getCode(), $exception); + throw new PatchDriverException($exception); } } diff --git a/src/Shell/Command/PatchDriverException.php b/src/Shell/Command/PatchDriverException.php new file mode 100644 index 0000000..39b09f8 --- /dev/null +++ b/src/Shell/Command/PatchDriverException.php @@ -0,0 +1,29 @@ +getMessage(); + if ($previous instanceof ProcessFailedException) { + $message = $previous->getProcess()->getErrorOutput() ?: ($previous->getProcess()->getOutput() ?: $message); + } + parent::__construct($message, $previous->getCode(), $previous); + } +} diff --git a/src/Test/Unit/Shell/Command/DriverExceptionTest.php b/src/Test/Unit/Shell/Command/DriverExceptionTest.php new file mode 100644 index 0000000..f55e3a0 --- /dev/null +++ b/src/Test/Unit/Shell/Command/DriverExceptionTest.php @@ -0,0 +1,138 @@ +assertEquals($expectedOutput, $exception->getMessage()); + } + + /** + * @return array + */ + public function formatMessageDataProvider(): array + { + return [ + [ + 'error' => 'The command "\'patch\' \'--silent\' \'-p1\' \'--dry-run\'" failed. + +Exit Code: 1(General error) + +Working directory: /var/www/html + +Output: +================ + + +Error Output: +================ +error: patch failed: path/to/path/file2.php b/path/to/path/file2.php:23 +error: path/to/path/file2.php b/path/to/path/file2.php: patch does not apply', + + 'expectedOutput' => ' +Error Output: +================ +error: patch failed: path/to/path/file2.php b/path/to/path/file2.php:23 +error: path/to/path/file2.php b/path/to/path/file2.php: patch does not apply' + ], + [ + 'error' => 'The command "\'patch\' \'--silent\' \'-p1\' \'--dry-run\'" failed. + +Exit Code: 1(General error) + +Working directory: /var/www/html + +Output: +================ +Hmm... Looks like a unified diff to me... +The text leading up to this was: +-------------------------- +|diff --git a/path/to/path/file1.php b/path/to/path/file1.php +|index 320e0adc29b..576281861d3 100644 +|--- a/path/to/path/file1.php +|+++ b/path/to/path/file1.php +-------------------------- +Patching file path/to/path/file1.php using Plan A... +Hunk #1 succeeded at 30. +Hunk #2 succeeded at 54. +Hunk #3 succeeded at 76. +Hunk #4 succeeded at 113. +Hmm... The next patch looks like a unified diff to me... +The text leading up to this was: +-------------------------- +|diff --git a/path/to/path/file2.php b/path/to/path/file2.php +|index 0ec65c88024..e550de9cb03 100644 +|--- a/path/to/path/file2.php +|+++ b/path/to/path/file2.php +-------------------------- +Patching file path/to/path/file2.php using Plan A... +Hunk #1 succeeded at 71. +Hunk #2 FAILED at 136. +Hunk #3 succeeded at 154. +1 out of 3 hunks FAILED -- saving rejects to file path/to/path/file2.php.rej +done + + +Error Output: +================ + +', + + 'expectedOutput' => ' +Output: +================ +Hmm... Looks like a unified diff to me... +The text leading up to this was: +-------------------------- +|diff --git a/path/to/path/file1.php b/path/to/path/file1.php +|index 320e0adc29b..576281861d3 100644 +|--- a/path/to/path/file1.php +|+++ b/path/to/path/file1.php +-------------------------- +Patching file path/to/path/file1.php using Plan A... +Hunk #1 succeeded at 30. +Hunk #2 succeeded at 54. +Hunk #3 succeeded at 76. +Hunk #4 succeeded at 113. +Hmm... The next patch looks like a unified diff to me... +The text leading up to this was: +-------------------------- +|diff --git a/path/to/path/file2.php b/path/to/path/file2.php +|index 0ec65c88024..e550de9cb03 100644 +|--- a/path/to/path/file2.php +|+++ b/path/to/path/file2.php +-------------------------- +Patching file path/to/path/file2.php using Plan A... +Hunk #1 succeeded at 71. +Hunk #2 FAILED at 136. +Hunk #3 succeeded at 154. +1 out of 3 hunks FAILED -- saving rejects to file path/to/path/file2.php.rej +done + +' + ], + [ + 'error' => 'Some other output', + 'expectedOutput' => 'Some other output' + ], + ]; + } +} From ff40569505fa39bde675fc3c7d32d38b99c282ea Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Thu, 24 Sep 2020 16:37:17 -0500 Subject: [PATCH 12/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available - Remove formatErrorOutput --- src/Command/Process/Action/RevertAction.php | 2 +- src/Command/Process/ApplyLocal.php | 2 +- src/Command/Process/Ece/Revert.php | 2 +- src/Command/Process/Renderer.php | 15 ------ src/Patch/Conflict/Processor.php | 2 +- src/Shell/Command/DriverException.php | 4 +- .../Process/Action/RevertActionTest.php | 3 -- .../Unit/Command/Process/ApplyLocalTest.php | 3 -- .../Unit/Command/Process/Ece/RevertTest.php | 4 -- .../Unit/Command/Process/RendererTest.php | 47 ------------------- .../Unit/Patch/Conflict/ProcessorTest.php | 7 +-- .../Shell/Command/DriverExceptionTest.php | 7 ++- 12 files changed, 10 insertions(+), 88 deletions(-) diff --git a/src/Command/Process/Action/RevertAction.php b/src/Command/Process/Action/RevertAction.php index 5c47259..3ebd64e 100644 --- a/src/Command/Process/Action/RevertAction.php +++ b/src/Command/Process/Action/RevertAction.php @@ -179,7 +179,7 @@ private function printPatchRevertingFailed(OutputInterface $output, PatchInterfa 'Reverting patch %s (%s) failed.%s', $patch->getId(), $patch->getPath(), - $this->renderer->formatErrorOutput($errorOutput) + PHP_EOL . $errorOutput ); $this->logger->error($errorMessage); diff --git a/src/Command/Process/ApplyLocal.php b/src/Command/Process/ApplyLocal.php index 3ba8ebd..50db151 100644 --- a/src/Command/Process/ApplyLocal.php +++ b/src/Command/Process/ApplyLocal.php @@ -95,7 +95,7 @@ public function run(InputInterface $input, OutputInterface $output) $errorMessage = sprintf( 'Applying patch %s failed.%s', $patch->getPath(), - $this->renderer->formatErrorOutput($exception->getMessage()) + PHP_EOL . $exception->getMessage() ); throw new RuntimeException($errorMessage, $exception->getCode()); diff --git a/src/Command/Process/Ece/Revert.php b/src/Command/Process/Ece/Revert.php index b2b93b4..876267f 100644 --- a/src/Command/Process/Ece/Revert.php +++ b/src/Command/Process/Ece/Revert.php @@ -121,7 +121,7 @@ function ($patch) { $errorMessage = sprintf( 'Reverting patch %s failed.%s', $patch->getPath(), - $this->renderer->formatErrorOutput($exception->getMessage()) + PHP_EOL . $exception->getMessage() ); $this->printError($output, $errorMessage); } diff --git a/src/Command/Process/Renderer.php b/src/Command/Process/Renderer.php index 319ce7b..8c39bc9 100644 --- a/src/Command/Process/Renderer.php +++ b/src/Command/Process/Renderer.php @@ -140,21 +140,6 @@ public function printPatchInfo( $output->writeln(''); } - /** - * Format error output. - * - * @param string $errorOutput - * @return string - */ - public function formatErrorOutput(string $errorOutput): string - { - if (preg_match('#^.*?Error Output:(?.*?)$#is', $errorOutput, $matches)) { - $errorOutput = PHP_EOL . 'Error Output:' . $matches['errors']; - } - - return $errorOutput; - } - /** * Asks a confirmation question to the user. * diff --git a/src/Patch/Conflict/Processor.php b/src/Patch/Conflict/Processor.php index 2927899..a903b8e 100644 --- a/src/Patch/Conflict/Processor.php +++ b/src/Patch/Conflict/Processor.php @@ -84,7 +84,7 @@ public function process( 'Applying patch %s (%s) failed.%s%s', $patch->getId(), $patch->getPath(), - $this->renderer->formatErrorOutput($exceptionMessage), + PHP_EOL. $exceptionMessage, $conflictDetails ? PHP_EOL . $conflictDetails : '' ); diff --git a/src/Shell/Command/DriverException.php b/src/Shell/Command/DriverException.php index ea5b01a..7c6020f 100644 --- a/src/Shell/Command/DriverException.php +++ b/src/Shell/Command/DriverException.php @@ -37,14 +37,14 @@ private function formatMessage(string $message): string $errorMsg = null; $generalMsg = null; if (preg_match('#^.*?Error Output:(?.*?)$#is', $result, $matches)) { - $errorMsg = PHP_EOL .'Error Output:' . $matches['errors']; + $errorMsg = 'Error Output:' . $matches['errors']; $result = str_replace($errorMsg, '', $result); if (!trim(str_replace('=', '', $matches['errors']))) { $errorMsg = null; } } if (empty($errorMsg) && preg_match('#^.*?Output:(?.*?)$#is', $result, $matches)) { - $generalMsg = PHP_EOL .'Output:' . $matches['errors']; + $generalMsg = 'Output:' . $matches['errors']; if (!trim(str_replace('=', '', $matches['errors']))) { $generalMsg = null; } diff --git a/src/Test/Unit/Command/Process/Action/RevertActionTest.php b/src/Test/Unit/Command/Process/Action/RevertActionTest.php index 4c0e3c5..9ef6b34 100644 --- a/src/Test/Unit/Command/Process/Action/RevertActionTest.php +++ b/src/Test/Unit/Command/Process/Action/RevertActionTest.php @@ -184,9 +184,6 @@ public function testRevertWithException() $this->applier->method('revert') ->willThrowException(new ApplierException('Error')); - $this->renderer->expects($this->once()) - ->method('formatErrorOutput') - ->with('Error'); $outputMock->expects($this->once()) ->method('writeln') ->withConsecutive( diff --git a/src/Test/Unit/Command/Process/ApplyLocalTest.php b/src/Test/Unit/Command/Process/ApplyLocalTest.php index cc6490c..b9f7e85 100644 --- a/src/Test/Unit/Command/Process/ApplyLocalTest.php +++ b/src/Test/Unit/Command/Process/ApplyLocalTest.php @@ -171,9 +171,6 @@ function ($path, $title) { ->method('process') ->withConsecutive([[$patch1]]) ->willReturn($rollbackMessages); - $this->renderer->expects($this->once()) - ->method('formatErrorOutput') - ->with('Applier error message'); $this->expectException(RuntimeException::class); $this->manager->run($inputMock, $outputMock); diff --git a/src/Test/Unit/Command/Process/Ece/RevertTest.php b/src/Test/Unit/Command/Process/Ece/RevertTest.php index 82dacb0..1197ba2 100644 --- a/src/Test/Unit/Command/Process/Ece/RevertTest.php +++ b/src/Test/Unit/Command/Process/Ece/RevertTest.php @@ -165,10 +165,6 @@ function ($path, $title) { } ); - $this->renderer->expects($this->once()) - ->method('formatErrorOutput') - ->with('Applier error message'); - $this->revertAction->expects($this->once()) ->method('execute') ->withConsecutive([$inputMock, $outputMock, []]); diff --git a/src/Test/Unit/Command/Process/RendererTest.php b/src/Test/Unit/Command/Process/RendererTest.php index f78e161..6a944b4 100644 --- a/src/Test/Unit/Command/Process/RendererTest.php +++ b/src/Test/Unit/Command/Process/RendererTest.php @@ -122,53 +122,6 @@ public function printPatchInfoDataProvider(): array ]; } - /** - * Tests error output formatting. - * - * @param string $errorOutput - * @param string $expectedOutput - * @dataProvider formatErrorOutputDataProvider - */ - public function testFormatErrorOutput(string $errorOutput, string $expectedOutput) - { - $this->assertEquals($expectedOutput, $this->renderer->formatErrorOutput($errorOutput)); - } - - /** - * @return array - */ - public function formatErrorOutputDataProvider(): array - { - return [ - [ - 'error' => 'The command "\'git\' \'apply\' \'/path/to/patch/MC-1111_test_patch_1.1.1_ce.patch\'" failed. - -Exit Code: 1(General error) - -Working directory: /path/to/patch - -Output: -================ - - -Error Output: -================ -error: patch failed: vendor/magento/module-admin-analytics/Controller/Adminhtml/Config/DisableAdminUsage.php:23 -error: vendor/magento/module-admin-analytics/Controller/Adminhtml/Config/DisableAdminUsage.php: patch does not apply', - - 'expectedOutput' => ' -Error Output: -================ -error: patch failed: vendor/magento/module-admin-analytics/Controller/Adminhtml/Config/DisableAdminUsage.php:23 -error: vendor/magento/module-admin-analytics/Controller/Adminhtml/Config/DisableAdminUsage.php: patch does not apply' - ], - [ - 'error' => 'Some other output', - 'expectedOutput' => 'Some other output' - ], - ]; - } - /** * Creates patch mock. * diff --git a/src/Test/Unit/Patch/Conflict/ProcessorTest.php b/src/Test/Unit/Patch/Conflict/ProcessorTest.php index 00569d6..6b4d8b8 100644 --- a/src/Test/Unit/Patch/Conflict/ProcessorTest.php +++ b/src/Test/Unit/Patch/Conflict/ProcessorTest.php @@ -76,7 +76,6 @@ public function testProcess() $failedPatch = $this->createPatch('MC-3', 'path3'); $exceptionMessage = 'exceptionMessage'; $conflictDetails = 'Conflict details'; - $formattedOutput = 'formattedOutput'; $rollbackMessages = ['Patch 1 has been reverted', 'Patch 2 has been reverted']; /** @var OutputInterface|MockObject $outputMock */ @@ -90,10 +89,6 @@ public function testProcess() ->method('analyze') ->withConsecutive([$failedPatch]) ->willReturn($conflictDetails); - $this->renderer->expects($this->once()) - ->method('formatErrorOutput') - ->withConsecutive([$exceptionMessage]) - ->willReturn($formattedOutput); $outputMock->expects($this->exactly(2)) ->method('writeln') ->withConsecutive( @@ -105,7 +100,7 @@ public function testProcess() 'Applying patch %s (%s) failed.%s%s', $failedPatch->getId(), $failedPatch->getPath(), - $formattedOutput, + PHP_EOL .$exceptionMessage, PHP_EOL . $conflictDetails ); diff --git a/src/Test/Unit/Shell/Command/DriverExceptionTest.php b/src/Test/Unit/Shell/Command/DriverExceptionTest.php index f55e3a0..ab5c997 100644 --- a/src/Test/Unit/Shell/Command/DriverExceptionTest.php +++ b/src/Test/Unit/Shell/Command/DriverExceptionTest.php @@ -47,8 +47,7 @@ public function formatMessageDataProvider(): array error: patch failed: path/to/path/file2.php b/path/to/path/file2.php:23 error: path/to/path/file2.php b/path/to/path/file2.php: patch does not apply', - 'expectedOutput' => ' -Error Output: + 'expectedOutput' => 'Error Output: ================ error: patch failed: path/to/path/file2.php b/path/to/path/file2.php:23 error: path/to/path/file2.php b/path/to/path/file2.php: patch does not apply' @@ -96,8 +95,7 @@ public function formatMessageDataProvider(): array ', - 'expectedOutput' => ' -Output: + 'expectedOutput' => 'Output: ================ Hmm... Looks like a unified diff to me... The text leading up to this was: @@ -127,6 +125,7 @@ public function formatMessageDataProvider(): array 1 out of 3 hunks FAILED -- saving rejects to file path/to/path/file2.php.rej done + ' ], [ From 60a7dabf79490694af2bdd671422a13cd4889ee9 Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Thu, 24 Sep 2020 18:17:24 -0500 Subject: [PATCH 13/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available - Remove dead code --- src/Shell/Command/DriverException.php | 38 ----- .../Shell/Command/DriverExceptionTest.php | 137 ------------------ 2 files changed, 175 deletions(-) delete mode 100644 src/Test/Unit/Shell/Command/DriverExceptionTest.php diff --git a/src/Shell/Command/DriverException.php b/src/Shell/Command/DriverException.php index 7c6020f..b92a283 100644 --- a/src/Shell/Command/DriverException.php +++ b/src/Shell/Command/DriverException.php @@ -8,48 +8,10 @@ namespace Magento\CloudPatches\Shell\Command; use Magento\CloudPatches\Patch\PatchCommandException; -use Throwable; /** * Patch command driver exception */ class DriverException extends PatchCommandException { - /** - * @param string $message - * @param int $code - * @param Throwable|null $previous - */ - public function __construct(string $message, int $code = 0, Throwable $previous = null) - { - parent::__construct($this->formatMessage($message), $code, $previous); - } - - /** - * Format error message - * - * @param string $message - * @return string - */ - private function formatMessage(string $message): string - { - $result = $message; - $errorMsg = null; - $generalMsg = null; - if (preg_match('#^.*?Error Output:(?.*?)$#is', $result, $matches)) { - $errorMsg = 'Error Output:' . $matches['errors']; - $result = str_replace($errorMsg, '', $result); - if (!trim(str_replace('=', '', $matches['errors']))) { - $errorMsg = null; - } - } - if (empty($errorMsg) && preg_match('#^.*?Output:(?.*?)$#is', $result, $matches)) { - $generalMsg = 'Output:' . $matches['errors']; - if (!trim(str_replace('=', '', $matches['errors']))) { - $generalMsg = null; - } - } - - return $errorMsg ?? $generalMsg ?? $message; - } } diff --git a/src/Test/Unit/Shell/Command/DriverExceptionTest.php b/src/Test/Unit/Shell/Command/DriverExceptionTest.php deleted file mode 100644 index ab5c997..0000000 --- a/src/Test/Unit/Shell/Command/DriverExceptionTest.php +++ /dev/null @@ -1,137 +0,0 @@ -assertEquals($expectedOutput, $exception->getMessage()); - } - - /** - * @return array - */ - public function formatMessageDataProvider(): array - { - return [ - [ - 'error' => 'The command "\'patch\' \'--silent\' \'-p1\' \'--dry-run\'" failed. - -Exit Code: 1(General error) - -Working directory: /var/www/html - -Output: -================ - - -Error Output: -================ -error: patch failed: path/to/path/file2.php b/path/to/path/file2.php:23 -error: path/to/path/file2.php b/path/to/path/file2.php: patch does not apply', - - 'expectedOutput' => 'Error Output: -================ -error: patch failed: path/to/path/file2.php b/path/to/path/file2.php:23 -error: path/to/path/file2.php b/path/to/path/file2.php: patch does not apply' - ], - [ - 'error' => 'The command "\'patch\' \'--silent\' \'-p1\' \'--dry-run\'" failed. - -Exit Code: 1(General error) - -Working directory: /var/www/html - -Output: -================ -Hmm... Looks like a unified diff to me... -The text leading up to this was: --------------------------- -|diff --git a/path/to/path/file1.php b/path/to/path/file1.php -|index 320e0adc29b..576281861d3 100644 -|--- a/path/to/path/file1.php -|+++ b/path/to/path/file1.php --------------------------- -Patching file path/to/path/file1.php using Plan A... -Hunk #1 succeeded at 30. -Hunk #2 succeeded at 54. -Hunk #3 succeeded at 76. -Hunk #4 succeeded at 113. -Hmm... The next patch looks like a unified diff to me... -The text leading up to this was: --------------------------- -|diff --git a/path/to/path/file2.php b/path/to/path/file2.php -|index 0ec65c88024..e550de9cb03 100644 -|--- a/path/to/path/file2.php -|+++ b/path/to/path/file2.php --------------------------- -Patching file path/to/path/file2.php using Plan A... -Hunk #1 succeeded at 71. -Hunk #2 FAILED at 136. -Hunk #3 succeeded at 154. -1 out of 3 hunks FAILED -- saving rejects to file path/to/path/file2.php.rej -done - - -Error Output: -================ - -', - - 'expectedOutput' => 'Output: -================ -Hmm... Looks like a unified diff to me... -The text leading up to this was: --------------------------- -|diff --git a/path/to/path/file1.php b/path/to/path/file1.php -|index 320e0adc29b..576281861d3 100644 -|--- a/path/to/path/file1.php -|+++ b/path/to/path/file1.php --------------------------- -Patching file path/to/path/file1.php using Plan A... -Hunk #1 succeeded at 30. -Hunk #2 succeeded at 54. -Hunk #3 succeeded at 76. -Hunk #4 succeeded at 113. -Hmm... The next patch looks like a unified diff to me... -The text leading up to this was: --------------------------- -|diff --git a/path/to/path/file2.php b/path/to/path/file2.php -|index 0ec65c88024..e550de9cb03 100644 -|--- a/path/to/path/file2.php -|+++ b/path/to/path/file2.php --------------------------- -Patching file path/to/path/file2.php using Plan A... -Hunk #1 succeeded at 71. -Hunk #2 FAILED at 136. -Hunk #3 succeeded at 154. -1 out of 3 hunks FAILED -- saving rejects to file path/to/path/file2.php.rej -done - - -' - ], - [ - 'error' => 'Some other output', - 'expectedOutput' => 'Some other output' - ], - ]; - } -} From 8644f03779aa9523b41a8a8d0681516546a394e3 Mon Sep 17 00:00:00 2001 From: Viktor Tymchynskyi Date: Thu, 24 Sep 2020 18:30:51 -0500 Subject: [PATCH 14/15] - Update error message --- src/Command/Process/ApplyLocal.php | 2 +- src/Patch/Conflict/Processor.php | 4 ++-- src/Test/Unit/Patch/Conflict/ProcessorTest.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Command/Process/ApplyLocal.php b/src/Command/Process/ApplyLocal.php index 50db151..6316fab 100644 --- a/src/Command/Process/ApplyLocal.php +++ b/src/Command/Process/ApplyLocal.php @@ -89,7 +89,7 @@ public function run(InputInterface $input, OutputInterface $output) $this->printInfo($output, $message); array_push($appliedPatches, $patch); } catch (ApplierException $exception) { - $this->printError($output, 'Error: patch conflict happened'); + $this->printError($output, 'Error: patch can\'t be applied'); $messages = $this->rollbackProcessor->process($appliedPatches); $output->writeln($messages); $errorMessage = sprintf( diff --git a/src/Patch/Conflict/Processor.php b/src/Patch/Conflict/Processor.php index a903b8e..68b99de 100644 --- a/src/Patch/Conflict/Processor.php +++ b/src/Patch/Conflict/Processor.php @@ -73,7 +73,7 @@ public function process( array $appliedPatches, string $exceptionMessage ) { - $errorMessage = 'Error: patch conflict happened'; + $errorMessage = 'Error: patch can\'t be applied'; $this->logger->error($errorMessage); $output->writeln('' . $errorMessage . ''); @@ -84,7 +84,7 @@ public function process( 'Applying patch %s (%s) failed.%s%s', $patch->getId(), $patch->getPath(), - PHP_EOL. $exceptionMessage, + PHP_EOL . $exceptionMessage, $conflictDetails ? PHP_EOL . $conflictDetails : '' ); diff --git a/src/Test/Unit/Patch/Conflict/ProcessorTest.php b/src/Test/Unit/Patch/Conflict/ProcessorTest.php index 6b4d8b8..8f09459 100644 --- a/src/Test/Unit/Patch/Conflict/ProcessorTest.php +++ b/src/Test/Unit/Patch/Conflict/ProcessorTest.php @@ -92,7 +92,7 @@ public function testProcess() $outputMock->expects($this->exactly(2)) ->method('writeln') ->withConsecutive( - [$this->stringContains('Error: patch conflict happened')], + [$this->stringContains('Error: patch can\'t be applied')], [$rollbackMessages] ); From 04156562ba97aba766d52149d091b55adef9a558 Mon Sep 17 00:00:00 2001 From: Buba Suma Date: Fri, 25 Sep 2020 07:22:31 -0500 Subject: [PATCH 15/15] MC-37324: Add fallback to 'patch' command when 'git' command is not available - Remove redundant exceptions --- config/services.xml | 2 -- src/Shell/Command/DriverException.php | 13 ++++++++++ src/Shell/Command/GitDriver.php | 8 +++--- src/Shell/Command/GitDriverException.php | 29 ---------------------- src/Shell/Command/PatchDriver.php | 8 +++--- src/Shell/Command/PatchDriverException.php | 29 ---------------------- 6 files changed, 21 insertions(+), 68 deletions(-) delete mode 100644 src/Shell/Command/GitDriverException.php delete mode 100644 src/Shell/Command/PatchDriverException.php diff --git a/config/services.xml b/config/services.xml index d9ce683..6c668fb 100644 --- a/config/services.xml +++ b/config/services.xml @@ -30,8 +30,6 @@ - - diff --git a/src/Shell/Command/DriverException.php b/src/Shell/Command/DriverException.php index b92a283..a2f7dd0 100644 --- a/src/Shell/Command/DriverException.php +++ b/src/Shell/Command/DriverException.php @@ -8,10 +8,23 @@ namespace Magento\CloudPatches\Shell\Command; use Magento\CloudPatches\Patch\PatchCommandException; +use Symfony\Component\Process\Exception\ProcessFailedException; +use Throwable; /** * Patch command driver exception */ class DriverException extends PatchCommandException { + /** + * @param Throwable $previous + */ + public function __construct(Throwable $previous) + { + $message = $previous->getMessage(); + if ($previous instanceof ProcessFailedException) { + $message = $previous->getProcess()->getErrorOutput() ?: ($previous->getProcess()->getOutput() ?: $message); + } + parent::__construct($message, $previous->getCode(), $previous); + } } diff --git a/src/Shell/Command/GitDriver.php b/src/Shell/Command/GitDriver.php index 16b6a56..a03eb61 100644 --- a/src/Shell/Command/GitDriver.php +++ b/src/Shell/Command/GitDriver.php @@ -38,7 +38,7 @@ public function apply(string $patch) $this->processFactory->create(['git', 'apply'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new GitDriverException($exception); + throw new DriverException($exception); } } @@ -51,7 +51,7 @@ public function revert(string $patch) $this->processFactory->create(['git', 'apply', '--reverse'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new GitDriverException($exception); + throw new DriverException($exception); } } @@ -64,7 +64,7 @@ public function applyCheck(string $patch) $this->processFactory->create(['git', 'apply', '--check'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new GitDriverException($exception); + throw new DriverException($exception); } } @@ -77,7 +77,7 @@ public function revertCheck(string $patch) $this->processFactory->create(['git', 'apply', '--reverse', '--check'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new GitDriverException($exception); + throw new DriverException($exception); } } diff --git a/src/Shell/Command/GitDriverException.php b/src/Shell/Command/GitDriverException.php deleted file mode 100644 index 490af43..0000000 --- a/src/Shell/Command/GitDriverException.php +++ /dev/null @@ -1,29 +0,0 @@ -getMessage(); - if ($previous instanceof ProcessFailedException) { - $message = $previous->getProcess()->getErrorOutput() ?: ($previous->getProcess()->getOutput() ?: $message); - } - parent::__construct($message, $previous->getCode(), $previous); - } -} diff --git a/src/Shell/Command/PatchDriver.php b/src/Shell/Command/PatchDriver.php index 11b5388..b22c4cc 100644 --- a/src/Shell/Command/PatchDriver.php +++ b/src/Shell/Command/PatchDriver.php @@ -39,7 +39,7 @@ public function apply(string $patch) $this->processFactory->create(['patch', '--silent', '-p1', '--no-backup-if-mismatch'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new PatchDriverException($exception); + throw new DriverException($exception); } } @@ -53,7 +53,7 @@ public function revert(string $patch) $this->processFactory->create(['patch', '--silent', '-p1', '--no-backup-if-mismatch', '--reverse'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new PatchDriverException($exception); + throw new DriverException($exception); } } @@ -66,7 +66,7 @@ public function applyCheck(string $patch) $this->processFactory->create(['patch', '--silent', '-p1', '--dry-run'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new PatchDriverException($exception); + throw new DriverException($exception); } } @@ -79,7 +79,7 @@ public function revertCheck(string $patch) $this->processFactory->create(['patch', '--silent', '-p1', '--reverse', '--dry-run'], $patch) ->mustRun(); } catch (ProcessFailedException $exception) { - throw new PatchDriverException($exception); + throw new DriverException($exception); } } diff --git a/src/Shell/Command/PatchDriverException.php b/src/Shell/Command/PatchDriverException.php deleted file mode 100644 index 39b09f8..0000000 --- a/src/Shell/Command/PatchDriverException.php +++ /dev/null @@ -1,29 +0,0 @@ -getMessage(); - if ($previous instanceof ProcessFailedException) { - $message = $previous->getProcess()->getErrorOutput() ?: ($previous->getProcess()->getOutput() ?: $message); - } - parent::__construct($message, $previous->getCode(), $previous); - } -}