Get comment file from workspace, fixes #343

[jrudolph]

Using the workspace will find the comment file even when the build was executed on a slave node.

[reverson]

What's the status of this Pull Request? I'm running into this issue, and my current setup uses 0 executors on the master, so I can't pin this job to the master.

[reverson]

Can one of the admins verify this patch?

[reverson]

@benpatterson (sorry to @ you directly, I see you're generally the most active). What needs to be done to have this looked at?

Sorry to be pushy, I just fought with this behavior for a good few hours before I realized what was going on.

[roguishmountain]

Hi @benpatterson, do you know when you'll have time to verify and merge this patch? I tested this patch locally and it seems to work. The merge conflicts are minor and the style guide issues are also minor. I'm not sure if you are the correct person to tag, but it looks like you have done all the releases recently. Thanks!

[benpatterson]

Hi @roguishmountain -- Sorry for the quiet on my end. I am no longer the maintainer; however the new maintainers have been somewhat engaged. I CC @bjoernhaeuser and @samrocketman here in case they can give you a better idea/feedback.

Also CC @jrudolph

[samrocketman]

@roverson @jrudolf can this be rebased off of the latest master?

[jrudolph]

I merged it with the Github interface.

[samrocketman]

Question: I'm not familiar with this feature of GHPRB. Can you describe more about it?

In general, I personally avoid workspaces in lieu of API calls directly to GitHub when possible.

Note: my question doesn't block review or merging. Just a curiosity. I'll review this after work hours.

[samrocketman]

Also, looks like the Jenkins build returned checkstyle errors. @bjoernhaeuser added checkstyle to improve maintenance going forward. Please update so the checkstyle passes.

[roguishmountain]

@samrocketman This feature allows you to create custom messages to post on the PR that was built. Normally after the commit is built there is a Jenkins wide message that gets posted to the PR such as "commit failed to build" or "commit succeeded". This feature allows you to create a file in the workspace and the content of that file will also get posted to the PR. previously, the plugin would look for this file on the jenkins server. this PR changes this to search in the project workspace for the file. @jrudolph has an example of this here: akka/akka-http#865 (comment)

[samrocketman]

@roguishmountain thanks for taking the time to explain. I fully understand now. It makes sense to support loading from workspaces on agents and not force building on master for this feature (building on master being a known bad practice).

As soon as this is updated fixing the build issues I'll take the time to review and test upgrade ability.

[samrocketman]

In general, it would be nice if a test was included but does not block merging. Update the checkstyle fixes from the failed build and I would say good to merge.

[jrudolph]

Fixed the checkstyle issues.

[jrudolph]

Thanks for merging, @samrocketman!

[ntindall]

has this been published with 1.42.0 ? I am still seeing this issue (with a declarative pipeline):

pipeline {
  agent {
    node {
      label 'CI-Slave'
    }
  }

  stages {
    stage('Prepare') {
      steps {
        sh "env"
      }
    }
    stage('Output') {
      steps {
        sh "echo hello world > output.txt"
      }
    }
    stage('Parse body') {
      steps {
        echo "$ghprbCommentBody"
      }
    }
    stage('ls') {
      steps {
        sh "ls -lah"
        sh "pwd"
      }
    }
  }

  post {
    always {
      archiveArtifacts artifacts: 'output.txt', fingerprint: true
    }
  }
}

Adding one-line test results to commit status...
Setting status of 3c9402a5dd8a6fef5664d2e945690f38c4776902 to SUCCESS with url https://ci.plaid.com/job/pdaas-constellation/24/ and message: 'Constellation results
 No test results found.'
Using context: constellation
Couldn't read comment file at output.txt
java.io.FileNotFoundException: File 'output.txt' does not exist
	at org.apache.commons.io.FileUtils.openInputStream(FileUtils.java:299)
	at org.apache.commons.io.FileUtils.readFileToString(FileUtils.java:1711)
	at org.apache.commons.io.FileUtils.readFileToString(FileUtils.java:1748)
	at org.jenkinsci.plugins.ghprb.extensions.comments.GhprbCommentFile.postBuildComment(GhprbCommentFile.java:60)
	at org.jenkinsci.plugins.ghprb.GhprbBuilds.commentOnBuildResult(GhprbBuilds.java:241)
	at org.jenkinsci.plugins.ghprb.GhprbBuilds.onCompleted(GhprbBuilds.java:215)
	at org.jenkinsci.plugins.ghprb.GhprbBuildListener.onCompleted(GhprbBuildListener.java:28)
	at hudson.model.listeners.RunListener.fireCompleted(RunListener.java:211)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun.finish(WorkflowRun.java:652)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun.access$1400(WorkflowRun.java:134)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun$GraphL.onNewHead(WorkflowRun.java:967)
	at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.notifyListeners(CpsFlowExecution.java:1438)
	at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup$3.run(CpsThreadGroup.java:417)
	at org.jenkinsci.plugins.workflow.cps.CpsVmExecutorService$1.run(CpsVmExecutorService.java:35)
	at hudson.remoting.SingleLaneExecutorService$1.run(SingleLaneExecutorService.java:112)
	at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

[jrudolph]

@ntindall, I created #708 to track this issue.

[samrocketman]

Looking at the merge commit 3df5a93 it is indeed part of the 1.42.0 release.