From 206f5865b3d914509f22f4ef2b5f655ad7a63da3 Mon Sep 17 00:00:00 2001 From: ThusithaDJ Date: Thu, 11 Feb 2021 03:35:38 +0530 Subject: [PATCH 1/5] Changed conclusion status based on ChecksStatus --- .../checks/steps/PublishChecksStep.java | 4 ++ .../checks/steps/PublishChecksStepTest.java | 62 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java b/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java index 22724010..e85f0d36 100644 --- a/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java +++ b/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java @@ -169,6 +169,10 @@ static class PublishChecksStepExecution extends SynchronousNonBlockingStepExecut PublishChecksStepExecution(final StepContext context, final PublishChecksStep step) { super(context); this.step = step; + if ((step.getConclusion() == ChecksConclusion.SUCCESS) + && (step.getStatus() == ChecksStatus.QUEUED || step.getStatus() == ChecksStatus.IN_PROGRESS)) { + this.step.setConclusion(ChecksConclusion.NONE); + } } @Override diff --git a/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java b/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java index c23381a1..f96f402c 100644 --- a/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java +++ b/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java @@ -42,6 +42,68 @@ void shouldPublishCheckWithDefaultValues() throws IOException, InterruptedExcept .build()); } + @Test + void shouldPublishCheckWithStatusInProgress() throws IOException, InterruptedException { + PublishChecksStep step = new PublishChecksStep(); + step.setName("Jenkins"); + step.setSummary("a check made by Jenkins"); + step.setTitle("Jenkins Build"); + step.setText("an in progress build"); + step.setStatus(ChecksStatus.IN_PROGRESS); + step.setDetailsURL("http://ci.jenkins.io"); + + StepContext context = mock(StepContext.class); + when(context.get(Run.class)).thenReturn(mock(Run.class)); + when(context.get(TaskListener.class)).thenReturn(TaskListener.NULL); + + StepExecution execution = step.start(context); + assertThat(execution).isInstanceOf(PublishChecksStep.PublishChecksStepExecution.class); + assertThat(((PublishChecksStep.PublishChecksStepExecution)execution).extractChecksDetails()) + .usingRecursiveComparison() + .isEqualTo(new ChecksDetails.ChecksDetailsBuilder() + .withName("Jenkins") + .withStatus(ChecksStatus.IN_PROGRESS) + .withConclusion(ChecksConclusion.NONE) + .withDetailsURL("http://ci.jenkins.io") + .withOutput(new ChecksOutput.ChecksOutputBuilder() + .withTitle("Jenkins Build") + .withSummary("a check made by Jenkins") + .withText("an in progress build") + .build()) + .build()); + } + + @Test + void shouldPublishCheckWithStatusQueue() throws IOException, InterruptedException { + PublishChecksStep step = new PublishChecksStep(); + step.setName("Jenkins"); + step.setSummary("a check made by Jenkins"); + step.setTitle("Jenkins Build"); + step.setText("a queued build"); + step.setStatus(ChecksStatus.QUEUED); + step.setDetailsURL("http://ci.jenkins.io"); + + StepContext context = mock(StepContext.class); + when(context.get(Run.class)).thenReturn(mock(Run.class)); + when(context.get(TaskListener.class)).thenReturn(TaskListener.NULL); + + StepExecution execution = step.start(context); + assertThat(execution).isInstanceOf(PublishChecksStep.PublishChecksStepExecution.class); + assertThat(((PublishChecksStep.PublishChecksStepExecution)execution).extractChecksDetails()) + .usingRecursiveComparison() + .isEqualTo(new ChecksDetails.ChecksDetailsBuilder() + .withName("Jenkins") + .withStatus(ChecksStatus.QUEUED) + .withConclusion(ChecksConclusion.NONE) + .withDetailsURL("http://ci.jenkins.io") + .withOutput(new ChecksOutput.ChecksOutputBuilder() + .withTitle("Jenkins Build") + .withSummary("a check made by Jenkins") + .withText("a queued build") + .build()) + .build()); + } + @Test void shouldPublishCheckWithSetValues() throws IOException, InterruptedException { PublishChecksStep step = new PublishChecksStep(); From 4b9b7486291045fcf715aa460a12338eb207ec57 Mon Sep 17 00:00:00 2001 From: ThusithaDJ Date: Sun, 14 Feb 2021 03:54:57 +0530 Subject: [PATCH 2/5] Refactoring the implementation --- .../checks/steps/PublishChecksStep.java | 7 +- .../checks/steps/PublishChecksStepTest.java | 71 +++++++++---------- 2 files changed, 37 insertions(+), 41 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java b/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java index e85f0d36..ca279e5f 100644 --- a/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java +++ b/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java @@ -68,6 +68,9 @@ public void setDetailsURL(final String detailsURL) { @DataBoundSetter public void setStatus(final ChecksStatus status) { this.status = status; + if (status == ChecksStatus.QUEUED || status == ChecksStatus.IN_PROGRESS) { + this.conclusion = ChecksConclusion.NONE; + } } @DataBoundSetter @@ -169,10 +172,6 @@ static class PublishChecksStepExecution extends SynchronousNonBlockingStepExecut PublishChecksStepExecution(final StepContext context, final PublishChecksStep step) { super(context); this.step = step; - if ((step.getConclusion() == ChecksConclusion.SUCCESS) - && (step.getStatus() == ChecksStatus.QUEUED || step.getStatus() == ChecksStatus.IN_PROGRESS)) { - this.step.setConclusion(ChecksConclusion.NONE); - } } @Override diff --git a/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java b/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java index f96f402c..63aa8c86 100644 --- a/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java +++ b/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java @@ -9,21 +9,28 @@ import org.apache.commons.lang3.StringUtils; import org.jenkinsci.plugins.workflow.steps.StepContext; import org.jenkinsci.plugins.workflow.steps.StepExecution; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.io.IOException; +import java.util.Objects; import static org.assertj.core.api.Assertions.assertThat; import static io.jenkins.plugins.checks.assertions.Assertions.assertThat; import static org.mockito.Mockito.*; class PublishChecksStepTest { - @Test - void shouldPublishCheckWithDefaultValues() throws IOException, InterruptedException { - StepContext context = mock(StepContext.class); + private StepContext context; + @BeforeEach + void setup() throws IOException, InterruptedException { + this.context = mock(StepContext.class); when(context.get(Run.class)).thenReturn(mock(Run.class)); when(context.get(TaskListener.class)).thenReturn(TaskListener.NULL); + } + + @Test + void shouldPublishCheckWithDefaultValues() throws IOException, InterruptedException { StepExecution execution = new PublishChecksStep().start(context); assertThat(execution).isInstanceOf(PublishChecksStep.PublishChecksStepExecution.class); @@ -44,17 +51,8 @@ void shouldPublishCheckWithDefaultValues() throws IOException, InterruptedExcept @Test void shouldPublishCheckWithStatusInProgress() throws IOException, InterruptedException { - PublishChecksStep step = new PublishChecksStep(); - step.setName("Jenkins"); - step.setSummary("a check made by Jenkins"); - step.setTitle("Jenkins Build"); - step.setText("an in progress build"); - step.setStatus(ChecksStatus.IN_PROGRESS); - step.setDetailsURL("http://ci.jenkins.io"); - - StepContext context = mock(StepContext.class); - when(context.get(Run.class)).thenReturn(mock(Run.class)); - when(context.get(TaskListener.class)).thenReturn(TaskListener.NULL); + PublishChecksStep step = getModifiedPublishChecksStepObject("an in progress build", + ChecksStatus.IN_PROGRESS,null); StepExecution execution = step.start(context); assertThat(execution).isInstanceOf(PublishChecksStep.PublishChecksStepExecution.class); @@ -75,17 +73,8 @@ void shouldPublishCheckWithStatusInProgress() throws IOException, InterruptedExc @Test void shouldPublishCheckWithStatusQueue() throws IOException, InterruptedException { - PublishChecksStep step = new PublishChecksStep(); - step.setName("Jenkins"); - step.setSummary("a check made by Jenkins"); - step.setTitle("Jenkins Build"); - step.setText("a queued build"); - step.setStatus(ChecksStatus.QUEUED); - step.setDetailsURL("http://ci.jenkins.io"); - - StepContext context = mock(StepContext.class); - when(context.get(Run.class)).thenReturn(mock(Run.class)); - when(context.get(TaskListener.class)).thenReturn(TaskListener.NULL); + PublishChecksStep step = getModifiedPublishChecksStepObject("a queued build", + ChecksStatus.QUEUED,null); StepExecution execution = step.start(context); assertThat(execution).isInstanceOf(PublishChecksStep.PublishChecksStepExecution.class); @@ -106,18 +95,8 @@ void shouldPublishCheckWithStatusQueue() throws IOException, InterruptedExceptio @Test void shouldPublishCheckWithSetValues() throws IOException, InterruptedException { - PublishChecksStep step = new PublishChecksStep(); - step.setName("Jenkins"); - step.setSummary("a check made by Jenkins"); - step.setTitle("Jenkins Build"); - step.setText("a failed build"); - step.setStatus(ChecksStatus.IN_PROGRESS); - step.setConclusion(ChecksConclusion.FAILURE); - step.setDetailsURL("http://ci.jenkins.io"); - - StepContext context = mock(StepContext.class); - when(context.get(Run.class)).thenReturn(mock(Run.class)); - when(context.get(TaskListener.class)).thenReturn(TaskListener.NULL); + PublishChecksStep step = getModifiedPublishChecksStepObject("a failed build", + ChecksStatus.IN_PROGRESS, ChecksConclusion.FAILURE); StepExecution execution = step.start(context); assertThat(execution).isInstanceOf(PublishChecksStep.PublishChecksStepExecution.class); @@ -143,4 +122,22 @@ void shouldDefinePublishChecksStepDescriptorCorrectly() { assertThat(descriptor.getDisplayName()).isEqualTo("Publish customized checks to SCM platforms"); assertThat(descriptor.getRequiredContext().toArray()).containsExactlyInAnyOrder(Run.class, TaskListener.class); } + + private PublishChecksStep getModifiedPublishChecksStepObject(String stepText, ChecksStatus status, + ChecksConclusion conclusion) { + PublishChecksStep step = new PublishChecksStep(); + step.setName("Jenkins"); + step.setSummary("a check made by Jenkins"); + step.setTitle("Jenkins Build"); + step.setText(stepText); + if (Objects.nonNull(status)) { + step.setStatus(status); + } + if (Objects.nonNull(conclusion)) { + step.setConclusion(conclusion); + } + step.setDetailsURL("http://ci.jenkins.io"); + + return step; + } } From 263e404db76ee95e8b42bdcd50c26697903461f6 Mon Sep 17 00:00:00 2001 From: ThusithaDJ Date: Sun, 14 Feb 2021 04:33:18 +0530 Subject: [PATCH 3/5] Fix PMD issues. --- .../io/jenkins/plugins/checks/steps/PublishChecksStep.java | 4 ++++ .../jenkins/plugins/checks/steps/PublishChecksStepTest.java | 5 ++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java b/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java index ca279e5f..c6b1e309 100644 --- a/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java +++ b/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java @@ -65,6 +65,10 @@ public void setDetailsURL(final String detailsURL) { this.detailsURL = detailsURL; } + /** + * Change the status of the Publish Check Step. + * @param status CheckStatus + */ @DataBoundSetter public void setStatus(final ChecksStatus status) { this.status = status; diff --git a/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java b/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java index 63aa8c86..879fb244 100644 --- a/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java +++ b/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java @@ -31,7 +31,6 @@ void setup() throws IOException, InterruptedException { @Test void shouldPublishCheckWithDefaultValues() throws IOException, InterruptedException { - StepExecution execution = new PublishChecksStep().start(context); assertThat(execution).isInstanceOf(PublishChecksStep.PublishChecksStepExecution.class); assertThat(((PublishChecksStep.PublishChecksStepExecution)execution).extractChecksDetails()) @@ -123,8 +122,8 @@ void shouldDefinePublishChecksStepDescriptorCorrectly() { assertThat(descriptor.getRequiredContext().toArray()).containsExactlyInAnyOrder(Run.class, TaskListener.class); } - private PublishChecksStep getModifiedPublishChecksStepObject(String stepText, ChecksStatus status, - ChecksConclusion conclusion) { + private PublishChecksStep getModifiedPublishChecksStepObject(final String stepText, final ChecksStatus status, + final ChecksConclusion conclusion) { PublishChecksStep step = new PublishChecksStep(); step.setName("Jenkins"); step.setSummary("a check made by Jenkins"); From 3f39628900310a0d8d236cc2c2ddcb1b74924d25 Mon Sep 17 00:00:00 2001 From: Thusitha Jayasundara Date: Thu, 18 Feb 2021 02:55:50 +0530 Subject: [PATCH 4/5] Update src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java Co-authored-by: Kezhi Xiong --- .../jenkins/plugins/checks/steps/PublishChecksStep.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java b/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java index c6b1e309..82d4f479 100644 --- a/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java +++ b/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java @@ -66,8 +66,12 @@ public void setDetailsURL(final String detailsURL) { } /** - * Change the status of the Publish Check Step. - * @param status CheckStatus + * Change the status of the check. + * When the {@code status} is {@link ChecksStatus#QUEUED} or {@link ChecksStatus#IN_PROGRESS}, + * the conclusion will be reset to {@link ChecksConclusion#NONE} + * + * @param status + * the status to be set */ @DataBoundSetter public void setStatus(final ChecksStatus status) { From b3c45229172924c4c82c6e1c73a82795b4125771 Mon Sep 17 00:00:00 2001 From: ThusithaDJ Date: Thu, 18 Feb 2021 03:27:57 +0530 Subject: [PATCH 5/5] Refactor unit tests --- .../checks/steps/PublishChecksStepTest.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java b/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java index 879fb244..78d5ad89 100644 --- a/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java +++ b/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepTest.java @@ -20,18 +20,17 @@ import static org.mockito.Mockito.*; class PublishChecksStepTest { - private StepContext context; - @BeforeEach - void setup() throws IOException, InterruptedException { - this.context = mock(StepContext.class); + StepContext getStepContext() throws IOException, InterruptedException { + StepContext context = mock(StepContext.class); when(context.get(Run.class)).thenReturn(mock(Run.class)); when(context.get(TaskListener.class)).thenReturn(TaskListener.NULL); + return context; } @Test void shouldPublishCheckWithDefaultValues() throws IOException, InterruptedException { - StepExecution execution = new PublishChecksStep().start(context); + StepExecution execution = new PublishChecksStep().start(getStepContext()); assertThat(execution).isInstanceOf(PublishChecksStep.PublishChecksStepExecution.class); assertThat(((PublishChecksStep.PublishChecksStepExecution)execution).extractChecksDetails()) .usingRecursiveComparison() @@ -51,9 +50,9 @@ void shouldPublishCheckWithDefaultValues() throws IOException, InterruptedExcept @Test void shouldPublishCheckWithStatusInProgress() throws IOException, InterruptedException { PublishChecksStep step = getModifiedPublishChecksStepObject("an in progress build", - ChecksStatus.IN_PROGRESS,null); + ChecksStatus.IN_PROGRESS, null); - StepExecution execution = step.start(context); + StepExecution execution = step.start(getStepContext()); assertThat(execution).isInstanceOf(PublishChecksStep.PublishChecksStepExecution.class); assertThat(((PublishChecksStep.PublishChecksStepExecution)execution).extractChecksDetails()) .usingRecursiveComparison() @@ -73,9 +72,9 @@ void shouldPublishCheckWithStatusInProgress() throws IOException, InterruptedExc @Test void shouldPublishCheckWithStatusQueue() throws IOException, InterruptedException { PublishChecksStep step = getModifiedPublishChecksStepObject("a queued build", - ChecksStatus.QUEUED,null); + ChecksStatus.QUEUED, null); - StepExecution execution = step.start(context); + StepExecution execution = step.start(getStepContext()); assertThat(execution).isInstanceOf(PublishChecksStep.PublishChecksStepExecution.class); assertThat(((PublishChecksStep.PublishChecksStepExecution)execution).extractChecksDetails()) .usingRecursiveComparison() @@ -97,7 +96,7 @@ void shouldPublishCheckWithSetValues() throws IOException, InterruptedException PublishChecksStep step = getModifiedPublishChecksStepObject("a failed build", ChecksStatus.IN_PROGRESS, ChecksConclusion.FAILURE); - StepExecution execution = step.start(context); + StepExecution execution = step.start(getStepContext()); assertThat(execution).isInstanceOf(PublishChecksStep.PublishChecksStepExecution.class); assertThat(((PublishChecksStep.PublishChecksStepExecution)execution).extractChecksDetails()) .usingRecursiveComparison()