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.

Sign up for GitHub

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-42971] CpsScmFlowDefinition sends build to SCMFileSystem in … #577

Merged
merged 13 commits into from
Jan 4, 2023
Merged

[JENKINS-42971] CpsScmFlowDefinition sends build to SCMFileSystem in … #577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
808c9f2
[JENKINS-42971] CpsScmFlowDefinition sends build to SCMFileSystem in …
MartinKosicky Aug 2, 2022
811b551
[JENKINS-42971] CpsScmFlowDefinition added integration test for envir…
MartinKosicky Aug 4, 2022
b69d380
Format of test
MartinKosicky Aug 4, 2022
003b2ba
[JENKINS-42971] Simplifyied test
MartinKosicky Aug 4, 2022
6b13373
[JENKINS-42971] Linting fixes
MartinKosicky Aug 5, 2022
c5273bb
Update src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScmFlowDef…
MartinKosicky Aug 5, 2022
21562e5
Using scm-api that was already merged
MartinKosicky Aug 11, 2022
9c4f46e
Bumped git plugin to newest increment
MartinKosicky Aug 11, 2022
54d0770
Bumped BOM , to fix dependencies
MartinKosicky Aug 11, 2022
1073bfa
Fix of bad merge
MartinKosicky Dec 16, 2022
24ad926
Update pom.xml
MartinKosicky Dec 16, 2022
2731d65
Merge branch 'master' into use_new_scp_api_lw_checkout
jglick Jan 3, 2023
eb24fb7
Typo in test name
jglick Jan 3, 2023
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
2 changes: 1 addition & 1 deletion plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScmFlowDefinition.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public boolean isLightweight() {
Run<?,?> build = (Run<?,?>) _build;
String expandedScriptPath = build.getEnvironment(listener).expand(scriptPath);
if (isLightweight()) {
try (SCMFileSystem fs = SCMFileSystem.of(build.getParent(), scm)) {
try (SCMFileSystem fs = SCMFileSystem.of(build.getParent(), scm, null, build)) {
if (fs != null) {
try {
String script = fs.child(expandedScriptPath).contentAsString();
Expand Down
27 changes: 27 additions & 0 deletions plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScmFlowDefinitionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,14 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.logging.Level;

import jenkins.model.Jenkins;
import jenkins.plugins.git.GitSCMFileSystem;
import jenkins.plugins.git.GitSampleRepoRule;
import jenkins.plugins.git.GitStep;
import jenkins.scm.impl.subversion.SubversionSampleRepoRule;
Expand All @@ -59,6 +63,8 @@
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasItem;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
Expand All @@ -70,6 +76,7 @@
import org.jvnet.hudson.test.BuildWatcher;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.LoggerRule;
import org.jvnet.hudson.test.SingleFileSCM;

import static org.hamcrest.MatcherAssert.assertThat;
Expand Down Expand Up @@ -195,6 +202,26 @@ public class CpsScmFlowDefinitionTest {
r.assertLogContains("version one", b);
}

@Issue("JENKINS-42971")
@Test
public void lightweight_branch_parametrised() throws Exception {
sampleRepo.init();
sampleRepo.git("checkout", "-b", "master2");
sampleRepo.write("flow.groovy", "echo 'version one'");
sampleRepo.git("add", "flow.groovy");
sampleRepo.git("commit", "--message=init");
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
p.addProperty(new ParametersDefinitionProperty(new StringParameterDefinition("BRANCH", "")));
GitSCM scm = new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null),
Collections.singletonList(new BranchSpec("${BRANCH}")), null, null, Collections.emptyList());

CpsScmFlowDefinition def = new CpsScmFlowDefinition(scm, "flow.groovy");
def.setLightweight(true);
p.setDefinition(def);

r.assertBuildStatusSuccess(p.scheduleBuild2(0, new ParametersAction(new StringParameterValue("BRANCH", "master2"))));
Copy link
Member

Choose a reason for hiding this comment

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

Do you mean to also assert that it actually loaded the specified revision of flow.groovy, for example by checking the altered message?

Copy link
Member

Choose a reason for hiding this comment

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

FTR with main reverted, the failure is as expected

java.lang.AssertionError: 
unexpected build status; build log was:
------
Started
hudson.plugins.git.GitException: Command "git fetch --tags --force --progress --prune -- origin +refs/heads/${BRANCH}:refs/remotes/origin/${BRANCH}" returned status code 128:
stdout: 
stderr: fatal: couldn't find remote ref refs/heads/${BRANCH}

	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:2734)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandWithCredentials(CliGitAPIImpl.java:2111)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.access$500(CliGitAPIImpl.java:87)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$1.execute(CliGitAPIImpl.java:623)
	at jenkins.plugins.git.GitSCMFileSystem$BuilderImpl.build(GitSCMFileSystem.java:406)
	at jenkins.scm.api.SCMFileSystem.of(SCMFileSystem.java:219)
	at jenkins.scm.api.SCMFileSystem.of(SCMFileSystem.java:191)
	at jenkins.scm.api.SCMFileSystem.of(SCMFileSystem.java:174)
	at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition.create(CpsScmFlowDefinition.java:118)
	at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition.create(CpsScmFlowDefinition.java:70)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:312)
	at hudson.model.ResourceController.execute(ResourceController.java:107)
	at hudson.model.Executor.run(Executor.java:449)
Finished: FAILURE

------

Expected: is <SUCCESS>
     but: was <FAILURE>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.jvnet.hudson.test.JenkinsRule.assertBuildStatus(JenkinsRule.java:1438)
	at org.jvnet.hudson.test.JenkinsRule.assertBuildStatus(JenkinsRule.java:1444)
	at org.jvnet.hudson.test.JenkinsRule.assertBuildStatusSuccess(JenkinsRule.java:1472)
	at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinitionTest.lightweight_brach_parametrised(CpsScmFlowDefinitionTest.java:222)

Would just be nice to strengthen the test to prove that it was really running from the correct branch, and not merely that the build passed.

Copy link
Contributor Author

@MartinKosicky MartinKosicky Dec 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm but on 4th august you wrote that :

jglick on Aug 4
Probably a more straightforward way to assert this. Or just delete entirely—the fact that the build ran at all (when the repo only contains the master2 branch) is proof enough that the fix worked, right?

I was parsing the log output before, or did i misunderstood the conversation before...

Copy link
Member

Choose a reason for hiding this comment

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

Sigh, this is what happens when months go by between review comments. Fine enough as it is.

}

@Issue("JENKINS-59425")
@Test public void missingFile() throws Exception {
sampleRepo.init();
Expand Down