Skip to content

Idempotent operations for reading, checkout branch prior to writing #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 12 additions & 2 deletions bin/console.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -87,6 +96,7 @@ static function (int $errorCode, string $message = '', string $file = '', int $l
$githubToken
);
$bumpChangelogVersion = new BumpAndCommitChangelogVersionViaKeepAChangelog(
$changelogExists,
$checkoutBranch,
$commit,
$push,
Expand Down
20 changes: 11 additions & 9 deletions src/Changelog/BumpAndCommitChangelogVersionViaKeepAChangelog.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
use Psr\Log\LoggerInterface;
use Webmozart\Assert\Assert;

use function file_exists;
use function sprintf;

class BumpAndCommitChangelogVersionViaKeepAChangelog implements BumpAndCommitChangelogVersion
Expand All @@ -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(
Expand All @@ -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);
Expand Down
18 changes: 18 additions & 0 deletions src/Changelog/ChangelogExists.php
Original file line number Diff line number Diff line change
@@ -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;
}
24 changes: 24 additions & 0 deletions src/Changelog/ChangelogExistsViaConsole.php
Original file line number Diff line number Diff line change
@@ -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();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very specifically not "mustRun()" here, as failure is an expected status.


return $process->isSuccessful();
}
}
22 changes: 15 additions & 7 deletions src/Changelog/CommitReleaseChangelogViaKeepAChangelog.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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;
}

/**
Expand All @@ -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;
Expand Down
36 changes: 29 additions & 7 deletions src/Github/CreateReleaseTextViaKeepAChangelog.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that these filesystem access operations went away from here 👍


class CreateReleaseTextViaKeepAChangelog implements CreateReleaseText
{
private ChangelogExists $changelogExists;

public function __construct(ChangelogExists $changelogExists)
{
$this->changelogExists = $changelogExists;
}

public function __invoke(
Milestone $milestone,
RepositoryName $repositoryName,
Expand All @@ -27,7 +33,7 @@ public function __invoke(
): string {
$changelog = (new ChangelogParser())
->findChangelogForVersion(
file_get_contents($repositoryDirectory . '/CHANGELOG.md'),
$this->fetchChangelogContentsFromBranch($sourceBranch, $repositoryDirectory),
$semVerVersion->fullReleaseName()
);

Expand All @@ -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()
);

Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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())
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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', '[email protected]'], $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;
}

Expand Down
Loading