Merge pull request #1450 from derrabus/improvement/test-logger-impl

composer.json

@@ -40,6 +40,7 @@
         "doctrine/orm": "^2.13 || ^3",
         "doctrine/persistence": "^2 || ^3",
         "doctrine/sql-formatter": "^1.0",
+        "fig/log-test": "^1",
         "phpstan/phpstan": "^1.10",
         "phpstan/phpstan-deprecation-rules": "^1.1",
         "phpstan/phpstan-phpunit": "^1.3",

phpcs.xml.dist

@@ -38,7 +38,7 @@
     </rule>
 
     <rule ref="Squiz.Strings.DoubleQuoteUsage.ContainsVar">
-        <exclude-pattern>tests/TestLogger.php</exclude-pattern>
+        <exclude-pattern>tests/LogUtil.php</exclude-pattern>
         <exclude-pattern>src/Tools/Console/ConsoleLogger.php</exclude-pattern>
     </rule>
 

tests/AbstractMigrationTest.php

@@ -12,9 +12,12 @@
 use Doctrine\Migrations\Query\Query;
 use Doctrine\Migrations\Tests\Stub\AbstractMigrationStub;
 use Doctrine\Migrations\Tests\Stub\AbstractMigrationWithoutDownStub;
+use Psr\Log\Test\TestLogger;
 
 class AbstractMigrationTest extends MigrationTestCase
 {
+    use LogUtil;
+
     private AbstractMigrationStub $migration;
 
     private TestLogger $logger;
@@ -73,7 +76,7 @@ public function testWarnIfAddDefaultMessage(): void
     public function testWarnIfDontOutputMessageIfFalse(): void
     {
         $this->migration->warnIf(false, 'trallala');
-        self::assertSame('', $this->getLogOutput($this->logger));
+        self::assertSame([], $this->logger->records);
     }
 
     public function testWriteInvokesOutputWriter(): void

tests/TestLogger.php → tests/LogUtil.php

@@ -4,51 +4,49 @@
 
 namespace Doctrine\Migrations\Tests;
 
-use DateTime;
 use DateTimeInterface;
-use Psr\Log\AbstractLogger;
+use Psr\Log\Test\TestLogger;
 use Stringable;
 
+use function array_map;
 use function gettype;
+use function implode;
 use function is_object;
 use function is_scalar;
 use function str_contains;
 use function strtr;
 
-class TestLogger extends AbstractLogger
+trait LogUtil
 {
-    /** @var string[] */
-    public array $logs = [];
+    private function getLogOutput(TestLogger $logger): string
+    {
+        return implode("\n", $this->getInterpolatedLogRecords($logger));
+    }
 
-    /**
-     * {@inheritDoc}
-     *
-     * @param string|Stringable $message
-     * @param mixed[]           $context
-     */
-    public function log($level, $message, array $context = []): void
+    /** @return list<string> */
+    private function getInterpolatedLogRecords(TestLogger $logger): array
     {
-        $this->logs[] = $this->interpolate($message, $context);
+        return array_map($this->interpolate(...), $logger->records);
     }
 
     /**
      * Interpolates context values into the message placeholders.
      *
-     * @param mixed[] $context
+     * @param array{level: mixed, message: string|Stringable, context: mixed[]} $record
      */
-    private function interpolate(string|Stringable $message, array $context): string
+    private function interpolate(array $record): string
     {
-        $message = (string) $message;
+        $message = (string) $record['message'];
         if (! str_contains($message, '{')) {
             return $message;
         }
 
         $replacements = [];
-        foreach ($context as $key => $val) {
+        foreach ($record['context'] as $key => $val) {
             if ($val === null || is_scalar($val) || $val instanceof Stringable) {
                 $replacements["{{$key}}"] = $val;
             } elseif ($val instanceof DateTimeInterface) {
-                $replacements["{{$key}}"] = $val->format(DateTime::RFC3339);
+                $replacements["{{$key}}"] = $val->format(DateTimeInterface::RFC3339);
             } elseif (is_object($val)) {
                 $replacements["{{$key}}"] = '[object ' . $val::class . ']';
             } else {

tests/Metadata/Storage/TableMetadataStorageTest.php

@@ -26,6 +26,7 @@
 use Doctrine\Migrations\Version\ExecutionResult;
 use Doctrine\Migrations\Version\Version;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\Test\TestLogger;
 
 use function sprintf;
 
@@ -42,7 +43,7 @@ class TableMetadataStorageTest extends TestCase
     /** @var AbstractSchemaManager<AbstractPlatform> */
     private AbstractSchemaManager $schemaManager;
 
-    private DebugLogger $debugLogger;
+    private TestLogger $testLogger;
 
     private function getSqliteConnection(Configuration|null $configuration = null): Connection
     {
@@ -54,8 +55,8 @@ private function getSqliteConnection(Configuration|null $configuration = null):
     public function setUp(): void
     {
         $this->connectionConfig = new Configuration();
-        $this->debugLogger      = new DebugLogger();
-        $this->connectionConfig->setMiddlewares([new Middleware($this->debugLogger)]);
+        $this->testLogger       = new TestLogger();
+        $this->connectionConfig->setMiddlewares([new Middleware($this->testLogger)]);
         $this->connection    = $this->getSqliteConnection($this->connectionConfig);
         $this->schemaManager = $this->connection->createSchemaManager();
 
@@ -67,13 +68,12 @@ public function testSchemaIntrospectionExecutedOnlyOnce(): void
     {
         $this->storage->ensureInitialized();
 
-        $oldQueryCount = $this->debugLogger->count;
+        $this->testLogger->reset();
         $this->storage->ensureInitialized();
-        self::assertSame(0, $this->debugLogger->count - $oldQueryCount);
+        self::assertCount(0, $this->testLogger->records);
 
-        $oldQueryCount = $this->debugLogger->count;
         $this->storage->getExecutedMigrations();
-        self::assertSame(1, $this->debugLogger->count - $oldQueryCount);
+        self::assertCount(1, $this->testLogger->records);
     }
 
     public function testDifferentTableNotUpdatedOnRead(): void

tests/MigrationTestCase.php

@@ -8,8 +8,6 @@
 use Doctrine\DBAL\DriverManager;
 use PHPUnit\Framework\TestCase;
 
-use function implode;
-
 abstract class MigrationTestCase extends TestCase
 {
     public function getSqliteConnection(): Connection
@@ -18,9 +16,4 @@ public function getSqliteConnection(): Connection
 
         return DriverManager::getConnection($params);
     }
-
-    public function getLogOutput(TestLogger $logger): string
-    {
-        return implode("\n", $logger->logs);
-    }
 }

tests/MigratorTest.php

@@ -23,6 +23,7 @@
 use Doctrine\Migrations\Version\Direction;
 use Doctrine\Migrations\Version\Version;
 use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\Test\TestLogger;
 use Symfony\Component\Console\Output\StreamOutput;
 use Symfony\Component\Stopwatch\Stopwatch;
 use Throwable;
@@ -79,8 +80,8 @@ public function testEmptyPlanShowsMessage(): void
         $planList = new MigrationPlanList([], Direction::UP);
         $migrator->migrate($planList, $this->migratorConfiguration);
 
-        self::assertCount(1, $this->logger->logs, 'should output the no migrations message');
-        self::assertStringContainsString('No migrations', $this->logger->logs[0]);
+        self::assertCount(1, $this->logger->records, 'should output the no migrations message');
+        self::assertStringContainsString('No migrations', $this->logger->records[0]['message']);
     }
 
     protected function createTestMigrator(): DbalMigrator

tests/Tools/Console/Command/MigrationVersionTest.php

@@ -13,12 +13,12 @@
 use Doctrine\Migrations\MigrationsRepository;
 use Doctrine\Migrations\Tests\Helper;
 use Doctrine\Migrations\Tests\MigrationTestCase;
-use Doctrine\Migrations\Tests\TestLogger;
 use Doctrine\Migrations\Tools\Console\Command\VersionCommand;
 use Doctrine\Migrations\Version\Direction;
 use Doctrine\Migrations\Version\ExecutionResult;
 use Doctrine\Migrations\Version\Version;
 use InvalidArgumentException;
+use Psr\Log\NullLogger;
 use Symfony\Component\Console\Helper\HelperSet;
 use Symfony\Component\Console\Helper\QuestionHelper;
 use Symfony\Component\Console\Tester\CommandTester;
@@ -40,13 +40,12 @@ protected function setUp(): void
         $configuration = new Configuration();
         $configuration->addMigrationsDirectory('DoctrineMigrations', sys_get_temp_dir());
 
-        $conn   = $this->getSqliteConnection();
-        $logger = new TestLogger();
+        $conn = $this->getSqliteConnection();
 
         $dependencyFactory = DependencyFactory::fromConnection(
             new ExistingConfiguration($configuration),
             new ExistingConnection($conn),
-            $logger,
+            new NullLogger(),
         );
 
         $this->migrationRepository = $dependencyFactory->getMigrationRepository();

tests/Version/ExecutorTest.php

@@ -14,7 +14,7 @@
 use Doctrine\Migrations\ParameterFormatter;
 use Doctrine\Migrations\Provider\SchemaDiffProvider;
 use Doctrine\Migrations\Query\Query;
-use Doctrine\Migrations\Tests\TestLogger;
+use Doctrine\Migrations\Tests\LogUtil;
 use Doctrine\Migrations\Tests\Version\Fixture\EmptyTestMigration;
 use Doctrine\Migrations\Tests\Version\Fixture\VersionExecutorTestMigration;
 use Doctrine\Migrations\Version\DbalExecutor;
@@ -25,13 +25,16 @@
 use Exception;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\Test\TestLogger;
 use Symfony\Component\Stopwatch\Stopwatch;
 use Symfony\Component\Stopwatch\StopwatchEvent;
 use Symfony\Component\Stopwatch\StopwatchPeriod;
 use Throwable;
 
 class ExecutorTest extends TestCase
 {
+    use LogUtil;
+
     /** @var Connection&MockObject */
     private Connection $connection;
 
@@ -90,7 +93,7 @@ public function testExecuteWithNoQueries(): void
             '++ migrating xx',
             'Migration xx was executed but did not result in any SQL statements.',
             'Migration xx migrated (took 100ms, used 100 memory)',
-        ], $this->logger->logs);
+        ], $this->getInterpolatedLogRecords($this->logger));
     }
 
     public function testExecuteUp(): void
@@ -137,7 +140,7 @@ public function testExecuteUp(): void
             3 => 'SELECT 2 ',
             4 => 'Query took 100ms',
             5 => 'Migration test migrated (took 100ms, used 100 memory)',
-        ], $this->logger->logs);
+        ], $this->getInterpolatedLogRecords($this->logger));
     }
 
     /** @test */
@@ -151,7 +154,7 @@ public function executeUpShouldAppendDescriptionWhenItIsNotEmpty(): void
 
         $this->versionExecutor->execute($plan, $migratorConfiguration);
 
-        self::assertSame('++ migrating test (testing)', $this->logger->logs[0]);
+        self::assertSame('++ migrating test (testing)', $this->getInterpolatedLogRecords($this->logger)[0]);
     }
 
     public function testExecuteDown(): void
@@ -198,7 +201,7 @@ public function testExecuteDown(): void
             3 => 'SELECT 4 ',
             4 => 'Query took 100ms',
             5 => 'Migration test reverted (took 100ms, used 100 memory)',
-        ], $this->logger->logs);
+        ], $this->getInterpolatedLogRecords($this->logger));
     }
 
     public function testExecuteDryRun(): void
@@ -261,7 +264,7 @@ public function testExecuteDryRun(): void
             'SELECT 1 ',
             'SELECT 2 ',
             'Migration test migrated (took 100ms, used 100 memory)',
-        ], $this->logger->logs);
+        ], $this->getInterpolatedLogRecords($this->logger));
     }
 
     /** @test */
@@ -498,7 +501,7 @@ public function executeDownShouldAppendDescriptionWhenItIsNotEmpty(): void
             $migratorConfiguration,
         );
 
-        self::assertSame('++ reverting test', $this->logger->logs[0]);
+        self::assertSame('++ reverting test', $this->getInterpolatedLogRecords($this->logger)[0]);
     }
 
     protected function setUp(): void