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
allegro · Aug 27, 2024 · ec7fe55 · ec7fe55
1 parent b84cd97
commit ec7fe55
Show file tree

Hide file tree

Showing 12 changed files with 58 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,22 +21,25 @@ 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}")
             logger.error("remote message: ${message}")
             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
@@ -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
@@ -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<String> 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<String> 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
@@ -6,20 +6,22 @@
 
 public class ScmPushResult {
 
-    private final boolean success;
+    private final ScmPushResultOutcome outcome;
 
     private final Optional<RemoteRefUpdate.Status> failureCause;
 
     private final Optional<String> remoteMessage;
 
-    public ScmPushResult(boolean success, Optional<RemoteRefUpdate.Status> failureCause, Optional<String> remoteMessage) {
-        this.success = success;
+    public ScmPushResult(ScmPushResultOutcome outcome,
+                         Optional<RemoteRefUpdate.Status> failureCause,
+                         Optional<String> remoteMessage) {
+        this.outcome = outcome;
         this.failureCause = failureCause;
         this.remoteMessage = remoteMessage;
     }
 
-    public boolean isSuccess() {
-        return success;
+    public ScmPushResultOutcome getOutcome() {
+        return outcome;
     }
 
     public Optional<RemoteRefUpdate.Status> 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
@@ -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
@@ -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
@@ -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<PushResult> pushResults) {
                 && !ref.getStatus().equals(RemoteRefUpdate.Status.UP_TO_DATE)
         ).findFirst();
 
-        boolean isSuccess = !failedRefUpdate.isPresent();
+        boolean isSuccess = failedRefUpdate.isEmpty();
         Optional<RemoteRefUpdate.Status> 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
@@ -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()