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.

Sign up for GitHub

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

Jump to bottom

Listen to checkout instead of on started. #13

Merged
merged 4 commits into from
Aug 21, 2020
Merged

Listen to checkout instead of on started. #13

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ab606af
Listen to checkout instead of on started.
XiongKezhi Aug 16, 2020
430aab9
Use TaskListener.NULL for checks publishing when job enters queue
XiongKezhi Aug 17, 2020
072aabf
Update src/main/java/io/jenkins/plugins/checks/BuildStatusChecksPubli…
timja Aug 17, 2020
cbeae0e
Close opening tag in JavaDoc
XiongKezhi Aug 18, 2020
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
52 changes: 32 additions & 20 deletions src/main/java/io/jenkins/plugins/checks/BuildStatusChecksPublisher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
package io.jenkins.plugins.checks;

import edu.umd.cs.findbugs.annotations.NonNull;

import edu.umd.cs.findbugs.annotations.Nullable;
import hudson.Extension;
import hudson.FilePath;
import hudson.model.*;

import hudson.model.listeners.RunListener;
import hudson.model.listeners.SCMListener;
import hudson.model.queue.QueueListener;
import hudson.scm.SCM;
import hudson.scm.SCMRevisionState;
import io.jenkins.plugins.checks.api.ChecksConclusion;
import io.jenkins.plugins.checks.api.ChecksDetails.ChecksDetailsBuilder;
import io.jenkins.plugins.checks.api.ChecksPublisher;
import io.jenkins.plugins.checks.api.ChecksPublisherFactory;
import io.jenkins.plugins.checks.api.ChecksStatus;

import java.io.File;

/**
* A publisher which publishes different statuses through the checks API based on the stage of the {@link Queue.Item}
* or {@link Run}.
Expand All @@ -31,54 +35,62 @@ private static void publish(final ChecksPublisher publisher, final ChecksStatus

/**
* {@inheritDoc}
*
* <p>
* Listens to the queue and publishes checks in "queued" state for entering items.
*/
@Extension
public static class JobScheduledListener extends QueueListener {
/**
* {@inheritDoc}
*
* <p>
* When a job enters queue, creates the check on "queued".
*/
@Override
public void onEnterWaiting(final Queue.WaitingItem wi) {
if (!(wi.task instanceof Job)) {
if (!(wi.task instanceof Job)) {
return;
}

publish(ChecksPublisherFactory.fromJob((Job)wi.task, null),
ChecksStatus.QUEUED, ChecksConclusion.NONE);
publish(ChecksPublisherFactory.fromJob((Job) wi.task, TaskListener.NULL),ChecksStatus.QUEUED,
timja marked this conversation as resolved.
Show resolved Hide resolved
ChecksConclusion.NONE);
Copy link
Member

Choose a reason for hiding this comment

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

The Codecov annotation is really nice 🥰

I think this is a good topic for a thesis for one of my students that can be based on the new Coverage and Checks API plugins...

}
}

/**
* {@inheritDoc}
*
* Listens to the run and publishes checks for started and completed run.
* <p>
* Listens to the SCM checkout and publishes checks.
*/
@Extension
public static class JobStartedListener extends RunListener<Run<?, ?>> {
public static class JobCheckoutListener extends SCMListener {
/**
* {@inheritDoc}
*
* When a job starts, updates the check to "in progress".
* <p>
* When checkout finished, update the check to "in progress".
*/
@Override
public void onStarted(final Run run, final TaskListener listener) {
publish(ChecksPublisherFactory.fromRun(run, listener),
ChecksStatus.IN_PROGRESS, ChecksConclusion.NONE);
public void onCheckout(final Run<?, ?> run, final SCM scm, final FilePath workspace,
Copy link
Member

Choose a reason for hiding this comment

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

Is it a problem if this method will be invoked multiple times in a job? E.g., a job may have several SCM checkouts. I'm not sure how our CI is configured, but from https://ci.jenkins.io/job/Plugins/job/analysis-model/job/master/984/ it looks like we have 2 checkouts for each job.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is there a way to inject an extension to Jenkins in integration test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder why there are two checkouts so that I can reproduce that and test.

Copy link
Member

Choose a reason for hiding this comment

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

Yes you can have a static inner class annotated with TestExtension.

Our jobs should be just checking out the shared library and the repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't find a better way to test the listeners than adding log information (maybe fine level) and then check the log, but I don't know if this is a good practice to add such log information for tests.

Our jobs should be just checking out the shared library and the repo.

I don't understand this, should we do any other works (check whether the checkout is about the shared library or the repo) in this onCheckout method?

Copy link
Member

Choose a reason for hiding this comment

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

should be fine.

re: testing,

I wouldn't worry about testing listeners get invoked that's tested by Jenkins, if there's logic in it you should test that separately by just passing required params and checking it does what is expected

final TaskListener listener, @Nullable final File changelogFile,
@Nullable final SCMRevisionState pollingBaseline) {
publish(ChecksPublisherFactory.fromRun(run, listener), ChecksStatus.IN_PROGRESS, ChecksConclusion.NONE);
}
}

/**
* {@inheritDoc}
* <p>
* Listens to the run and publishes checks.
*/
@Extension
public static class JobCompletedListener extends RunListener<Run<?, ?>> {
/**
* {@inheritDoc}
*
* <p>
* When a job completes, completes the check.
*/
@Override
public void onCompleted(final Run run, @NonNull final TaskListener listener) {
publish(ChecksPublisherFactory.fromRun(run, listener),
ChecksStatus.COMPLETED, extractConclusion(run));
public void onCompleted(final Run run, @Nullable final TaskListener listener) {
publish(ChecksPublisherFactory.fromRun(run, listener), ChecksStatus.COMPLETED, extractConclusion(run));
}

private ChecksConclusion extractConclusion(final Run<?, ?> run) {
Expand Down