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 withChecks step #49

Merged
merged 11 commits into from
Dec 20, 2020
10 changes: 9 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,20 @@ If enabled, the statuses will be published in different stages of a Jenkins buil

### Pipeline Usage

Instead of depending on consumers plugins, the users can publish their checks directly in the pipeline script:
- publishChecks: you can publish checks directly in the pipeline script instead of depending on consumer plugins:

```
publishChecks name: 'example', title: 'Pipeline Check', summary: 'check through pipeline', text: 'you can publish checks in pipeline script', detailsURL: 'https://github.com/jenkinsci/checks-api-plugin#pipeline-usage'
```

- withChecks: you can inject the check's name into the closure for other steps to use:
XiongKezhi marked this conversation as resolved.
Show resolved Hide resolved

```
withChecks(name: 'injected name') {
// some other steps that will extract the name
}
```

timja marked this conversation as resolved.
Show resolved Hide resolved
## Guides

- [Consumers Guide](docs/consumers-guide.md)
Expand Down
3 changes: 1 addition & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@

<properties>
<java.level>8</java.level>
<revision>1.1.2</revision>
<changelist>-SNAPSHOT</changelist>
<revision>1.2.0</revision>
Copy link
Member

Choose a reason for hiding this comment

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

It still should be a SNAPSHOT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The sem-ver plugin will warn about compatibility if add SNAPSHOT


<!-- Jenkins Plug-in Dependencies Versions -->
<plugin-util-api.version>1.6.0</plugin-util-api.version>
Expand Down
26 changes: 26 additions & 0 deletions src/main/java/io/jenkins/plugins/checks/steps/ChecksInfo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package io.jenkins.plugins.checks.steps;

import java.io.Serializable;

/**
* A collection of checks properties that will be injected into {@link WithChecksStep} closure.
*/
public class ChecksInfo implements Serializable {
private static final long serialVersionUID = 1L;

private final String name;

/**
* Creates a {@link ChecksInfo} with checks name.
*
* @param name
* the name of the check
*/
public ChecksInfo(final String name) {
this.name = name;
}

public String getName() {
return name;
}
}
104 changes: 104 additions & 0 deletions src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package io.jenkins.plugins.checks.steps;

import edu.hm.hafner.util.VisibleForTesting;
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.model.Run;
import hudson.model.TaskListener;
import org.jenkinsci.plugins.workflow.steps.*;
import org.kohsuke.stapler.DataBoundConstructor;

import java.io.Serializable;
import java.util.*;

/**
* Pipeline step that injects a {@link ChecksInfo} into the closure.
*/
public class WithChecksStep extends Step implements Serializable {
private static final long serialVersionUID = 1L;

private final String name;

/**
* Creates the step with a name to inject.
*
* @param name
* name to inject
*/
@DataBoundConstructor
public WithChecksStep(final String name) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

yes it can take more than one, normally you would use a databoundsetter for any optional parameters

Then the syntax would be:

withChecks('Tests') {
...
}
withChecks(name: 'Tests', myotherparam: 'hello') {
...
}

if there's only one mandatory field in constructor then you can omit the parameter name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So should we make all params optional in case that in the future, some users may only want to inject a certain field like conclusion instead of name?

Copy link
Member

Choose a reason for hiding this comment

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

if we do that then the user will have to specify the name in a more verbose way,

withChecks(name: 'hello') rather than withChecks('hello')

Copy link
Contributor Author

@XiongKezhi XiongKezhi Dec 16, 2020

Choose a reason for hiding this comment

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

Specify the name wouldn't be annoying for users IMO but make it mandatory will be an issue for future changes?

Also, it seems the snippet generator will always specify name

@mrginglymus What do you think? Do you think there will be situations where you do not want to specify the name but the status (e.g. publish several checks in the same status (say in progress) but with different names in the closure)?

Copy link
Member

Choose a reason for hiding this comment

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

I think it should be fine to be mandatory

super();

this.name = name;
}

public String getName() {
return name;
}

@Override
public StepExecution start(final StepContext stepContext) {
return new WithChecksStepExecution(stepContext, this);
}

/**
* This step's descriptor which defines the function name ("withChecks") and required context.
*/
@Extension
public static class WithChecksStepDescriptor extends StepDescriptor {
@Override
public String getFunctionName() {
return "withChecks";
}

@Override
public Set<? extends Class<?>> getRequiredContext() {
return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(Run.class, TaskListener.class)));
}

@Override
public boolean takesImplicitBlockArgument() {
return true;
}

@NonNull
@Override
public String getDisplayName() {
return "Inject checks properties into its closure";
}
}

/**
* The step's execution to actually inject the {@link ChecksInfo} into the closure.
*/
static class WithChecksStepExecution extends AbstractStepExecutionImpl {
private static final long serialVersionUID = 1L;

private final WithChecksStep step;

WithChecksStepExecution(final StepContext context, final WithChecksStep step) {
super(context);
this.step = step;
}

@Override
public boolean start() {
StepContext context = getContext();
context.newBodyInvoker()
.withContext(extractChecksInfo())
.withCallback(BodyExecutionCallback.wrap(context))
.start();
return false;
}

@VisibleForTesting
ChecksInfo extractChecksInfo() {
return new ChecksInfo(step.name);
}

@Override
public void stop(final Throwable cause) {
getContext().onFailure(cause);
}
}
}
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.steps;

import edu.umd.cs.findbugs.annotations.DefaultAnnotation;
import edu.umd.cs.findbugs.annotations.NonNull;
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:f="/lib/form">

<f:entry title="${%title.name}" field="name">
<f:textbox />
</f:entry>

</j:jelly>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
title.name=Name
104 changes: 104 additions & 0 deletions src/test/java/io/jenkins/plugins/checks/steps/WithChecksStepITest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package io.jenkins.plugins.checks.steps;

import hudson.model.Run;
import io.jenkins.plugins.util.IntegrationTestWithJenkinsPerTest;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.steps.*;
import org.junit.Test;
import org.jvnet.hudson.test.TestExtension;
import org.kohsuke.stapler.DataBoundConstructor;

import java.io.Serializable;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import static org.assertj.core.api.Assertions.assertThat;

/**
* Tests the "withChecks" step.
*/
public class WithChecksStepITest extends IntegrationTestWithJenkinsPerTest {
/**
* Tests that the step can inject the {@link ChecksInfo} into the closure.
*/
@Test
public void shouldInjectChecksInfoIntoClosure() {
WorkflowJob job = createPipeline();
job.setDefinition(asStage("withChecks('test injection') { assertChecksInfoInjection('test injection') }"));

buildSuccessfully(job);
}

/**
* Assert that the injected {@link ChecksInfo} is as expected.
*/
@TestExtension
public static class AssertChecksInfoInjectionStep extends Step implements Serializable {
private static final long serialVersionUID = 1L;

private final ChecksInfo expected;

/**
* Required by {@link TestExtension} annotation.
*/
public AssertChecksInfoInjectionStep() {
this("");
}

/**
* Creates the step with expected name injected.
*
* @param expectedName
* expected name that will be injected
*/
@DataBoundConstructor
public AssertChecksInfoInjectionStep(final String expectedName) {
super();

expected = new ChecksInfo(expectedName);
}

@Override
public StepExecution start(final StepContext stepContext) {
return new AssertChecksInfoInjectionStepExecution(stepContext, this);
}

/**
* Descriptor for {@link AssertChecksInfoInjectionStep} that defines function name and required context.
*/
@TestExtension
public static class AssertChecksInfoStepInjectionDescriptor extends StepDescriptor {
@Override
public String getFunctionName() {
return "assertChecksInfoInjection";
}

@Override
public Set<? extends Class<?>> getRequiredContext() {
return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(Run.class, ChecksInfo.class)));
}
}

static class AssertChecksInfoInjectionStepExecution extends SynchronousNonBlockingStepExecution<Void> {
private static final long serialVersionUID = 1L;

private final AssertChecksInfoInjectionStep step;

AssertChecksInfoInjectionStepExecution(final StepContext context, final AssertChecksInfoInjectionStep step) {
super(context);

this.step = step;
}

@Override
protected Void run() throws Exception {
assertThat(getContext().get(ChecksInfo.class))
.usingRecursiveComparison()
.isEqualTo(step.expected);
return null;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package io.jenkins.plugins.checks.steps;

import hudson.model.Run;
import hudson.model.TaskListener;
import org.jenkinsci.plugins.workflow.steps.StepContext;
import org.junit.jupiter.api.Test;

import java.io.IOException;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class WithChecksStepTest {
@Test
void shouldStartWithCorrectExecution() 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);

assertThat(((WithChecksStep.WithChecksStepExecution) (new WithChecksStep("test").start(context)))
.extractChecksInfo())
.hasFieldOrPropertyWithValue("name", "test");
}

@Test
void shouldDefinePublishChecksStepDescriptorCorrectly() {
WithChecksStep.WithChecksStepDescriptor descriptor = new WithChecksStep.WithChecksStepDescriptor();
assertThat(descriptor.getFunctionName()).isEqualTo("withChecks");
assertThat(descriptor.getRequiredContext().toArray()).containsExactlyInAnyOrder(Run.class, TaskListener.class);
}
}