Skip to content
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

Add capturing checks publisher, make publishChecks withChecksName aware #55

Merged
merged 4 commits into from
Dec 22, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package io.jenkins.plugins.checks.api.test;

import hudson.ExtensionList;
import hudson.model.Job;
import hudson.model.Run;
import hudson.model.TaskListener;
import io.jenkins.plugins.checks.api.ChecksDetails;
import io.jenkins.plugins.checks.api.ChecksPublisher;
import io.jenkins.plugins.checks.api.ChecksPublisherFactory;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;

/**
* Implementation of {@link ChecksPublisher} for use in testing, that records each captured checks in a simple list.
*
* For example:
*
* <pre>
* public class ChecksPublishingTest {
* &#64;TestExtension
* public static final CapturingChecksPublisher.Factory PUBLISHER_FACTORY = new CapturingChecksPublisher.Factory();
*
* &#64;After
* public void clearPublishedChecks() {
* PUBLISHER_FACTORY.getPublishedChecks().clear();
* }
* }
* </pre>
*/
public class CapturingChecksPublisher extends ChecksPublisher {

private final List<ChecksDetails> publishedChecks = new ArrayList<>();

@Override
public void publish(final ChecksDetails details) {
publishedChecks.add(details);
}

/**
* Implementation of {@link ChecksPublisherFactory} that returns a {@link CapturingChecksPublisher}.
*/
public static class Factory extends ChecksPublisherFactory {

private final CapturingChecksPublisher publisher = new CapturingChecksPublisher();

@Override
protected Optional<ChecksPublisher> createPublisher(final Run<?, ?> run, final TaskListener listener) {
return Optional.of(publisher);
}

@Override
protected Optional<ChecksPublisher> createPublisher(final Job<?, ?> job, final TaskListener listener) {
return Optional.of(publisher);
}

public List<ChecksDetails> getPublishedChecks() {
return ExtensionList.lookup(Factory.class).get(0).publisher.publishedChecks;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* Provides default Findbugs annotations.
*/
@DefaultAnnotation(NonNull.class)
package io.jenkins.plugins.checks.api.test;

import edu.umd.cs.findbugs.annotations.DefaultAnnotation;
import edu.umd.cs.findbugs.annotations.NonNull;
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import edu.hm.hafner.util.VisibleForTesting;
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.Util;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.util.ListBoxModel;
Expand Down Expand Up @@ -180,9 +181,16 @@ protected Void run() throws IOException, InterruptedException {
}

@VisibleForTesting
ChecksDetails extractChecksDetails() {
ChecksDetails extractChecksDetails() throws IOException, InterruptedException {
// If a checks name has been provided as part of the step, use that.
// If not, check to see if there is an active ChecksInfo context (e.g. from withChecks).
String checksName = Optional.ofNullable(Util.fixEmpty(step.getName())).orElse(
Copy link
Contributor

Choose a reason for hiding this comment

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

For better grammar

Suggested change
String checksName = Optional.ofNullable(Util.fixEmpty(step.getName())).orElse(
StringUtils.defaultIfEmpty(step.getName(),
Optional.ofNullable(getContext().get(ChecksInfo.class))
.map(ChecksInfo::getName)
.orElse(StringUtils.EMPTY))

Optional.ofNullable(getContext().get(ChecksInfo.class))
.map(ChecksInfo::getName)
.orElse(StringUtils.EMPTY)
);
return new ChecksDetails.ChecksDetailsBuilder()
.withName(step.getName())
.withName(checksName)
.withStatus(step.getStatus())
.withConclusion(step.getConclusion())
.withDetailsURL(step.getDetailsURL())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,37 @@
package io.jenkins.plugins.checks.steps;

import io.jenkins.plugins.checks.api.ChecksConclusion;
import io.jenkins.plugins.checks.api.ChecksDetails;
import io.jenkins.plugins.checks.api.ChecksStatus;
import io.jenkins.plugins.checks.api.test.CapturingChecksPublisher;
import io.jenkins.plugins.util.IntegrationTestWithJenkinsPerTest;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.TestExtension;

import java.io.IOException;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.eq;

/**
* Tests the pipeline step to publish checks.
*/
public class PublishChecksStepITest extends IntegrationTestWithJenkinsPerTest {

/**
* Provide a {@link CapturingChecksPublisher} to check published checks on each test.
*/
@TestExtension
public static final CapturingChecksPublisher.Factory PUBLISHER_FACTORY = new CapturingChecksPublisher.Factory();

/**
* Tests that the step "publishChecks" can be used in pipeline script.
*
* @throws IOException if fails get log from run
*/
@SuppressWarnings("OptionalGetWithoutIsPresent")
Copy link
Member

Choose a reason for hiding this comment

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

I would just extract the output to a variable and chuck and orElseThrow instead of .get

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That'll be a lot of orElseThrows! Might make it a bit less readable.

@Test
public void shouldPublishChecksWhenUsingPipeline() throws IOException {
WorkflowJob job = createPipeline();
Expand All @@ -26,7 +40,17 @@ public void shouldPublishChecksWhenUsingPipeline() throws IOException {
+ "text: 'Pipeline support for checks', status: 'IN_PROGRESS', conclusion: 'NONE'"));

assertThat(JenkinsRule.getLog(buildSuccessfully(job)))
.contains("[Pipeline] publishChecks")
.contains("[Checks API] No suitable checks publisher found.");
.contains("[Pipeline] publishChecks");

assertThat(PUBLISHER_FACTORY.getPublishedChecks().size()).isEqualTo(1);

ChecksDetails details = PUBLISHER_FACTORY.getPublishedChecks().get(0);

assertThat(details.getName().get()).isEqualTo("customized-check");
Copy link
Member

Choose a reason for hiding this comment

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

Typically, one would use in AssertJ:

Suggested change
assertThat(details.getName().get()).isEqualTo("customized-check");
assertThat(details.getName()).isPresent().get().isEqualTo("customized-check");

This will also not require the annotation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, neat! Thanks.

assertThat(details.getOutput().get().getTitle().get()).isEqualTo("Publish Checks Step");
assertThat(details.getOutput().get().getSummary().get()).isEqualTo("customized check created in pipeline");
assertThat(details.getOutput().get().getText().get()).isEqualTo("Pipeline support for checks");
assertThat(details.getStatus()).isEqualTo(ChecksStatus.IN_PROGRESS);
assertThat(details.getConclusion()).isEqualTo(ChecksConclusion.NONE);
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
package io.jenkins.plugins.checks.steps;

import hudson.model.Run;
import io.jenkins.plugins.checks.api.ChecksConclusion;
import io.jenkins.plugins.checks.api.ChecksDetails;
import io.jenkins.plugins.checks.api.ChecksStatus;
import io.jenkins.plugins.checks.api.test.CapturingChecksPublisher;
import io.jenkins.plugins.util.IntegrationTestWithJenkinsPerTest;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.steps.*;
import org.junit.After;
import org.junit.Test;
import org.jvnet.hudson.test.TestExtension;
import org.kohsuke.stapler.DataBoundConstructor;
Expand All @@ -20,6 +25,7 @@
* Tests the "withChecks" step.
*/
public class WithChecksStepITest extends IntegrationTestWithJenkinsPerTest {

/**
* Tests that the step can inject the {@link ChecksInfo} into the closure.
*/
Expand All @@ -31,6 +37,68 @@ public void shouldInjectChecksInfoIntoClosure() {
buildSuccessfully(job);
}

/**
* Provide a {@link CapturingChecksPublisher} to check published checks on each test.
*/
@TestExtension
public static final CapturingChecksPublisher.Factory PUBLISHER_FACTORY = new CapturingChecksPublisher.Factory();

/**
* Clear captured checks on the {@link WithChecksStepITest#PUBLISHER_FACTORY} after each test.
*/
@After
public void clearPublisher() {
PUBLISHER_FACTORY.getPublishedChecks().clear();
}

/**
* Test that the publishChecks step picks up names from the withChecks context.
*/
@SuppressWarnings("OptionalGetWithoutIsPresent")
@Test
public void publishChecksShouldTakeNameFromWithChecks() {
WorkflowJob job = createPipeline();
job.setDefinition(asStage("withChecks('test injection') { publishChecks() }"));

buildSuccessfully(job);

assertThat(PUBLISHER_FACTORY.getPublishedChecks().size()).isEqualTo(2);
ChecksDetails autoChecks = PUBLISHER_FACTORY.getPublishedChecks().get(0);
ChecksDetails manualChecks = PUBLISHER_FACTORY.getPublishedChecks().get(1);

assertThat(autoChecks.getName().get()).isEqualTo("test injection");
assertThat(autoChecks.getStatus()).isEqualTo(ChecksStatus.IN_PROGRESS);
assertThat(autoChecks.getConclusion()).isEqualTo(ChecksConclusion.NONE);

assertThat(manualChecks.getName().get()).isEqualTo("test injection");
assertThat(manualChecks.getStatus()).isEqualTo(ChecksStatus.COMPLETED);
assertThat(manualChecks.getConclusion()).isEqualTo(ChecksConclusion.SUCCESS);
}

/**
* Tests that withChecks step ignores names from the withChecks context if one has been explicitly set.
*/
@SuppressWarnings("OptionalGetWithoutIsPresent")
@Test
public void publishChecksShouldTakeNameFromWithChecksUnlessOverridden() {
WorkflowJob job = createPipeline();
job.setDefinition(asStage("withChecks('test injection') { publishChecks name: 'test override' }"));

buildSuccessfully(job);

assertThat(PUBLISHER_FACTORY.getPublishedChecks().size()).isEqualTo(2);
ChecksDetails autoChecks = PUBLISHER_FACTORY.getPublishedChecks().get(0);
ChecksDetails manualChecks = PUBLISHER_FACTORY.getPublishedChecks().get(1);

assertThat(autoChecks.getName().get()).isEqualTo("test injection");
assertThat(autoChecks.getStatus()).isEqualTo(ChecksStatus.IN_PROGRESS);
assertThat(autoChecks.getConclusion()).isEqualTo(ChecksConclusion.NONE);

assertThat(manualChecks.getName().get()).isEqualTo("test override");
assertThat(manualChecks.getStatus()).isEqualTo(ChecksStatus.COMPLETED);
assertThat(manualChecks.getConclusion()).isEqualTo(ChecksConclusion.SUCCESS);
}

/**
* Assert that the injected {@link ChecksInfo} is as expected.
*/
Expand Down
2 changes: 2 additions & 0 deletions src/test/resources/design.puml
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ skinparam component {
}

[API] <<..checks.api>>
[Test] <<..checks.api.test>>
[Steps] <<..checks.steps>>
[Status] <<..checks.status>>
[Checks] <<..checks>>

[Steps] --> [API]
[Status] --> [API]
[Checks] --> [API]
[Test] --> [API]

@enduml