-
Notifications
You must be signed in to change notification settings - Fork 98
Make this test compatible with Jenkins 2.307+ #167
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
Conversation
" echo \"My name on master is ${env.NODE_NAME} using labels ${env.NODE_LABELS}\"\n" + | ||
"}\n", | ||
true)); | ||
r.assertLogContains("My name on master is master using labels master", r.assertBuildStatusSuccess(p.scheduleBuild2(0))); | ||
r.assertLogContains("My name on master is master using labels " + builtInNodeLabel, r.assertBuildStatusSuccess(p.scheduleBuild2(0))); |
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.
This is awkward; I missed NODE_NAME
being defined in
Line 812 in cffc780
env.put("NODE_NAME", "master"); |
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.
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "workflow-test"); | ||
|
||
p.setDefinition(new CpsFlowDefinition( | ||
"node('master') {\n" + | ||
"node('" + builtInNodeLabel + "') {\n" + |
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.
"node('" + builtInNodeLabel + "') {\n" + | |
"node('built-in || master') {\n" + |
would also work right?
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.
Yes.
https://www.jenkins.io/changelog/#v2.307 / jenkinsci/jenkins#5425