-
Notifications
You must be signed in to change notification settings - Fork 28
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
Listen to checkout instead of on started. #13
Changes from all commits
ab606af
430aab9
072aabf
cbeae0e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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}. | ||
|
@@ -32,53 +36,72 @@ private static void publish(final ChecksPublisher publisher, final ChecksStatus | |
/** | ||
* {@inheritDoc} | ||
* | ||
* <p> | ||
* Listens to the queue and publishes checks in "queued" state for entering items. | ||
* </p> | ||
*/ | ||
@Extension | ||
public static class JobScheduledListener extends QueueListener { | ||
/** | ||
* {@inheritDoc} | ||
* | ||
* <p> | ||
* When a job enters queue, creates the check on "queued". | ||
* </p> | ||
*/ | ||
@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, | ||
ChecksConclusion.NONE); | ||
} | ||
} | ||
|
||
/** | ||
* {@inheritDoc} | ||
* | ||
* Listens to the run and publishes checks for started and completed run. | ||
* <p> | ||
* Listens to the SCM checkout and publishes checks. | ||
* </p> | ||
*/ | ||
@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". | ||
* </p> | ||
*/ | ||
@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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
* </p> | ||
*/ | ||
@Extension | ||
public static class JobCompletedListener extends RunListener<Run<?, ?>> { | ||
/** | ||
* {@inheritDoc} | ||
* | ||
* <p> | ||
* When a job completes, completes the check. | ||
* </p> | ||
*/ | ||
@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) { | ||
|
There was a problem hiding this comment.
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...