From ec7fe55610ceb0e5310ac546b56f011bc69a9b74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Ga=C5=82ek?= Date: Tue, 27 Aug 2024 15:30:33 +0200 Subject: [PATCH] ScmPushResultOutcome introduced (#800) * ScmPushResultOutcome introduced instead of boolean value, fixes bug when GITHUB_OUTPUT should be empty when release task was skipped * log messages updated --- README.md | 2 +- .../axion/release/RemoteRejectionTest.groovy | 3 ++- .../release/SimpleIntegrationTest.groovy | 22 +++++++++++++++++-- .../axion/release/PushReleaseTask.groovy | 3 ++- .../build/axion/release/ReleaseTask.groovy | 18 +++++++++------ .../infrastructure/NoOpRepository.groovy | 2 +- .../build/axion/release/domain/Releaser.java | 7 +++--- .../release/domain/scm/ScmPushResult.java | 12 +++++----- .../domain/scm/ScmPushResultOutcome.java | 5 +++++ .../axion/release/domain/scm/ScmService.java | 2 +- .../infrastructure/git/GitRepository.java | 7 +++--- .../axion/release/domain/ReleaserTest.groovy | 2 +- 12 files changed, 58 insertions(+), 27 deletions(-) create mode 100644 src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmPushResultOutcome.java diff --git a/README.md b/README.md index 7b128903..d02da269 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ JDK11+ & Gradle 7+ required. ```kotlin plugins { - id("pl.allegro.tech.build.axion-release") version "1.18.6" + id("pl.allegro.tech.build.axion-release") version "1.18.7" } version = scmVersion.version diff --git a/src/integration/groovy/pl/allegro/tech/build/axion/release/RemoteRejectionTest.groovy b/src/integration/groovy/pl/allegro/tech/build/axion/release/RemoteRejectionTest.groovy index d0e74778..f427bd9c 100644 --- a/src/integration/groovy/pl/allegro/tech/build/axion/release/RemoteRejectionTest.groovy +++ b/src/integration/groovy/pl/allegro/tech/build/axion/release/RemoteRejectionTest.groovy @@ -11,6 +11,7 @@ import pl.allegro.tech.build.axion.release.domain.scm.ScmIdentity import pl.allegro.tech.build.axion.release.domain.scm.ScmPropertiesBuilder import pl.allegro.tech.build.axion.release.domain.scm.ScmPushOptions import pl.allegro.tech.build.axion.release.domain.scm.ScmPushResult +import pl.allegro.tech.build.axion.release.domain.scm.ScmPushResultOutcome import pl.allegro.tech.build.axion.release.infrastructure.git.GitRepository import pl.allegro.tech.build.axion.release.infrastructure.git.SshConnector import spock.lang.Shared @@ -59,7 +60,7 @@ class RemoteRejectionTest extends Specification { ScmPushResult result = repository.push(keyIdentity, new ScmPushOptions('origin', false), true) then: - !result.success + result.outcome == ScmPushResultOutcome.FAILED result.remoteMessage.get().contains("I reject this push!") } } diff --git a/src/integration/groovy/pl/allegro/tech/build/axion/release/SimpleIntegrationTest.groovy b/src/integration/groovy/pl/allegro/tech/build/axion/release/SimpleIntegrationTest.groovy index 0a3c89c0..24ce14aa 100644 --- a/src/integration/groovy/pl/allegro/tech/build/axion/release/SimpleIntegrationTest.groovy +++ b/src/integration/groovy/pl/allegro/tech/build/axion/release/SimpleIntegrationTest.groovy @@ -155,7 +155,7 @@ class SimpleIntegrationTest extends BaseIntegrationTest { then: releaseResult.task(':release').outcome == TaskOutcome.SUCCESS - releaseResult.output.contains('Release step skipped since \'releaseOnlyOnDefaultBranches\' option is set, and \'master\' was not in \'releaseBranchNames\' list [develop,release]') + releaseResult.output.contains('Release step skipped since \'releaseOnlyOnReleaseBranches\' option is set, and \'master\' was not in \'releaseBranchNames\' list [develop,release]') } def "should skip release when releaseOnlyOnReleaseBranches is set by gradle task property and current branch is not on releaseBranchNames list"() { @@ -167,7 +167,7 @@ class SimpleIntegrationTest extends BaseIntegrationTest { then: releaseResult.task(':release').outcome == TaskOutcome.SUCCESS - releaseResult.output.contains('Release step skipped since \'releaseOnlyOnDefaultBranches\' option is set, and \'master\' was not in \'releaseBranchNames\' list [develop,release]') + releaseResult.output.contains('Release step skipped since \'releaseOnlyOnReleaseBranches\' option is set, and \'master\' was not in \'releaseBranchNames\' list [develop,release]') } def "should not skip release when releaseOnlyOnReleaseBranches is true when on master branch (default releaseBranches list)"() { @@ -185,4 +185,22 @@ class SimpleIntegrationTest extends BaseIntegrationTest { releaseResult.task(':release').outcome == TaskOutcome.SUCCESS releaseResult.output.contains('Creating tag: ' + fullPrefix() + '1.0.0') } + + def "should skip release and no GITHUB_OUTPUT should be written"() { + given: + def outputFile = File.createTempFile("github-outputs", ".tmp") + environmentVariablesRule.set("GITHUB_ACTIONS", "true") + environmentVariablesRule.set("GITHUB_OUTPUT", outputFile.getAbsolutePath()) + + buildFile('') + + when: + runGradle('release', '-Prelease.releaseOnlyOnReleaseBranches', '-Prelease.releaseBranchNames=develop,release', '-Prelease.version=1.0.0', '-Prelease.localOnly', '-Prelease.disableChecks') + + then: + outputFile.getText().isEmpty() + + cleanup: + environmentVariablesRule.clear("GITHUB_ACTIONS", "GITHUB_OUTPUT") + } } diff --git a/src/main/groovy/pl/allegro/tech/build/axion/release/PushReleaseTask.groovy b/src/main/groovy/pl/allegro/tech/build/axion/release/PushReleaseTask.groovy index 3c3ce0d2..4098e9e4 100644 --- a/src/main/groovy/pl/allegro/tech/build/axion/release/PushReleaseTask.groovy +++ b/src/main/groovy/pl/allegro/tech/build/axion/release/PushReleaseTask.groovy @@ -4,6 +4,7 @@ package pl.allegro.tech.build.axion.release import org.gradle.api.tasks.TaskAction import pl.allegro.tech.build.axion.release.domain.Releaser import pl.allegro.tech.build.axion.release.domain.scm.ScmPushResult +import pl.allegro.tech.build.axion.release.domain.scm.ScmPushResultOutcome import pl.allegro.tech.build.axion.release.infrastructure.di.VersionResolutionContext abstract class PushReleaseTask extends BaseAxionTask { @@ -14,7 +15,7 @@ abstract class PushReleaseTask extends BaseAxionTask { Releaser releaser = context.releaser() ScmPushResult result = releaser.pushRelease() - if (!result.success) { + if (result.outcome === ScmPushResultOutcome.FAILED) { def message = result.remoteMessage.orElse("Unknown error during push") logger.error("remote message: ${message}") throw new ReleaseFailedException(message) diff --git a/src/main/groovy/pl/allegro/tech/build/axion/release/ReleaseTask.groovy b/src/main/groovy/pl/allegro/tech/build/axion/release/ReleaseTask.groovy index 8ff57718..702e27fc 100644 --- a/src/main/groovy/pl/allegro/tech/build/axion/release/ReleaseTask.groovy +++ b/src/main/groovy/pl/allegro/tech/build/axion/release/ReleaseTask.groovy @@ -3,6 +3,7 @@ package pl.allegro.tech.build.axion.release import org.gradle.api.tasks.TaskAction import pl.allegro.tech.build.axion.release.domain.Releaser import pl.allegro.tech.build.axion.release.domain.scm.ScmPushResult +import pl.allegro.tech.build.axion.release.domain.scm.ScmPushResultOutcome import pl.allegro.tech.build.axion.release.infrastructure.di.VersionResolutionContext import java.nio.file.Files @@ -20,9 +21,10 @@ abstract class ReleaseTask extends BaseAxionTask { context.repository().currentPosition().getBranch(), context.scmService().getReleaseBranchNames() ) + ScmPushResult result = releaser.releaseAndPush(context.rules(), releaseBranchesConfiguration) - if (!result.success) { + if (result.outcome === ScmPushResultOutcome.FAILED) { def status = result.failureStatus def message = result.remoteMessage.orElse("Unknown error during push") logger.error("remote status: ${status}") @@ -30,12 +32,14 @@ abstract class ReleaseTask extends BaseAxionTask { throw new ReleaseFailedException("Status: ${status}\nMessage: ${message}") } - if (System.getenv().containsKey('GITHUB_ACTIONS')) { - Files.write( - Paths.get(System.getenv('GITHUB_OUTPUT')), - "released-version=${versionConfig.uncached.decoratedVersion}\n".getBytes(), - StandardOpenOption.APPEND - ) + if (result.outcome === ScmPushResultOutcome.SUCCESS) { + if (System.getenv().containsKey('GITHUB_ACTIONS')) { + Files.write( + Paths.get(System.getenv('GITHUB_OUTPUT')), + "released-version=${versionConfig.uncached.decoratedVersion}\n".getBytes(), + StandardOpenOption.APPEND + ) + } } } } diff --git a/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/NoOpRepository.groovy b/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/NoOpRepository.groovy index fe833d59..5946f2ce 100644 --- a/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/NoOpRepository.groovy +++ b/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/NoOpRepository.groovy @@ -35,7 +35,7 @@ class NoOpRepository implements ScmRepository { @Override ScmPushResult push(ScmIdentity identity, ScmPushOptions pushOptions) { log("pushing to remote: ${pushOptions.remote}") - return new ScmPushResult(true, Optional.empty(), Optional.empty()) + return new ScmPushResult(ScmPushResultOutcome.SUCCESS, Optional.empty(), Optional.empty()) } @Override diff --git a/src/main/java/pl/allegro/tech/build/axion/release/domain/Releaser.java b/src/main/java/pl/allegro/tech/build/axion/release/domain/Releaser.java index 9bbfb5f3..c70dd4d3 100644 --- a/src/main/java/pl/allegro/tech/build/axion/release/domain/Releaser.java +++ b/src/main/java/pl/allegro/tech/build/axion/release/domain/Releaser.java @@ -7,6 +7,7 @@ import pl.allegro.tech.build.axion.release.domain.hooks.ReleaseHooksRunner; import pl.allegro.tech.build.axion.release.domain.properties.Properties; import pl.allegro.tech.build.axion.release.domain.scm.ScmPushResult; +import pl.allegro.tech.build.axion.release.domain.scm.ScmPushResultOutcome; import pl.allegro.tech.build.axion.release.domain.scm.ScmService; import java.util.Optional; @@ -27,7 +28,7 @@ public Releaser(VersionService versionService, ScmService repository, ReleaseHoo public Optional release(Properties properties, ReleaseBranchesConfiguration releaseBranchesConfiguration) { if (releaseBranchesConfiguration.shouldRelease()) { String message = String.format( - "Release step skipped since 'releaseOnlyOnDefaultBranches' option is set, and '%s' was not in 'releaseBranchNames' list [%s]", + "Release step skipped since 'releaseOnlyOnReleaseBranches' option is set, and '%s' was not in 'releaseBranchNames' list [%s]", releaseBranchesConfiguration.getCurrentBranch(), String.join(",", releaseBranchesConfiguration.getReleaseBranchNames()) ); @@ -61,12 +62,12 @@ public ScmPushResult releaseAndPush(Properties rules, ReleaseBranchesConfigurati Optional releasedTagName = release(rules, releaseBranchesConfiguration); if (releasedTagName.isEmpty()) { - return new ScmPushResult(true, Optional.empty(), Optional.empty()); + return new ScmPushResult(ScmPushResultOutcome.SKIPPED, Optional.empty(), Optional.empty()); } ScmPushResult result = pushRelease(); - if (!result.isSuccess()) { + if (result.getOutcome().equals(ScmPushResultOutcome.FAILED)) { releasedTagName.ifPresent(this::rollbackRelease); } diff --git a/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmPushResult.java b/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmPushResult.java index a6d490d9..0839b2ee 100644 --- a/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmPushResult.java +++ b/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmPushResult.java @@ -6,20 +6,22 @@ public class ScmPushResult { - private final boolean success; + private final ScmPushResultOutcome outcome; private final Optional failureCause; private final Optional remoteMessage; - public ScmPushResult(boolean success, Optional failureCause, Optional remoteMessage) { - this.success = success; + public ScmPushResult(ScmPushResultOutcome outcome, + Optional failureCause, + Optional remoteMessage) { + this.outcome = outcome; this.failureCause = failureCause; this.remoteMessage = remoteMessage; } - public boolean isSuccess() { - return success; + public ScmPushResultOutcome getOutcome() { + return outcome; } public Optional getFailureStatus() { diff --git a/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmPushResultOutcome.java b/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmPushResultOutcome.java new file mode 100644 index 00000000..380cbb64 --- /dev/null +++ b/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmPushResultOutcome.java @@ -0,0 +1,5 @@ +package pl.allegro.tech.build.axion.release.domain.scm; + +public enum ScmPushResultOutcome { + SUCCESS, SKIPPED, FAILED +} diff --git a/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmService.java b/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmService.java index 6db50e02..9fd7b9ab 100644 --- a/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmService.java +++ b/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmService.java @@ -38,7 +38,7 @@ public void dropTag(String tagName) { public ScmPushResult push() { if (localOnlyResolver.localOnly(this.remoteAttached())) { logger.quiet("Changes made to local repository only"); - return new ScmPushResult(true, Optional.of(RemoteRefUpdate.Status.NOT_ATTEMPTED), Optional.empty()); + return new ScmPushResult(ScmPushResultOutcome.SUCCESS, Optional.of(RemoteRefUpdate.Status.NOT_ATTEMPTED), Optional.empty()); } try { diff --git a/src/main/java/pl/allegro/tech/build/axion/release/infrastructure/git/GitRepository.java b/src/main/java/pl/allegro/tech/build/axion/release/infrastructure/git/GitRepository.java index f8d7c7fd..068c0733 100644 --- a/src/main/java/pl/allegro/tech/build/axion/release/infrastructure/git/GitRepository.java +++ b/src/main/java/pl/allegro/tech/build/axion/release/infrastructure/git/GitRepository.java @@ -154,10 +154,9 @@ public ScmPushResult push(ScmIdentity identity, ScmPushOptions pushOptions, bool if (!pushOptions.isPushTagsOnly()) { PushCommand command = pushCommand(identity, pushOptions.getRemote(), all); ScmPushResult result = verifyPushResults(callPush(command)); - if (!result.isSuccess()) { + if (result.getOutcome().equals(ScmPushResultOutcome.FAILED)) { return result; } - } // and again for tags @@ -182,12 +181,12 @@ private ScmPushResult verifyPushResults(Iterable pushResults) { && !ref.getStatus().equals(RemoteRefUpdate.Status.UP_TO_DATE) ).findFirst(); - boolean isSuccess = !failedRefUpdate.isPresent(); + boolean isSuccess = failedRefUpdate.isEmpty(); Optional failureCause = isSuccess ? Optional.empty() : Optional.of(failedRefUpdate.get().getStatus()); return new ScmPushResult( - isSuccess, + isSuccess ? ScmPushResultOutcome.SUCCESS : ScmPushResultOutcome.FAILED, failureCause, Optional.ofNullable(pushResult.getMessages()) ); diff --git a/src/test/groovy/pl/allegro/tech/build/axion/release/domain/ReleaserTest.groovy b/src/test/groovy/pl/allegro/tech/build/axion/release/domain/ReleaserTest.groovy index a14572e1..a16853b9 100644 --- a/src/test/groovy/pl/allegro/tech/build/axion/release/domain/ReleaserTest.groovy +++ b/src/test/groovy/pl/allegro/tech/build/axion/release/domain/ReleaserTest.groovy @@ -115,7 +115,7 @@ class ReleaserTest extends RepositoryBasedTest { repository.lastLogMessages(1) == ['release version: 3.2.0'] } - def "should do release when isReleaseOnlyOnDefaultBranches option is set and current branch is in releaseBranchNames list"() { + def "should do release when releaseOnlyOnReleaseBranches option is set and current branch is in releaseBranchNames list"() { given: repository.tag(fullPrefix() + '1.0.0') Properties rules = properties()