Merge pull request #47 from weierophinney/hotfix/fix-changelog-operations

Ocramius · web-flow · commit a048debdbced · 2020-08-12T22:36:26.000+02:00
Idempotent operations for reading, checkout branch prior to writing
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,7 +22,7 @@ All notable changes to this project will be documented in this file, in reverse
 
 ### Fixed
 
-- Nothing.
+- [#47](https://github.com/laminas/automatic-releases/pull/47) fixes `CHANGELOG.md` update operations to avoid preventable failures during the release process.
 
 ## 1.2.1 - 2020-08-12
 
diff --git a/bin/console.php b/bin/console.php
@@ -13,6 +13,7 @@
 use Laminas\AutomaticReleases\Application\Command\ReleaseCommand;
 use Laminas\AutomaticReleases\Application\Command\SwitchDefaultBranchToNextMinor;
 use Laminas\AutomaticReleases\Changelog\BumpAndCommitChangelogVersionViaKeepAChangelog;
+use Laminas\AutomaticReleases\Changelog\ChangelogExistsViaConsole;
 use Laminas\AutomaticReleases\Changelog\CommitReleaseChangelogViaKeepAChangelog;
 use Laminas\AutomaticReleases\Environment\EnvironmentVariables;
 use Laminas\AutomaticReleases\Git\CheckoutBranchViaConsole;
@@ -69,16 +70,24 @@ static function (int $errorCode, string $message = '', string $file = '', int $l
         $httpClient,
         $githubToken
     ));
+    $changelogExists      = new ChangelogExistsViaConsole();
     $checkoutBranch       = new CheckoutBranchViaConsole();
     $commit               = new CommitFileViaConsole();
     $push                 = new PushViaConsole();
-    $commitChangelog      = new CommitReleaseChangelogViaKeepAChangelog(new SystemClock(), $commit, $push, $logger);
+    $commitChangelog      = new CommitReleaseChangelogViaKeepAChangelog(
+        new SystemClock(),
+        $changelogExists,
+        $checkoutBranch,
+        $commit,
+        $push,
+        $logger
+    );
     $createCommitText     = new CreateReleaseTextThroughChangelog(JwageGenerateChangelog::create(
         $makeRequests,
         $httpClient
     ));
     $createReleaseText    = new ConcatenateMultipleReleaseTexts([
-        new CreateReleaseTextViaKeepAChangelog(),
+        new CreateReleaseTextViaKeepAChangelog($changelogExists),
         $createCommitText,
     ]);
     $createRelease        = new CreateReleaseThroughApiCall(
@@ -87,6 +96,7 @@ static function (int $errorCode, string $message = '', string $file = '', int $l
         $githubToken
     );
     $bumpChangelogVersion = new BumpAndCommitChangelogVersionViaKeepAChangelog(
+        $changelogExists,
         $checkoutBranch,
         $commit,
         $push,
diff --git a/src/Changelog/BumpAndCommitChangelogVersionViaKeepAChangelog.php b/src/Changelog/BumpAndCommitChangelogVersionViaKeepAChangelog.php
@@ -13,7 +13,6 @@
 use Psr\Log\LoggerInterface;
 use Webmozart\Assert\Assert;
 
-use function file_exists;
 use function sprintf;
 
 class BumpAndCommitChangelogVersionViaKeepAChangelog implements BumpAndCommitChangelogVersion
@@ -26,21 +25,24 @@ class BumpAndCommitChangelogVersionViaKeepAChangelog implements BumpAndCommitCha
         Updates the %s file to add a changelog entry for a new %s version.
         COMMIT;
 
+    private ChangelogExists $changelogExists;
     private CheckoutBranch $checkoutBranch;
     private CommitFile $commitFile;
     private Push $push;
     private LoggerInterface $logger;
 
     public function __construct(
+        ChangelogExists $changelogExists,
         CheckoutBranch $checkoutBranch,
         CommitFile $commitFile,
         Push $push,
         LoggerInterface $logger
     ) {
-        $this->checkoutBranch = $checkoutBranch;
-        $this->commitFile     = $commitFile;
-        $this->push           = $push;
-        $this->logger         = $logger;
+        $this->changelogExists = $changelogExists;
+        $this->checkoutBranch  = $checkoutBranch;
+        $this->commitFile      = $commitFile;
+        $this->push            = $push;
+        $this->logger          = $logger;
     }
 
     public function __invoke(
@@ -49,16 +51,16 @@ public function __invoke(
         SemVerVersion $version,
         BranchName $sourceBranch
     ): void {
-        ($this->checkoutBranch)($repositoryDirectory, $sourceBranch);
-
-        $changelogFile = sprintf('%s/%s', $repositoryDirectory, self::CHANGELOG_FILE);
-        if (! file_exists($changelogFile)) {
+        if (! ($this->changelogExists)($sourceBranch, $repositoryDirectory)) {
             // No changelog
             $this->logger->info('BumpAndCommitChangelog: No CHANGELOG.md file detected');
 
             return;
         }
 
+        ($this->checkoutBranch)($repositoryDirectory, $sourceBranch);
+
+        $changelogFile = sprintf('%s/%s', $repositoryDirectory, self::CHANGELOG_FILE);
         $versionString = $version->fullReleaseName();
         $bumper        = new ChangelogBump($changelogFile);
         $newVersion    = $bumper->$bumpType($versionString);
diff --git a/src/Changelog/ChangelogExists.php b/src/Changelog/ChangelogExists.php
@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Laminas\AutomaticReleases\Changelog;
+
+use Laminas\AutomaticReleases\Git\Value\BranchName;
+
+interface ChangelogExists
+{
+    /**
+     * @param non-empty-string $repositoryDirectory
+     */
+    public function __invoke(
+        BranchName $sourceBranch,
+        string $repositoryDirectory
+    ): bool;
+}
diff --git a/src/Changelog/ChangelogExistsViaConsole.php b/src/Changelog/ChangelogExistsViaConsole.php
@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Laminas\AutomaticReleases\Changelog;
+
+use Laminas\AutomaticReleases\Git\Value\BranchName;
+use Symfony\Component\Process\Process;
+
+class ChangelogExistsViaConsole implements ChangelogExists
+{
+    /**
+     * @param non-empty-string $repositoryDirectory
+     */
+    public function __invoke(
+        BranchName $sourceBranch,
+        string $repositoryDirectory
+    ): bool {
+        $process = new Process(['git', 'show', $sourceBranch->name() . ':CHANGELOG.md'], $repositoryDirectory);
+        $process->run();
+
+        return $process->isSuccessful();
+    }
+}
diff --git a/src/Changelog/CommitReleaseChangelogViaKeepAChangelog.php b/src/Changelog/CommitReleaseChangelogViaKeepAChangelog.php
@@ -4,6 +4,7 @@
 
 namespace Laminas\AutomaticReleases\Changelog;
 
+use Laminas\AutomaticReleases\Git\CheckoutBranch;
 use Laminas\AutomaticReleases\Git\CommitFile;
 use Laminas\AutomaticReleases\Git\Push;
 use Laminas\AutomaticReleases\Git\Value\BranchName;
@@ -17,7 +18,6 @@
 use Symfony\Component\Console\Output\NullOutput;
 use Webmozart\Assert\Assert;
 
-use function file_exists;
 use function sprintf;
 
 final class CommitReleaseChangelogViaKeepAChangelog implements CommitReleaseChangelog
@@ -31,20 +31,26 @@ final class CommitReleaseChangelogViaKeepAChangelog implements CommitReleaseChan
         COMMIT;
 
     private Clock $clock;
+    private ChangelogExists $changelogExists;
+    private CheckoutBranch $checkoutBranch;
     private CommitFile $commitFile;
     private Push $push;
     private LoggerInterface $logger;
 
     public function __construct(
         Clock $clock,
+        ChangelogExists $changelogExists,
+        CheckoutBranch $checkoutBranch,
         CommitFile $commitFile,
         Push $push,
         LoggerInterface $logger
     ) {
-        $this->clock      = $clock;
-        $this->commitFile = $commitFile;
-        $this->push       = $push;
-        $this->logger     = $logger;
+        $this->clock           = $clock;
+        $this->changelogExists = $changelogExists;
+        $this->checkoutBranch  = $checkoutBranch;
+        $this->commitFile      = $commitFile;
+        $this->push            = $push;
+        $this->logger          = $logger;
     }
 
     /**
@@ -55,16 +61,18 @@ public function __invoke(
         SemVerVersion $version,
         BranchName $sourceBranch
     ): void {
-        $changelogFile = sprintf('%s/%s', $repositoryDirectory, self::CHANGELOG_FILE);
-        if (! file_exists($changelogFile)) {
+        if (! ($this->changelogExists)($sourceBranch, $repositoryDirectory)) {
             // No changelog
             $this->logger->info('No CHANGELOG.md file detected');
 
             return;
         }
 
+        $changelogFile = sprintf('%s/%s', $repositoryDirectory, self::CHANGELOG_FILE);
         $versionString = $version->fullReleaseName();
 
+        ($this->checkoutBranch)($repositoryDirectory, $sourceBranch);
+
         if (! $this->updateChangelog($changelogFile, $versionString)) {
             // Failure to update; nothing to commit
             return;
diff --git a/src/Github/CreateReleaseTextViaKeepAChangelog.php b/src/Github/CreateReleaseTextViaKeepAChangelog.php
@@ -5,19 +5,25 @@
 namespace Laminas\AutomaticReleases\Github;
 
 use InvalidArgumentException;
+use Laminas\AutomaticReleases\Changelog\ChangelogExists;
 use Laminas\AutomaticReleases\Git\Value\BranchName;
 use Laminas\AutomaticReleases\Git\Value\SemVerVersion;
 use Laminas\AutomaticReleases\Github\Api\GraphQL\Query\GetMilestoneChangelog\Response\Milestone;
 use Laminas\AutomaticReleases\Github\Value\RepositoryName;
 use Phly\KeepAChangelog\Common\ChangelogParser;
 use Phly\KeepAChangelog\Exception\ExceptionInterface;
+use Symfony\Component\Process\Process;
 use Webmozart\Assert\Assert;
 
-use function file_exists;
-use function file_get_contents;
-
 class CreateReleaseTextViaKeepAChangelog implements CreateReleaseText
 {
+    private ChangelogExists $changelogExists;
+
+    public function __construct(ChangelogExists $changelogExists)
+    {
+        $this->changelogExists = $changelogExists;
+    }
+
     public function __invoke(
         Milestone $milestone,
         RepositoryName $repositoryName,
@@ -27,7 +33,7 @@ public function __invoke(
     ): string {
         $changelog = (new ChangelogParser())
             ->findChangelogForVersion(
-                file_get_contents($repositoryDirectory . '/CHANGELOG.md'),
+                $this->fetchChangelogContentsFromBranch($sourceBranch, $repositoryDirectory),
                 $semVerVersion->fullReleaseName()
             );
 
@@ -43,15 +49,14 @@ public function canCreateReleaseText(
         BranchName $sourceBranch,
         string $repositoryDirectory
     ): bool {
-        $changelogFile = $repositoryDirectory . '/CHANGELOG.md';
-        if (! file_exists($changelogFile)) {
+        if (! ($this->changelogExists)($sourceBranch, $repositoryDirectory)) {
             return false;
         }
 
         try {
             $changelog = (new ChangelogParser())
                 ->findChangelogForVersion(
-                    file_get_contents($changelogFile),
+                    $this->fetchChangelogContentsFromBranch($sourceBranch, $repositoryDirectory),
                     $semVerVersion->fullReleaseName()
                 );
 
@@ -62,4 +67,21 @@ public function canCreateReleaseText(
             return false;
         }
     }
+
+    /**
+     * @psalm-param non-empty-string $repositoryDirectory
+     * @psalm-return non-empty-string
+     */
+    private function fetchChangelogContentsFromBranch(
+        BranchName $sourceBranch,
+        string $repositoryDirectory
+    ): string {
+        $process = new Process(['git', 'show', $sourceBranch->name() . ':CHANGELOG.md'], $repositoryDirectory);
+        $process->mustRun();
+
+        $contents = $process->getOutput();
+        Assert::notEmpty($contents);
+
+        return $contents;
+    }
 }
diff --git a/test/unit/Changelog/BumpAndCommitChangelogVersionViaKeepAChangelogTest.php b/test/unit/Changelog/BumpAndCommitChangelogVersionViaKeepAChangelogTest.php
@@ -6,6 +6,8 @@
 
 use Laminas\AutomaticReleases\Changelog\BumpAndCommitChangelogVersion;
 use Laminas\AutomaticReleases\Changelog\BumpAndCommitChangelogVersionViaKeepAChangelog;
+use Laminas\AutomaticReleases\Changelog\ChangelogExists;
+use Laminas\AutomaticReleases\Changelog\ChangelogExistsViaConsole;
 use Laminas\AutomaticReleases\Git\CheckoutBranch;
 use Laminas\AutomaticReleases\Git\CommitFile;
 use Laminas\AutomaticReleases\Git\Push;
@@ -44,6 +46,7 @@ protected function setUp(): void
         $this->push                   = $this->createMock(Push::class);
         $this->logger                 = $this->createMock(LoggerInterface::class);
         $this->bumpAndCommitChangelog = new BumpAndCommitChangelogVersionViaKeepAChangelog(
+            new ChangelogExistsViaConsole(),
             $this->checkoutBranch,
             $this->commitFile,
             $this->push,
@@ -58,12 +61,8 @@ public function testReturnsEarlyWhenNoChangelogFilePresent(): void
         $version      = SemVerVersion::fromMilestoneName('1.0.1');
 
         $this->checkoutBranch
-            ->expects($this->once())
-            ->method('__invoke')
-            ->with(
-                $this->equalTo($repoDir),
-                $sourceBranch
-            );
+            ->expects($this->never())
+            ->method('__invoke');
 
         $this->logger
             ->expects($this->once())
@@ -118,6 +117,13 @@ public function testAddsNewReleaseVersionUsingBumpTypeToChangelogFileAndCommitsA
 
         Assert::stringNotEmpty($repoDir);
 
+        $changelogExists = $this->createMock(ChangelogExists::class);
+        $changelogExists
+            ->expects($this->once())
+            ->method('__invoke')
+            ->with($sourceBranch, $repoDir)
+            ->willReturn(true);
+
         $this->logger
             ->expects($this->once())
             ->method('info')
@@ -154,8 +160,16 @@ public function testAddsNewReleaseVersionUsingBumpTypeToChangelogFileAndCommitsA
             ->with(
             );
 
+        $bumpAndCommitChangelog = new BumpAndCommitChangelogVersionViaKeepAChangelog(
+            $changelogExists,
+            $this->checkoutBranch,
+            $this->commitFile,
+            $this->push,
+            $this->logger
+        );
+
         $this->assertNull(
-            ($this->bumpAndCommitChangelog)(
+            $bumpAndCommitChangelog(
                 $bumpType,
                 $repoDir,
                 $version,
@@ -191,6 +205,13 @@ private function createMockChangelog(): string
 
         file_put_contents($changelogFile, self::CHANGELOG_STUB);
 
+        (new Process(['git', 'init', '.'], $repo))->mustRun();
+        (new Process(['git', 'config', 'user.email', 'me@example.com'], $repo))->mustRun();
+        (new Process(['git', 'config', 'user.name', 'Just Me'], $repo))->mustRun();
+        (new Process(['git', 'add', '.'], $repo))->mustRun();
+        (new Process(['git', 'commit', '-m', 'Initial import'], $repo))->mustRun();
+        (new Process(['git', 'switch', '-c', '1.0.x'], $repo))->mustRun();
+
         return $changelogFile;
     }
 
diff --git a/test/unit/Changelog/ChangelogExistsViaConsoleTest.php b/test/unit/Changelog/ChangelogExistsViaConsoleTest.php
diff --git a/test/unit/Changelog/ReleaseChangelogViaKeepAChangelogTest.php b/test/unit/Changelog/ReleaseChangelogViaKeepAChangelogTest.php
diff --git a/test/unit/Github/CreateReleaseTextViaKeepAChangelogTest.php b/test/unit/Github/CreateReleaseTextViaKeepAChangelogTest.php
