Merge pull request #22 from jenkinsci/improve-log

Improve log

src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java

@@ -3,15 +3,15 @@
 import java.io.IOException;
 import java.time.Instant;
 import java.util.Date;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 import edu.umd.cs.findbugs.annotations.Nullable;
 import hudson.model.TaskListener;
 import org.apache.commons.lang3.StringUtils;
 
 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;
 
@@ -31,6 +31,7 @@ public class GitHubChecksPublisher extends ChecksPublisher {
     private final GitHubChecksContext context;
     @Nullable
     private final TaskListener listener;
+    private final String gitHubUrl;
 
     /**
      * {@inheritDoc}.
@@ -39,10 +40,16 @@ public class GitHubChecksPublisher extends ChecksPublisher {
      *         a context which contains SCM properties
      */
     public GitHubChecksPublisher(final GitHubChecksContext context, @Nullable final TaskListener listener) {
+        this(context, listener, GITHUB_URL);
+    }
+
+    GitHubChecksPublisher(final GitHubChecksContext context, @Nullable final TaskListener listener,
+                          final String gitHubUrl) {
         super();
 
         this.context = context;
         this.listener = listener;
+        this.gitHubUrl = gitHubUrl;
     }
 
     /**
@@ -55,17 +62,19 @@ public GitHubChecksPublisher(final GitHubChecksContext context, @Nullable final
     public void publish(final ChecksDetails details) {
         try {
             GitHubAppCredentials credentials = context.getCredentials();
-            GitHub gitHub = Connector.connect(StringUtils.defaultIfBlank(credentials.getApiUri(), GITHUB_URL),
+            GitHub gitHub = Connector.connect(StringUtils.defaultIfBlank(credentials.getApiUri(), gitHubUrl),
                     credentials);
-            GHCheckRunBuilder builder = createBuilder(gitHub, new GitHubChecksDetails(details));
-            builder.create();
+
+            GitHubChecksDetails gitHubDetails = new GitHubChecksDetails(details);
+            createBuilder(gitHub, gitHubDetails).create();
             if (listener != null) {
-                listener.getLogger().println("GitHub checks have been published.");
+                listener.getLogger().printf("GitHub check (name: %s, status: %s) has been published.",
+                        gitHubDetails.getName(), gitHubDetails.getStatus());
             }
         }
         catch (IllegalStateException | IOException e) {
             String message = "Failed Publishing GitHub checks: ";
-            LOGGER.log(Level.WARN, message, e);
+            LOGGER.log(Level.WARNING, (message + details).replaceAll("[\r\n]", ""), e);
             if (listener != null) {
                 listener.getLogger().println(message + e);
             }

src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java

@@ -68,7 +68,7 @@ protected Optional<ChecksPublisher> createPublisher(final GitHubChecksContext co
         }
 
         if (listener != null) {
-            listener.getLogger().println("Publishing GitHub checks...");
+            listener.getLogger().println("Publishing GitHub check...");
         }
         return Optional.of(new GitHubChecksPublisher(context, listener));
     }

src/main/java/io/jenkins/plugins/checks/github/GitHubSCMFacade.java

@@ -15,26 +15,61 @@
 import java.util.List;
 import java.util.Optional;
 
-class GitHubSCMFacade {
-    Optional<GitHubSCMSource> findGitHubSCMSource(final Job<?, ?> job) {
+/**
+ * Facade to GitHub SCM in Jenkins, used for finding GitHub SCM of a job.
+ */
+public class GitHubSCMFacade {
+    /**
+     * Find {@link GitHubSCMSource} (or GitHub repository) used by the {@code job}.
+     *
+     * @param job
+     *         the Jenkins project
+     * @return the found GitHub SCM source used or empty
+     */
+    public Optional<GitHubSCMSource> findGitHubSCMSource(final Job<?, ?> job) {
         SCMSource source = SCMSource.SourceByItem.findSource(job);
         return source instanceof GitHubSCMSource ? Optional.of((GitHubSCMSource) source) : Optional.empty();
     }
 
-    Optional<GitHubAppCredentials> findGitHubAppCredentials(final Job<?, ?> job, final String credentialsId) {
+    /**
+     * Find {@link GitHubAppCredentials} with the {@code credentialsId} used by the {@code job}.
+     *
+     * @param job
+     *         the Jenkins project
+     * @param credentialsId
+     *         the id of the target credentials
+     * @return the found GitHub App credentials or empty
+     */
+    public Optional<GitHubAppCredentials> findGitHubAppCredentials(final Job<?, ?> job, final String credentialsId) {
         List<GitHubAppCredentials> credentials = CredentialsProvider.lookupCredentials(
                 GitHubAppCredentials.class, job, ACL.SYSTEM, Collections.emptyList());
         GitHubAppCredentials appCredentials =
                 CredentialsMatchers.firstOrNull(credentials, CredentialsMatchers.withId(credentialsId));
         return Optional.ofNullable(appCredentials);
     }
 
-    Optional<SCMHead> findHead(final Job<?, ?> job) {
+    /**
+     * Find {@link SCMHead} (or branch) used by the {@code job}.
+     *
+     * @param job
+     *         the Jenkins project
+     * @return the found SCM head or empty
+     */
+    public Optional<SCMHead> findHead(final Job<?, ?> job) {
         SCMHead head = SCMHead.HeadByItem.findHead(job);
         return Optional.ofNullable(head);
     }
 
-    Optional<SCMRevision> findRevision(final GitHubSCMSource source, final SCMHead head) {
+    /**
+     * Fetch the current {@link SCMRevision} used by the {@code head} of the {@code source}.
+     *
+     * @param source
+     *         the GitHub repository
+     * @param head
+     *         the branch
+     * @return the fetched revision or empty
+     */
+    public Optional<SCMRevision> findRevision(final GitHubSCMSource source, final SCMHead head) {
         try {
             return Optional.ofNullable(source.fetch(head, null));
         }

src/test/java/io/jenkins/plugins/checks/github/GitHubCheckRunPublishITest.java → src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherITest.java

@@ -4,7 +4,7 @@
 import com.github.tomakehurst.wiremock.junit.WireMockRule;
 import hudson.model.Job;
 import hudson.model.Run;
-import hudson.model.TaskListener;
+import hudson.util.Secret;
 import io.jenkins.plugins.checks.api.*;
 import io.jenkins.plugins.checks.api.ChecksAnnotation.ChecksAnnotationBuilder;
 import io.jenkins.plugins.checks.api.ChecksAnnotation.ChecksAnnotationLevel;
@@ -16,36 +16,53 @@
 import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource;
 import org.jenkinsci.plugins.github_branch_source.PullRequestSCMRevision;
 import org.junit.Rule;
-import org.junit.jupiter.api.Test;
-import org.kohsuke.github.GitHubBuilder;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.LoggerRule;
 
-import java.io.IOException;
 import java.time.LocalDateTime;
 import java.time.ZoneOffset;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Optional;
+import java.util.logging.Level;
 
+import static io.jenkins.plugins.checks.github.assertions.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 /**
  * Tests if the {@link GitHubChecksPublisher} actually sends out the requests to GitHub in order to publish the check
  * runs.
  */
-public class GitHubCheckRunPublishITest {
+@SuppressWarnings({"PMD.ExcessiveImports", "checkstyle:ClassDataAbstractionCoupling"})
+public class GitHubChecksPublisherITest {
+    /**
+     * Rule for Jenkins instance.
+     */
+    @Rule
+    public JenkinsRule jenkinsRule = new JenkinsRule();
+
+    /**
+     * Rule for the log system.
+     */
+    @Rule
+    public LoggerRule loggerRule = new LoggerRule();
+
     /**
      * A rule which provides a mock server.
      */
     @Rule
     public WireMockRule wireMockRule = new WireMockRule(
             WireMockConfiguration.options().dynamicPort());
 
+    /**
+     * Checks should be published to GitHub correctly when GitHub SCM is found and parameters are correctly set.
+     */
     @Test
-    void shouldPublishGitHubCheckRunCorrectly() throws IOException {
-        wireMockRule.start();
-
-        ChecksDetails expectedDetails = new ChecksDetailsBuilder()
+    public void shouldPublishGitHubCheckRunCorrectly() {
+        ChecksDetails details = new ChecksDetailsBuilder()
                 .withName("Jenkins")
                 .withStatus(ChecksStatus.COMPLETED)
                 .withDetailsURL("https://ci.jenkins.io")
@@ -76,18 +93,71 @@ void shouldPublishGitHubCheckRunCorrectly() throws IOException {
                                         .withEndColumn(30)
                                         .withTitle("Hello GitHub Checks API")
                                         .withRawDetails("a simple echo command")
-                                        .build()
-                        ))
+                                        .build()))
                         .withImages(Collections.singletonList(
                                 new ChecksImage("Jenkins",
                                         "https://ci.jenkins.io/static/cd5757a8/images/jenkins-header-logo-v2.svg",
-                                        "Jenkins Symbol")
-                        ))
+                                        "Jenkins Symbol")))
                         .build())
                 .withActions(Collections.singletonList(
                         new ChecksAction("re-run", "re-run Jenkins build", "#0")))
                 .build();
 
+        new GitHubChecksPublisher(createGitHubChecksContextWithGitHubSCM(), jenkinsRule.createTaskListener(),
+                wireMockRule.baseUrl())
+                .publish(details);
+    }
+
+    /**
+     * If exception happens when publishing checks, it should output all parameters of the check to the system log.
+     */
+    @Issue("issue-20")
+    @Test
+    public void shouldLogChecksParametersIfExceptionHappensWhenPublishChecks() {
+        loggerRule.record(GitHubChecksPublisher.class.getName(), Level.WARNING).capture(1);
+
+        ChecksDetails details = new ChecksDetailsBuilder()
+                .withName("Jenkins")
+                .withStatus(ChecksStatus.COMPLETED)
+                .withConclusion(ChecksConclusion.SUCCESS)
+                .withOutput(new ChecksOutputBuilder()
+                        .withTitle("Jenkins Check")
+                        .withSummary("# A Successful Build")
+                        .withAnnotations(Collections.singletonList(
+                                new ChecksAnnotationBuilder()
+                                        .withPath("Jenkinsfile")
+                                        .withStartLine(1)
+                                        .withEndLine(2)
+                                        .withStartColumn(0)
+                                        .withEndColumn(20)
+                                        .withAnnotationLevel(ChecksAnnotationLevel.WARNING)
+                                        .withMessage("say hello to Jenkins")
+                                        .build()))
+                        .build())
+                .build();
+
+        new GitHubChecksPublisher(createGitHubChecksContextWithGitHubSCM(), jenkinsRule.createTaskListener(),
+                wireMockRule.baseUrl())
+                .publish(details);
+
+        assertThat(loggerRule.getRecords().size()).isEqualTo(1);
+        assertThat(loggerRule.getMessages().get(0))
+                .contains("Failed Publishing GitHub checks: ")
+                .contains("name='Jenkins'")
+                .contains("status=COMPLETED")
+                .contains("conclusion=SUCCESS")
+                .contains("title='Jenkins Check'")
+                .contains("summary='# A Successful Build'")
+                .contains("path='Jenkinsfile'")
+                .contains("startLine=1")
+                .contains("endLine=2")
+                .contains("startColumn=0")
+                .contains("endColumn=20")
+                .contains("annotationLevel=WARNING")
+                .contains("message='say hello to Jenkins'");
+    }
+
+    private GitHubChecksContext createGitHubChecksContextWithGitHubSCM() {
         Job job = mock(Job.class);
         Run run = mock(Run.class);
         GitHubSCMFacade scmFacade = mock(GitHubSCMFacade.class);
@@ -96,13 +166,13 @@ 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");
         when(source.getRepoOwner()).thenReturn("XiongKezhi");
         when(source.getRepository()).thenReturn("Sandbox");
         when(revision.getPullHash()).thenReturn("18c8e2fd86e7aa3748e279c14a00dc3f0b963e7f");
+        when(credentials.getPassword()).thenReturn(Secret.fromString("password"));
 
         when(scmFacade.findGitHubSCMSource(job)).thenReturn(Optional.of(source));
         when(scmFacade.findGitHubAppCredentials(job, "1")).thenReturn(Optional.of(credentials));
@@ -111,9 +181,6 @@ void shouldPublishGitHubCheckRunCorrectly() throws IOException {
 
         when(urlProvider.getRunURL(run)).thenReturn("https://ci.jenkins.io");
 
-        new GitHubChecksPublisher(new GitHubChecksContext(run, scmFacade, urlProvider), listener)
-                .createBuilder(new GitHubBuilder().withEndpoint(wireMockRule.baseUrl()).build(),
-                        new GitHubChecksDetails(expectedDetails))
-                .create();
+        return new GitHubChecksContext(run, scmFacade, urlProvider);
     }
 }

src/test/resources/__files/check-run-with-invalid-columns-response.json

@@ -0,0 +1,11 @@
+{
+  "message": "Validation Failed",
+  "errors": [
+    {
+      "resource": "CheckRun",
+      "code": "invalid",
+      "field": "annotations"
+    }
+  ],
+  "documentation_url": "https://docs.github.com/rest/reference/checks#create-a-check-run"
+}

src/test/resources/mappings/check-run-with-invalid-columns.json

@@ -0,0 +1,22 @@
+{
+  "request": {
+    "url": "/repos/XiongKezhi/Sandbox/check-runs",
+    "method": "POST",
+    "headers": {
+      "Accept": {
+        "equalTo": "application/vnd.github.antiope-preview+json"
+      }
+    },
+    "bodyPatterns": [
+      {
+        "equalToJson": "{\"conclusion\":\"success\",\"output\":{\"title\":\"Jenkins Check\",\"summary\":\"# A Successful Build\",\"annotations\":[{\"path\":\"Jenkinsfile\",\"start_line\":1,\"end_line\":2,\"annotation_level\":\"warning\",\"message\":\"say hello to Jenkins\",\"start_column\":0,\"end_column\":20}]},\"name\":\"Jenkins\",\"details_url\":\"https://ci.jenkins.io\",\"head_sha\":\"18c8e2fd86e7aa3748e279c14a00dc3f0b963e7f\",\"status\":\"completed\"}",
+        "ignoreArrayOrder": true,
+        "ignoreExtraElements": true
+      }
+    ]
+  },
+  "response": {
+    "status": 422,
+    "bodyFileName": "check-run-with-invalid-columns-response.json"
+  }
+}