From f8ca6abdc31d86aededbc23ce7b54819cd565e98 Mon Sep 17 00:00:00 2001
From: Ted Bowman <41201-tedbow@users.noreply.drupalcode.org>
Date: Thu, 6 Apr 2023 16:29:38 +0000
Subject: [PATCH] Issue #3337667 by omkar.podey, phenaproxima, tedbow, Wim
Leers: Allow JsonProcessOutputCallback and other Composer runner callbacks to
gracefully handle deprecated command options
---
package_manager/package_manager.services.yml | 6 +
package_manager/src/ComposerInspector.php | 74 +++++-----
.../src/JsonProcessOutputCallback.php | 73 ---------
package_manager/src/ProcessOutputCallback.php | 113 ++++++++++++++
.../src/Kernel/ComposerInspectorTest.php | 8 +-
.../Kernel/PackageManagerKernelTestBase.php | 29 ++++
.../tests/src/Kernel/ProjectInfoTest.php | 11 +-
.../Kernel/SupportedReleaseValidatorTest.php | 3 +
.../Unit/JsonProcessOutputCallbackTest.php | 68 ---------
.../src/Unit/ProcessOutputCallbackTest.php | 138 ++++++++++++++++++
10 files changed, 330 insertions(+), 193 deletions(-)
delete mode 100644 package_manager/src/JsonProcessOutputCallback.php
create mode 100644 package_manager/src/ProcessOutputCallback.php
delete mode 100644 package_manager/tests/src/Unit/JsonProcessOutputCallbackTest.php
create mode 100644 package_manager/tests/src/Unit/ProcessOutputCallbackTest.php
diff --git a/package_manager/package_manager.services.yml b/package_manager/package_manager.services.yml
index dc1a3c984d..791172e209 100644
--- a/package_manager/package_manager.services.yml
+++ b/package_manager/package_manager.services.yml
@@ -22,6 +22,10 @@ services:
alias: 'Drupal\package_manager\ProcessFactory'
PhpTuf\ComposerStager\Domain\Service\FileSyncer\FileSyncerInterface:
factory: ['@Drupal\package_manager\FileSyncerFactory', 'create']
+ logger.channel.package_manager:
+ parent: logger.channel_base
+ arguments:
+ - 'package_manager'
# Services provided to Drupal by Package Manager.
package_manager.beginner:
@@ -51,6 +55,8 @@ services:
- { name: event_subscriber }
package_manager.composer_inspector:
class: Drupal\package_manager\ComposerInspector
+ calls:
+ - [setLogger, ['@logger.channel.package_manager']]
Drupal\package_manager\ComposerInspector: '@package_manager.composer_inspector'
# Validators.
diff --git a/package_manager/src/ComposerInspector.php b/package_manager/src/ComposerInspector.php
index 3a4a6b6b31..602e871645 100644
--- a/package_manager/src/ComposerInspector.php
+++ b/package_manager/src/ComposerInspector.php
@@ -10,9 +10,12 @@ use Drupal\package_manager\Exception\ComposerNotReadyException;
use PhpTuf\ComposerStager\Domain\Exception\PreconditionException;
use PhpTuf\ComposerStager\Domain\Exception\RuntimeException;
use PhpTuf\ComposerStager\Domain\Service\Precondition\ComposerIsAvailableInterface;
-use PhpTuf\ComposerStager\Domain\Service\ProcessOutputCallback\ProcessOutputCallbackInterface;
use PhpTuf\ComposerStager\Domain\Service\ProcessRunner\ComposerRunnerInterface;
use PhpTuf\ComposerStager\Infrastructure\Factory\Path\PathFactoryInterface;
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerAwareTrait;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
/**
* Defines a class to get information from Composer.
@@ -23,16 +26,19 @@ use PhpTuf\ComposerStager\Infrastructure\Factory\Path\PathFactoryInterface;
* - read project & package configuration: getConfig() (`composer config`)
* - read root package info: getRootPackageInfo() (`composer show --self`)
*/
-class ComposerInspector {
+class ComposerInspector implements LoggerAwareInterface {
+ use LoggerAwareTrait {
+ setLogger as traitSetLogger;
+ }
use StringTranslationTrait;
/**
- * The JSON process output callback.
+ * The process output callback.
*
- * @var \Drupal\package_manager\JsonProcessOutputCallback
+ * @var \Drupal\package_manager\ProcessOutputCallback
*/
- private JsonProcessOutputCallback $jsonCallback;
+ private ProcessOutputCallback $processCallback;
/**
* Statically cached installed package lists, keyed by directory.
@@ -69,7 +75,16 @@ class ComposerInspector {
* The path factory service from Composer Stager.
*/
public function __construct(private ComposerRunnerInterface $runner, private ComposerIsAvailableInterface $composerIsAvailable, private PathFactoryInterface $pathFactory) {
- $this->jsonCallback = new JsonProcessOutputCallback();
+ $this->processCallback = new ProcessOutputCallback();
+ $this->setLogger(new NullLogger());
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function setLogger(LoggerInterface $logger): void {
+ $this->traitSetLogger($logger);
+ $this->processCallback->setLogger($logger);
}
/**
@@ -237,32 +252,8 @@ class ComposerInspector {
$this->validateProject($context);
$command[] = "--working-dir={$context}";
}
-
- // For whatever reason, PHPCS thinks that $output is not used, even though
- // it very clearly *is*. So, shut PHPCS up for the duration of this method.
- // phpcs:disable DrupalPractice.CodeAnalysis.VariableAnalysis.UnusedVariable
- $callback = new class () implements ProcessOutputCallbackInterface {
-
- /**
- * The command output.
- *
- * @var string
- */
- public string $output = '';
-
- /**
- * {@inheritdoc}
- */
- public function __invoke(string $type, string $buffer): void {
- if ($type === ProcessOutputCallbackInterface::OUT) {
- $this->output .= trim($buffer);
- }
- }
-
- };
- // phpcs:enable
try {
- $this->runner->run($command, $callback);
+ $this->runner->run($command, $this->processCallback->reset());
}
catch (RuntimeException $e) {
// Assume any error from `composer config` is about an undefined key-value
@@ -281,7 +272,8 @@ class ComposerInspector {
throw $e;
}
}
- return $callback->output;
+ $output = $this->processCallback->getOutput();
+ return isset($output) ? trim($output) : $output;
}
/**
@@ -291,11 +283,11 @@ class ComposerInspector {
* The Composer version.
*
* @throws \UnexpectedValueException
- * Thrown if the expect data format is not found.
+ * Thrown if the Composer version cannot be determined.
*/
private function getVersion(): string {
- $this->runner->run(['--format=json'], $this->jsonCallback);
- $data = $this->jsonCallback->getOutputData();
+ $this->runner->run(['--format=json'], $this->processCallback->reset());
+ $data = $this->processCallback->parseJsonOutput();
if (isset($data['application']['name'])
&& isset($data['application']['version'])
&& $data['application']['name'] === 'Composer'
@@ -401,8 +393,8 @@ class ComposerInspector {
public function getRootPackageInfo(string $working_dir): array {
$this->validate($working_dir);
- $this->runner->run(['show', '--self', '--format=json', "--working-dir={$working_dir}"], $this->jsonCallback);
- return $this->jsonCallback->getOutputData();
+ $this->runner->run(['show', '--self', '--format=json', "--working-dir={$working_dir}"], $this->processCallback->reset());
+ return $this->processCallback->parseJsonOutput();
}
/**
@@ -424,8 +416,8 @@ class ComposerInspector {
// about the installed packages. So, to work around this maddening quirk, we
// call `composer show` once without the --path option, and once with it,
// then merge the results together.
- $this->runner->run($options, $this->jsonCallback);
- $output = $this->jsonCallback->getOutputData();
+ $this->runner->run($options, $this->processCallback->reset());
+ $output = $this->processCallback->parseJsonOutput();
// $output['installed'] will not be set if no packages are installed.
if (isset($output['installed'])) {
foreach ($output['installed'] as $installed_package) {
@@ -433,8 +425,8 @@ class ComposerInspector {
}
$options[] = '--path';
- $this->runner->run($options, $this->jsonCallback);
- $output = $this->jsonCallback->getOutputData();
+ $this->runner->run($options, $this->processCallback->reset());
+ $output = $this->processCallback->parseJsonOutput();
foreach ($output['installed'] as $installed_package) {
$data[$installed_package['name']]['path'] = $installed_package['path'];
}
diff --git a/package_manager/src/JsonProcessOutputCallback.php b/package_manager/src/JsonProcessOutputCallback.php
deleted file mode 100644
index ed90ddae41..0000000000
--- a/package_manager/src/JsonProcessOutputCallback.php
+++ /dev/null
@@ -1,73 +0,0 @@
-<?php
-
-declare(strict_types = 1);
-
-namespace Drupal\package_manager;
-
-use Drupal\Component\Serialization\Json;
-use PhpTuf\ComposerStager\Domain\Service\ProcessOutputCallback\ProcessOutputCallbackInterface;
-
-/**
- * A process callback for handling output in the JSON format.
- *
- * @internal
- * This is an internal part of Package Manager and may be changed or removed
- * at any time without warning. External code should not interact with this
- * class.
- */
-final class JsonProcessOutputCallback implements ProcessOutputCallbackInterface {
-
- /**
- * The output buffer.
- *
- * @var string
- */
- private string $outBuffer = '';
-
- /**
- * The error buffer.
- *
- * @var string
- */
- private string $errorBuffer = '';
-
- /**
- * {@inheritdoc}
- */
- public function __invoke(string $type, string $buffer): void {
- // @todo Support self::ERR output in
- // https://www.drupal.org/project/automatic_updates/issues/3337504.
- if ($type === self::OUT) {
- $this->outBuffer .= $buffer;
- return;
- }
- elseif ($type === self::ERR) {
- $this->errorBuffer .= $buffer;
- return;
- }
- throw new \InvalidArgumentException("Unsupported output type: '$type'");
- }
-
- /**
- * Gets the output data.
- *
- * @return mixed|null
- * The output data or NULL if there was an exception.
- *
- * @throws \Exception
- * Thrown if error buffer was not empty.
- */
- public function getOutputData() {
- $error = $this->errorBuffer;
- $out = $this->outBuffer;
- $this->errorBuffer = '';
- $this->outBuffer = '';
- if ($error !== '') {
- // @todo Handle deprecations messages in the error output in
- // https://drupal.org/i/3337667.
- throw new \Exception($error);
- }
- return Json::decode($out);
- }
-
-}
diff --git a/package_manager/src/ProcessOutputCallback.php b/package_manager/src/ProcessOutputCallback.php
new file mode 100644
index 0000000000..f5896c94b4
--- /dev/null
+++ b/package_manager/src/ProcessOutputCallback.php
@@ -0,0 +1,113 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\package_manager;
+
+use PhpTuf\ComposerStager\Domain\Service\ProcessOutputCallback\ProcessOutputCallbackInterface;
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerAwareTrait;
+use Psr\Log\NullLogger;
+
+/**
+ * A process callback for capturing output.
+ *
+ * @see \Symfony\Component\Process\Process
+ *
+ * @internal
+ * This is an internal part of Package Manager and may be changed or removed
+ * at any time without warning. External code should not interact with this
+ * class.
+ */
+final class ProcessOutputCallback implements ProcessOutputCallbackInterface, LoggerAwareInterface {
+
+ use LoggerAwareTrait;
+
+ /**
+ * The output buffer.
+ *
+ * @var string
+ */
+ private string $outBuffer = '';
+
+ /**
+ * The error buffer.
+ *
+ * @var string
+ */
+ private string $errorBuffer = '';
+
+ /**
+ * Constructs a ProcessOutputCallback object.
+ */
+ public function __construct() {
+ $this->setLogger(new NullLogger());
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function __invoke(string $type, string $buffer): void {
+ if ($type === self::OUT) {
+ $this->outBuffer .= $buffer;
+ }
+ elseif ($type === self::ERR) {
+ $this->errorBuffer .= $buffer;
+ }
+ else {
+ throw new \InvalidArgumentException("Unsupported output type: '$type'");
+ }
+ }
+
+ /**
+ * Gets the output.
+ *
+ * If there is anything in the error buffer, it will be logged as a warning.
+ *
+ * @return string|null
+ * The output or NULL if there is none.
+ */
+ public function getOutput(): ?string {
+ $error_output = $this->getErrorOutput();
+ if ($error_output) {
+ $this->logger->warning($error_output);
+ }
+ return trim($this->outBuffer) !== '' ? $this->outBuffer : NULL;
+ }
+
+ /**
+ * Gets the parsed JSON output.
+ *
+ * @return mixed
+ * The decoded JSON output or NULL if there isn't any.
+ */
+ public function parseJsonOutput(): mixed {
+ $output = $this->getOutput();
+ if ($output !== NULL) {
+ return json_decode($output, TRUE, 512, JSON_THROW_ON_ERROR);
+ }
+ return NULL;
+ }
+
+ /**
+ * Gets the error output.
+ *
+ * @return string|null
+ * The error output or NULL if there isn't any.
+ */
+ public function getErrorOutput(): ?string {
+ return trim($this->errorBuffer) !== '' ? $this->errorBuffer : NULL;
+ }
+
+ /**
+ * Resets buffers.
+ *
+ * @return self
+ */
+ public function reset(): self {
+ $this->errorBuffer = '';
+ $this->outBuffer = '';
+ return $this;
+ }
+
+}
diff --git a/package_manager/tests/src/Kernel/ComposerInspectorTest.php b/package_manager/tests/src/Kernel/ComposerInspectorTest.php
index 217b569231..a645a34b31 100644
--- a/package_manager/tests/src/Kernel/ComposerInspectorTest.php
+++ b/package_manager/tests/src/Kernel/ComposerInspectorTest.php
@@ -10,8 +10,8 @@ use Drupal\fixture_manipulator\ActiveFixtureManipulator;
use Drupal\package_manager\ComposerInspector;
use Drupal\package_manager\Exception\ComposerNotReadyException;
use Drupal\package_manager\InstalledPackage;
+use Drupal\package_manager\ProcessOutputCallback;
use Drupal\package_manager\InstalledPackagesList;
-use Drupal\package_manager\JsonProcessOutputCallback;
use Drupal\Tests\package_manager\Traits\InstalledPackagesListTrait;
use Drupal\package_manager\PathLocator;
use PhpTuf\ComposerStager\Domain\Exception\PreconditionException;
@@ -248,9 +248,9 @@ class ComposerInspectorTest extends PackageManagerKernelTestBase {
],
]);
- /** @var \Drupal\package_manager\JsonProcessOutputCallback $callback */
+ /** @var \Drupal\package_manager\ProcessOutputCallback $callback */
[, $callback] = $arguments_passed_to_runner;
- $callback(JsonProcessOutputCallback::OUT, $command_output);
+ $callback($callback::OUT, $command_output);
};
// We expect the runner to be called with two arguments: an array whose
@@ -259,7 +259,7 @@ class ComposerInspectorTest extends PackageManagerKernelTestBase {
// once, even though we call validate() twice in this test.
$runner->run(
Argument::withEntry(0, '--format=json'),
- Argument::type(JsonProcessOutputCallback::class)
+ Argument::type(ProcessOutputCallback::class)
)->will($pass_version_to_output_callback)->shouldBeCalledOnce();
// The runner should be called with `validate` as the first argument, but
// it won't affect the outcome of this test.
diff --git a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
index 1999a918d5..bacf59f8b8 100644
--- a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
+++ b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
@@ -4,8 +4,10 @@ declare(strict_types = 1);
namespace Drupal\Tests\package_manager\Kernel;
+use ColinODell\PsrTestLogger\TestLogger;
use Drupal\Component\FileSystem\FileSystem as DrupalFileSystem;
use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\Core\Logger\RfcLogLevel;
use Drupal\Core\Site\Settings;
use Drupal\fixture_manipulator\StageFixtureManipulator;
use Drupal\KernelTests\KernelTestBase;
@@ -93,6 +95,15 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
*/
private readonly Filesystem $fileSystem;
+ /**
+ * A logger that will fail the test if Package Manager logs any errors.
+ *
+ * @var \ColinODell\PsrTestLogger\TestLogger
+ *
+ * @see ::tearDown()
+ */
+ protected readonly TestLogger $failureLogger;
+
/**
* {@inheritdoc}
*/
@@ -111,6 +122,13 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
// Make the update system think that all of System's post-update functions
// have run.
$this->registerPostUpdateFunctions();
+
+ // Ensure we can fail the test if any warnings, or worse, are logged by
+ // Package Manager.
+ // @see ::tearDown()
+ $this->failureLogger = new TestLogger();
+ $this->container->get('logger.channel.package_manager')
+ ->addLogger($this->failureLogger);
}
/**
@@ -134,6 +152,10 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
$container->getDefinition('package_manager.validator.disk_space')
->addTag('persist');
+ // Ensure that our failure logger will survive container rebuilds.
+ $container->getDefinition('logger.channel.package_manager')
+ ->addTag('persist');
+
foreach ($this->disableValidators as $service_id) {
if ($container->hasDefinition($service_id)) {
$container->getDefinition($service_id)->clearTag('event_subscriber');
@@ -388,6 +410,13 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
}
StageFixtureManipulator::handleTearDown();
+
+ // Ensure no warnings (or worse) were logged by Package Manager.
+ $this->assertFalse($this->failureLogger->hasRecords(RfcLogLevel::EMERGENCY), 'Package Manager logged emergencies.');
+ $this->assertFalse($this->failureLogger->hasRecords(RfcLogLevel::ALERT), 'Package Manager logged alerts.');
+ $this->assertFalse($this->failureLogger->hasRecords(RfcLogLevel::CRITICAL), 'Package Manager logged critical errors.');
+ $this->assertFalse($this->failureLogger->hasRecords(RfcLogLevel::ERROR), 'Package Manager logged errors.');
+ $this->assertFalse($this->failureLogger->hasRecords(RfcLogLevel::WARNING), 'Package Manager logged warnings.');
}
/**
diff --git a/package_manager/tests/src/Kernel/ProjectInfoTest.php b/package_manager/tests/src/Kernel/ProjectInfoTest.php
index adbd876e3b..e29be5c38e 100644
--- a/package_manager/tests/src/Kernel/ProjectInfoTest.php
+++ b/package_manager/tests/src/Kernel/ProjectInfoTest.php
@@ -6,7 +6,6 @@ namespace Drupal\Tests\package_manager\Kernel;
use Drupal\Core\Logger\RfcLogLevel;
use Drupal\package_manager\ProjectInfo;
-use ColinODell\PsrTestLogger\TestLogger;
/**
* @coversDefaultClass \Drupal\package_manager\ProjectInfo
@@ -130,10 +129,6 @@ class ProjectInfoTest extends PackageManagerKernelTestBase {
* Tests a project that is not in the codebase.
*/
public function testNewProject(): void {
- $logger = new TestLogger();
- $this->container->get('logger.factory')
- ->get('package_manager')
- ->addLogger($logger);
$fixtures_directory = __DIR__ . '/../../fixtures/release-history/';
$metadata_fixtures['drupal'] = $fixtures_directory . 'drupal.9.8.2.xml';
$metadata_fixtures['aaa_package_manager_test'] = $fixtures_directory . 'aaa_package_manager_test.7.0.1.xml';
@@ -176,8 +171,10 @@ class ProjectInfoTest extends PackageManagerKernelTestBase {
// the last checked time.
$this->assertSame(123, $state->get('update.last_check'));
- $this->assertTrue($logger->hasRecordThatContains('Invalid project format: Array', (string) RfcLogLevel::ERROR));
- $this->assertTrue($logger->hasRecordThatContains('[name] => AAA 8.x-5.x', (string) RfcLogLevel::ERROR));
+ $this->assertTrue($this->failureLogger->hasRecordThatContains('Invalid project format: Array', (string) RfcLogLevel::ERROR));
+ $this->assertTrue($this->failureLogger->hasRecordThatContains('[name] => AAA 8.x-5.x', (string) RfcLogLevel::ERROR));
+ // Prevent the logged errors from causing failures during tear-down.
+ $this->failureLogger->reset();
}
/**
diff --git a/package_manager/tests/src/Kernel/SupportedReleaseValidatorTest.php b/package_manager/tests/src/Kernel/SupportedReleaseValidatorTest.php
index 87f15c57de..2c945982b2 100644
--- a/package_manager/tests/src/Kernel/SupportedReleaseValidatorTest.php
+++ b/package_manager/tests/src/Kernel/SupportedReleaseValidatorTest.php
@@ -211,6 +211,9 @@ class SupportedReleaseValidatorTest extends PackageManagerKernelTestBase {
// @see \Drupal\package_manager\Validator\SupportedReleaseValidator::checkStagedReleases()
$stage_manipulator->setVersion('drupal/dependency', '9.8.1');
$this->assertResults($expected_results, PreApplyEvent::class);
+ // Ensure that any errors arising from invalid project info (which we expect
+ // in this test) will not fail the test during tear-down.
+ $this->failureLogger->reset();
}
}
diff --git a/package_manager/tests/src/Unit/JsonProcessOutputCallbackTest.php b/package_manager/tests/src/Unit/JsonProcessOutputCallbackTest.php
deleted file mode 100644
index e87159e91b..0000000000
--- a/package_manager/tests/src/Unit/JsonProcessOutputCallbackTest.php
+++ /dev/null
@@ -1,68 +0,0 @@
-<?php
-
-declare(strict_types = 1);
-
-namespace Drupal\Tests\package_manager\Unit;
-
-use Drupal\package_manager\JsonProcessOutputCallback;
-use Drupal\Tests\UnitTestCase;
-use PhpTuf\ComposerStager\Domain\Service\ProcessOutputCallback\ProcessOutputCallbackInterface;
-
-/**
- * @coversDefaultClass \Drupal\package_manager\JsonProcessOutputCallback
- *
- * @group package_manager
- */
-class JsonProcessOutputCallbackTest extends UnitTestCase {
-
- /**
- * @covers ::__invoke
- * @covers ::getOutputData
- */
- public function testGetOutputData(): void {
- // Create a data array that has 1 '*' character to allow easily splitting
- // up the JSON encoded string over multiple lines.
- $data = [
- 'value' => 'I have value!*',
- 'another value' => 'I have another value!',
- 'one' => 1,
- ];
- $json_string = json_encode($data, JSON_THROW_ON_ERROR);
- $lines = explode('*', $json_string);
- $lines[0] .= '*';
- // Confirm that 2 string concatenated together will recreate the original
- // data.
- $this->assertSame($data, json_decode($lines[0] . $lines[1], TRUE));
- $callback = new JsonProcessOutputCallback();
- $callback(ProcessOutputCallbackInterface::OUT, $lines[0]);
- $callback(ProcessOutputCallbackInterface::OUT, $lines[1]);
- $this->assertSame($data, $callback->getOutputData());
-
- $callback = new JsonProcessOutputCallback();
- $callback(ProcessOutputCallbackInterface::OUT, '1');
- $this->assertSame(1, $callback->getOutputData());
- }
-
- /**
- * @covers ::getOutputData
- */
- public function testNoInvokeCall(): void {
- $callback = new JsonProcessOutputCallback();
- $this->assertNull($callback->getOutputData());
- }
-
- /**
- * @covers ::getOutputData
- */
- public function testError(): void {
- $callback = new JsonProcessOutputCallback();
- $callback(ProcessOutputCallbackInterface::OUT, '1');
- $callback(ProcessOutputCallbackInterface::ERR, 'Oh no, what happened!!!!!');
- $callback(ProcessOutputCallbackInterface::OUT, '2');
- $callback(ProcessOutputCallbackInterface::ERR, 'Really what happened????');
- $this->expectException(\Exception::class);
- $this->expectExceptionMessage('Oh no, what happened!!!!!Really what happened????');
- $callback->getOutputData();
- }
-
-}
diff --git a/package_manager/tests/src/Unit/ProcessOutputCallbackTest.php b/package_manager/tests/src/Unit/ProcessOutputCallbackTest.php
new file mode 100644
index 0000000000..a1443385f8
--- /dev/null
+++ b/package_manager/tests/src/Unit/ProcessOutputCallbackTest.php
@@ -0,0 +1,138 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\Tests\package_manager\Unit;
+
+use ColinODell\PsrTestLogger\TestLogger;
+use Drupal\package_manager\ProcessOutputCallback;
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * @covers \Drupal\package_manager\ProcessOutputCallback
+ * @group package_manager
+ */
+class ProcessOutputCallbackTest extends UnitTestCase {
+
+ /**
+ * Tests what happens when the output buffer has invalid JSON.
+ */
+ public function testInvalidJson(): void {
+ $callback = new ProcessOutputCallback();
+ $callback($callback::OUT, '{A string of invalid JSON! 😈');
+
+ $this->expectException(\JsonException::class);
+ $this->expectExceptionMessage('Syntax error');
+ $callback->parseJsonOutput();
+ }
+
+ /**
+ * Tests that an invalid buffer type throws an exception.
+ */
+ public function testInvalidBufferType(): void {
+ $callback = new ProcessOutputCallback();
+
+ $this->expectException(\InvalidArgumentException::class);
+ $this->expectExceptionMessage("Unsupported output type: 'telegram'");
+ $callback('telegram', 'Into the void...');
+ }
+
+ /**
+ * Tests what happens when there is error output only.
+ */
+ public function testErrorOutputOnly(): void {
+ $callback = new ProcessOutputCallback();
+ $logger = new TestLogger();
+ $callback->setLogger($logger);
+
+ $error_text = 'What happened?';
+ $callback($callback::ERR, $error_text);
+
+ $this->assertSame($error_text, $callback->getErrorOutput());
+ // The error should not yet be logged.
+ $this->assertEmpty($logger->records);
+
+ // There should be no output data, but calling getOutput() should log the
+ // error.
+ $this->assertNull($callback->getOutput());
+ $this->assertNull($callback->parseJsonOutput());
+ $this->assertTrue($logger->hasWarning($error_text));
+
+ // Resetting the callback should clear the error buffer but the log should
+ // still have the error from before.
+ $callback->reset();
+ $this->assertTrue($logger->hasWarning($error_text));
+ }
+
+ /**
+ * Tests the full lifecycle of a ProcessOutputCallback object.
+ */
+ public function testCallback(): void {
+ $callback = new ProcessOutputCallback();
+ $logger = new TestLogger();
+ $callback->setLogger($logger);
+
+ // The buffers should initially be empty, and nothing should be logged.
+ $this->assertNull($callback->getOutput());
+ $this->assertNull($callback->getErrorOutput());
+ $this->assertNull($callback->parseJsonOutput());
+ $this->assertEmpty($logger->records);
+
+ // Send valid JSON data to the callback, one line at a time.
+ $data = [
+ 'value' => 'I have value!',
+ 'another value' => 'I have another value!',
+ 'one' => 1,
+ ];
+ $json = json_encode($data, JSON_PRETTY_PRINT);
+ // Ensure the JSON is a multi-line string.
+ $this->assertGreaterThan(1, substr_count($json, "\n"));
+ foreach (explode("\n", $json) as $line) {
+ $callback($callback::OUT, "$line\n");
+ }
+ $this->assertSame("$json\n", $callback->getOutput());
+ // Ensure that parseJsonOutput() can parse the data without errors.
+ $this->assertSame($data, $callback->parseJsonOutput());
+ $this->assertNull($callback->getErrorOutput());
+ $this->assertEmpty($logger->records);
+
+ // If we send error output, it should be logged, but we should still be able
+ // to get the data we already sent.
+ $callback($callback::ERR, 'Oh no, what happened?');
+ $callback($callback::ERR, 'Really what happened?!');
+ $this->assertSame($data, $callback->parseJsonOutput());
+ $this->assertSame('Oh no, what happened?Really what happened?!', $callback->getErrorOutput());
+ $this->assertTrue($logger->hasWarning('Oh no, what happened?Really what happened?!'));
+
+ // Send more output and error data to the callback; they should be appended
+ // to the data we previously sent.
+ $callback($callback::OUT, '{}');
+ $callback($callback::ERR, 'new Error 1!');
+ $callback($callback::ERR, 'new Error 2!');
+ // The output buffer will no longer be valid JSON, so don't try to parse it.
+ $this->assertSame("$json\n{}", $callback->getOutput());
+ $expected_error = 'Oh no, what happened?Really what happened?!new Error 1!new Error 2!';
+ $this->assertSame($expected_error, $callback->getErrorOutput());
+ $this->assertTrue($logger->hasWarning($expected_error));
+ // The previously logged error output should still be there.
+ $this->assertTrue($logger->hasWarning('Oh no, what happened?Really what happened?!'));
+
+ // Clear all stored output and errors.
+ $callback->reset();
+ $this->assertNull($callback->getOutput());
+ $this->assertNull($callback->getErrorOutput());
+ $this->assertNull($callback->parseJsonOutput());
+
+ // Send more output and error data.
+ $callback($callback::OUT, 'Bonjour!');
+ $callback($callback::ERR, 'You continue to annoy me.');
+ // We should now only see the stuff we just sent...
+ $this->assertSame('Bonjour!', $callback->getOutput());
+ $this->assertSame('You continue to annoy me.', $callback->getErrorOutput());
+ $this->assertTrue($logger->hasWarning('You continue to annoy me.'));
+ // ...but the previously logged errors should still be there.
+ $this->assertTrue($logger->hasWarning('Oh no, what happened?Really what happened?!new Error 1!new Error 2!'));
+ $this->assertTrue($logger->hasWarning('Oh no, what happened?Really what happened?!'));
+ }
+
+}
--
GitLab