-
Notifications
You must be signed in to change notification settings - Fork 65
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
[JENKINS-30412] Offering a sandbox-friendly RunWrapper.changeSets property #5
Changes from 1 commit
5724c96
ae0eb88
64b264b
7c30710
499bc58
bacf069
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 |
---|---|---|
|
@@ -28,8 +28,8 @@ | |
<parent> | ||
<groupId>org.jenkins-ci.plugins</groupId> | ||
<artifactId>plugin</artifactId> | ||
<version>2.5</version> | ||
<relativePath /> | ||
<version>2.11</version> | ||
<relativePath/> | ||
</parent> | ||
<groupId>org.jenkins-ci.plugins.workflow</groupId> | ||
<artifactId>workflow-support</artifactId> | ||
|
@@ -52,17 +52,18 @@ | |
<repositories> | ||
<repository> | ||
<id>repo.jenkins-ci.org</id> | ||
<url>http://repo.jenkins-ci.org/public/</url> | ||
<url>https://repo.jenkins-ci.org/public/</url> | ||
</repository> | ||
</repositories> | ||
<pluginRepositories> | ||
<pluginRepository> | ||
<id>repo.jenkins-ci.org</id> | ||
<url>http://repo.jenkins-ci.org/public/</url> | ||
<url>https://repo.jenkins-ci.org/public/</url> | ||
</pluginRepository> | ||
</pluginRepositories> | ||
<properties> | ||
<jenkins.version>1.642.3</jenkins.version> | ||
<jenkins-test-harness.version>2.11</jenkins-test-harness.version> | ||
</properties> | ||
<dependencies> | ||
<dependency> | ||
|
@@ -78,7 +79,7 @@ | |
<dependency> | ||
<groupId>org.jenkins-ci.plugins</groupId> | ||
<artifactId>script-security</artifactId> | ||
<version>1.16</version> | ||
<version>1.21-SNAPSHOT</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.jboss.marshalling</groupId> | ||
|
@@ -128,6 +129,26 @@ | |
<version>1.15</version> | ||
<scope>test</scope> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.jenkins-ci.plugins</groupId> | ||
<artifactId>git</artifactId> | ||
<version>2.5.0</version> | ||
<scope>test</scope> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.jenkins-ci.plugins</groupId> | ||
<artifactId>git</artifactId> | ||
<version>2.5.0</version> | ||
<classifier>tests</classifier> | ||
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.
|
||
<scope>test</scope> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.jenkins-ci.plugins.workflow</groupId> | ||
<artifactId>workflow-scm-step</artifactId> | ||
<version>2.1</version> | ||
<classifier>tests</classifier> | ||
<scope>test</scope> | ||
</dependency> | ||
</dependencies> | ||
<build> | ||
<plugins> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,10 +28,12 @@ | |
import hudson.model.AbstractBuild; | ||
import hudson.model.Result; | ||
import hudson.model.Run; | ||
import hudson.scm.ChangeLogSet; | ||
import hudson.security.ACL; | ||
import java.io.IOException; | ||
import java.io.Serializable; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.Map; | ||
import javax.annotation.CheckForNull; | ||
import javax.annotation.Nonnull; | ||
|
@@ -177,4 +179,14 @@ public String getId() throws AbortException { | |
return build().getAbsoluteUrl(); | ||
} | ||
|
||
@Whitelisted | ||
public List<ChangeLogSet<? extends ChangeLogSet.Entry>> getChangeSets() throws Exception { | ||
Run<?,?> build = build(); | ||
try { // TODO JENKINS-24141 should not need to use reflection here | ||
return (List) build.getClass().getMethod("getChangeSets").invoke(build); | ||
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. currently this will work on |
||
} catch (NoSuchMethodException x) { | ||
return Collections.emptyList(); | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,10 @@ | |
package org.jenkinsci.plugins.workflow.support.steps.build; | ||
|
||
import hudson.model.Result; | ||
import java.util.regex.Pattern; | ||
import jenkins.plugins.git.GitSampleRepoRule; | ||
import org.hamcrest.Matcher; | ||
import org.hamcrest.core.SubstringMatcher; | ||
import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; | ||
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; | ||
import org.jenkinsci.plugins.workflow.job.WorkflowJob; | ||
|
@@ -37,13 +41,16 @@ | |
import org.junit.runners.model.Statement; | ||
import org.jvnet.hudson.test.BuildWatcher; | ||
import org.jvnet.hudson.test.Issue; | ||
import org.jvnet.hudson.test.JenkinsRule; | ||
import org.jvnet.hudson.test.RestartableJenkinsRule; | ||
|
||
@Issue("JENKINS-26834") | ||
public class RunWrapperTest { | ||
|
||
@ClassRule public static BuildWatcher buildWatcher = new BuildWatcher(); | ||
@Rule public RestartableJenkinsRule r = new RestartableJenkinsRule(); | ||
@Rule public GitSampleRepoRule sampleRepo1 = new GitSampleRepoRule(); | ||
@Rule public GitSampleRepoRule sampleRepo2 = new GitSampleRepoRule(); | ||
|
||
@Test public void historyAndPickling() { | ||
r.addStep(new Statement() { | ||
|
@@ -99,4 +106,58 @@ public class RunWrapperTest { | |
}); | ||
} | ||
|
||
@Issue("JENKINS-30412") | ||
@Test public void getChangeSets() { | ||
r.addStep(new Statement() { | ||
@Override public void evaluate() throws Throwable { | ||
sampleRepo1.init(); | ||
sampleRepo2.init(); | ||
WorkflowJob p = r.j.jenkins.createProject(WorkflowJob.class, "p"); | ||
p.setDefinition(new CpsFlowDefinition( | ||
"node {dir('1') {git($/" + sampleRepo1 + "/$)}; dir('2') {git($/" + sampleRepo2 + "/$)}}\n" + | ||
"echo(/changeSets: ${summarize currentBuild}/)\n" + | ||
"@NonCPS def summarize(b) {\n" + | ||
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. Possible to iterate without iterators or Groovy helpers but this is more concise. Chose a script which could be represented in the test without |
||
" b.changeSets.collect {cs ->\n" + | ||
" /kind=${cs.kind}; entries=/ + cs.collect {entry ->\n" + | ||
" /${entry.commitId} by ${entry.author.id} ~ ${entry.author.fullName} on ${new Date(entry.timestamp)}: ${entry.msg}: / + entry.affectedFiles.collect {file ->\n" + | ||
" /${file.editType.name} ${file.path}/\n" + | ||
" }.join('; ')\n" + | ||
" }.join(', ')\n" + | ||
" }.join(' & ')\n" + | ||
"}", true)); | ||
r.j.assertLogContains("changeSets: ", r.j.assertBuildStatusSuccess(p.scheduleBuild2(0))); | ||
sampleRepo1.write("onefile", "stuff"); | ||
sampleRepo1.git("add", "onefile"); | ||
sampleRepo1.git("commit", "--message=stuff"); | ||
assertThat(JenkinsRule.getLog(r.j.assertBuildStatusSuccess(p.scheduleBuild2(0))), containsRegexp( | ||
"changeSets: kind=git; entries=[a-f0-9]{40} by .+ ~ .+ on .+: stuff: add onefile")); | ||
sampleRepo1.write("onefile", "more stuff"); | ||
sampleRepo1.write("anotherfile", "stuff"); | ||
sampleRepo1.git("add", "onefile", "anotherfile"); | ||
sampleRepo1.git("commit", "--message=more stuff"); | ||
sampleRepo1.write("onefile", "amended"); | ||
sampleRepo1.git("add", "onefile"); | ||
sampleRepo1.git("commit", "--message=amended"); | ||
sampleRepo2.write("elsewhere", "stuff"); | ||
sampleRepo2.git("add", "elsewhere"); | ||
sampleRepo2.git("commit", "--message=second repo"); | ||
assertThat(JenkinsRule.getLog(r.j.assertBuildStatusSuccess(p.scheduleBuild2(0))), containsRegexp( | ||
"changeSets: kind=git; entries=[a-f0-9]{40} by .+ ~ .+ on .+: more stuff: (edit onefile; add anotherfile|add anotherfile; edit onefile), " + | ||
"[a-f0-9]{40} by .+ ~ .+ on .+: amended: edit onefile & " + | ||
"kind=git; entries=[a-f0-9]{40} by .+ ~ .+ on .+: second repo: add elsewhere")); | ||
} | ||
}); | ||
} | ||
// TODO why is this not in Hamcrest to begin with? | ||
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. Hmm, seems to be in hamcrest/JavaHamcrest#57 but where is that? 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. Found it in <dependency>
<groupId>org.hamcrest</groupId>
<artifactId>java-hamcrest</artifactId>
<version>2.0.0.0</version>
<scope>test</scope>
</dependency> but less useful since it does a whole-string match, whereas I want to only look for a substring, so I will stick with this implementation. |
||
private static Matcher<String> containsRegexp(final String rx) { | ||
return new SubstringMatcher(rx) { | ||
@Override protected boolean evalSubstringOf(String string) { | ||
return Pattern.compile(rx).matcher(string).find(); | ||
} | ||
@Override protected String relationship() { | ||
return "containing the regexp"; | ||
} | ||
}; | ||
} | ||
|
||
} |
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.
Need
getLog
fixed to be plain text.