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

Print console log #17

Merged
merged 4 commits into from
Aug 4, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
Expand Up @@ -4,12 +4,13 @@
import java.time.Instant;
import java.util.Date;

import hudson.model.TaskListener;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Level;
import org.apache.log4j.Logger;

import edu.hm.hafner.util.VisibleForTesting;

import org.apache.log4j.Level;
import org.apache.log4j.Logger;
import org.kohsuke.github.GHCheckRunBuilder;
import org.kohsuke.github.GitHub;

Expand All @@ -23,23 +24,25 @@
* A publisher which publishes GitHub check runs.
*/
public class GitHubChecksPublisher extends ChecksPublisher {
private static final String GITHUB_URL = "https://api.github.com";
private static final Logger LOGGER = Logger.getLogger(GitHubChecksPublisher.class.getName());

private final GitHubChecksContext context;
private final TaskListener listener;

/**
* {@inheritDoc}.
*
* @param context
* a context which contains SCM properties
*/
public GitHubChecksPublisher(final GitHubChecksContext context) {
public GitHubChecksPublisher(final GitHubChecksContext context, final TaskListener listener) {
super();

this.context = context;
this.listener = listener;
}

private static final Logger LOGGER = Logger.getLogger(GitHubChecksPublisher.class.getName());
private static final String GITHUB_URL = "https://api.github.com";

/**
* Publishes a GitHub check run.
*
Expand All @@ -54,10 +57,12 @@ public void publish(final ChecksDetails details) {
credentials);
GHCheckRunBuilder builder = createBuilder(gitHub, new GitHubChecksDetails(details));
builder.create();
listener.getLogger().println("GitHub checks have been published.");
}
catch (IllegalStateException | IOException e) {
//TODO: log to the build console
LOGGER.log(Level.WARN, "Could not publish GitHub check run", e);
Copy link
Member

Choose a reason for hiding this comment

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

still worth logging this to the system?

String message = "Failed Publishing GitHub checks: " + e;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
String message = "Failed Publishing GitHub checks: " + e;
String message = "Failed Publishing GitHub checks: ";

LOGGER.log(Level.WARN, message);
Copy link
Member

Choose a reason for hiding this comment

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

always log the exception with the throwable arg

Suggested change
LOGGER.log(Level.WARN, message);
LOGGER.log(Level.WARN, message, e);

listener.getLogger().println(message);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
listener.getLogger().println(message);
listener.getLogger().println(message + e);

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,28 +35,33 @@ public GitHubChecksPublisherFactory() {
}

@Override
protected Optional<ChecksPublisher> createPublisher(final Run<?, ?> run, final TaskListener taskListener) {
return createPublisher(new GitHubChecksContext(run));
protected Optional<ChecksPublisher> createPublisher(final Run<?, ?> run, final TaskListener listener) {
return createPublisher(new GitHubChecksContext(run), listener);
}

@Override
protected Optional<ChecksPublisher> createPublisher(final Job<?, ?> job, final TaskListener taskListener) {
return createPublisher(new GitHubChecksContext(job));
protected Optional<ChecksPublisher> createPublisher(final Job<?, ?> job, final TaskListener listener) {
return createPublisher(new GitHubChecksContext(job), listener);
}

protected Optional<ChecksPublisher> createPublisher(final GitHubChecksContext context) {
protected Optional<ChecksPublisher> createPublisher(final GitHubChecksContext context,
final TaskListener listener) {
Job<?, ?> job = context.getJob();
Optional<GitHubSCMSource> source = scmFacade.findGitHubSCMSource(job);
if (!source.isPresent()) {
listener.getLogger().println("Skipped publishing GitHub checks: no GitHub SCM found.");
return Optional.empty();
}

String credentialsId = source.get().getCredentialsId();
if (credentialsId == null
|| !scmFacade.findGitHubAppCredentials(job, credentialsId).isPresent()) {
listener.getLogger().println("Skipped publishing GitHub checks: no GitHub APP credentials found, " +
"see https://github.com/jenkinsci/github-branch-source-plugin/blob/master/docs/github-app.adoc");
return Optional.empty();
}

return Optional.of(new GitHubChecksPublisher(context));
listener.getLogger().println("Publishing GitHub checks...");
return Optional.of(new GitHubChecksPublisher(context, listener));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.github.tomakehurst.wiremock.junit.WireMockRule;
import hudson.model.Job;
import hudson.model.Run;
import hudson.model.TaskListener;
import io.jenkins.plugins.checks.api.*;
import io.jenkins.plugins.checks.api.ChecksAnnotation.ChecksAnnotationBuilder;
import io.jenkins.plugins.checks.api.ChecksAnnotation.ChecksAnnotationLevel;
Expand Down Expand Up @@ -95,6 +96,7 @@ void shouldPublishGitHubCheckRunCorrectly() throws IOException {
SCMHead head = mock(SCMHead.class);
PullRequestSCMRevision revision = mock(PullRequestSCMRevision.class);
ClassicDisplayURLProvider urlProvider = mock(ClassicDisplayURLProvider.class);
TaskListener listener = mock(TaskListener.class);

when(run.getParent()).thenReturn(job);
when(source.getCredentialsId()).thenReturn("1");
Expand All @@ -109,7 +111,7 @@ void shouldPublishGitHubCheckRunCorrectly() throws IOException {

when(urlProvider.getRunURL(run)).thenReturn("https://ci.jenkins.io");

new GitHubChecksPublisher(new GitHubChecksContext(run, scmFacade, urlProvider))
new GitHubChecksPublisher(new GitHubChecksContext(run, scmFacade, urlProvider), listener)
.createBuilder(new GitHubBuilder().withEndpoint(wireMockRule.baseUrl()).build(),
new GitHubChecksDetails(expectedDetails))
.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource;
import org.junit.jupiter.api.Test;

import java.io.PrintStream;
import java.util.Optional;

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

@SuppressWarnings("PMD.CloseResource") // no need to close mocked PrintStream
class GitHubChecksPublisherFactoryTest {
@Test
void shouldCreateGitHubChecksPublisherFromRun() {
Expand All @@ -26,7 +28,7 @@ void shouldCreateGitHubChecksPublisherFromRun() {
when(scmFacade.findGitHubAppCredentials(job, "credentials id")).thenReturn(Optional.of(credentials));

GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade);
assertThat(factory.createPublisher(new GitHubChecksContext(run)))
assertThat(factory.createPublisher(new GitHubChecksContext(run), createTaskListener()))
.isPresent()
.containsInstanceOf(GitHubChecksPublisher.class);
}
Expand All @@ -43,7 +45,7 @@ void shouldReturnGitHubChecksPublisherFromJob() {
.thenReturn(Optional.empty());

GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade);
assertThat(factory.createPublisher(new GitHubChecksContext(job)))
assertThat(factory.createPublisher(new GitHubChecksContext(job), createTaskListener()))
.isNotPresent();
}

Expand All @@ -52,7 +54,7 @@ void shouldReturnEmptyWhenNoGitHubSCMSourceIsConfigured() {
Run run = mock(Run.class);

GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory();
assertThat(factory.createPublisher(new GitHubChecksContext(run)))
assertThat(factory.createPublisher(new GitHubChecksContext(run), createTaskListener()))
.isNotPresent();
}

Expand All @@ -68,7 +70,7 @@ void shouldReturnEmptyWhenNoCredentialsIsConfigured() {
when(source.getCredentialsId()).thenReturn(null);

GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade);
assertThat(factory.createPublisher(new GitHubChecksContext(job)))
assertThat(factory.createPublisher(new GitHubChecksContext(job), createTaskListener()))
.isNotPresent();
}

Expand All @@ -86,7 +88,16 @@ void shouldReturnEmptyWhenNoGitHubAppCredentialsIsConfigured() {
.thenReturn(Optional.empty());

GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade);
assertThat(factory.createPublisher(new GitHubChecksContext(job)))
assertThat(factory.createPublisher(new GitHubChecksContext(job), createTaskListener()))
.isNotPresent();
}
}

TaskListener createTaskListener() {
PrintStream stream = mock(PrintStream.class);
TaskListener listener = mock(TaskListener.class);

when(listener.getLogger()).thenReturn(stream);

return listener;
}
}