From 30a73e53013b44cec88f15f230e62c1d5ef5553d Mon Sep 17 00:00:00 2001 From: Artem Nikitin Date: Mon, 18 Jan 2021 12:25:30 +0300 Subject: [PATCH] Fix conditional step issue If there is the only one macro step in the conditional-step builders with more than one build steps in the macro, it translated into the SingleConditionalBuilder Jenkins XML tag, instead of ConditionalBuilder. It leads to an invalid Jenkins configuration in that case. Change-Id: Ib9866d93e4ee9fa2823dfb1b5d15870ff56ab9f8 --- jenkins_jobs/modules/builders.py | 24 +++++++------- ...itional-step_inner-macro-expansion-002.xml | 31 +++++++++++++++++++ ...tional-step_inner-macro-expansion-002.yaml | 19 ++++++++++++ 3 files changed, 63 insertions(+), 11 deletions(-) create mode 100644 tests/macros/fixtures/builder/conditional-step_inner-macro-expansion-002.xml create mode 100644 tests/macros/fixtures/builder/conditional-step_inner-macro-expansion-002.yaml diff --git a/jenkins_jobs/modules/builders.py b/jenkins_jobs/modules/builders.py index 58d798e14..b5a1a7fc7 100644 --- a/jenkins_jobs/modules/builders.py +++ b/jenkins_jobs/modules/builders.py @@ -1789,13 +1789,6 @@ def conditional_step(registry, xml_parent, data): ) build_condition(condition, conditions_container_tag, "condition") - def build_step(parent, step): - for edited_node in create_builders(registry, step): - if not has_multiple_steps: - edited_node.set("class", edited_node.tag) - edited_node.tag = "buildStep" - parent.append(edited_node) - cond_builder_tag = ( "org.jenkinsci.plugins.conditionalbuildstep." "singlestep.SingleConditionalBuilder" @@ -1803,8 +1796,14 @@ def conditional_step(registry, xml_parent, data): cond_builders_tag = ( "org.jenkinsci.plugins.conditionalbuildstep." "ConditionalBuilder" ) - steps = data["steps"] - has_multiple_steps = len(steps) > 1 + # A builder could be a macro, so it's required to create builders at first + # to set `has_multiple_steps` flag correctly. + edited_nodes = [ + edited_node + for step in data["steps"] + for edited_node in create_builders(registry, step) + ] + has_multiple_steps = len(edited_nodes) > 1 if has_multiple_steps: root_tag = XML.SubElement(xml_parent, cond_builders_tag) @@ -1827,8 +1826,11 @@ def conditional_step(registry, xml_parent, data): } evaluation_class = evaluation_classes[data.get("on-evaluation-failure", "fail")] XML.SubElement(root_tag, "runner").set("class", evaluation_class) - for step in steps: - build_step(steps_parent, step) + for edited_node in edited_nodes: + if not has_multiple_steps: + edited_node.set("class", edited_node.tag) + edited_node.tag = "buildStep" + steps_parent.append(edited_node) def maven_builder(registry, xml_parent, data): diff --git a/tests/macros/fixtures/builder/conditional-step_inner-macro-expansion-002.xml b/tests/macros/fixtures/builder/conditional-step_inner-macro-expansion-002.xml new file mode 100644 index 000000000..9e8416cfd --- /dev/null +++ b/tests/macros/fixtures/builder/conditional-step_inner-macro-expansion-002.xml @@ -0,0 +1,31 @@ + + + + <!-- Managed by Jenkins Job Builder --> + false + false + false + false + true + + + + + + + first + + + second + + + + filename + + + + + + + + diff --git a/tests/macros/fixtures/builder/conditional-step_inner-macro-expansion-002.yaml b/tests/macros/fixtures/builder/conditional-step_inner-macro-expansion-002.yaml new file mode 100644 index 000000000..52a49ed63 --- /dev/null +++ b/tests/macros/fixtures/builder/conditional-step_inner-macro-expansion-002.yaml @@ -0,0 +1,19 @@ +- builder: + name: Macro + builders: + - shell: "first" + - shell: "second" + +- job: + name: Job + builders: + - conditional-step: + condition-kind: file-exists + condition-filename: 'filename' + steps: + - Macro + +- project: + name: meow + jobs: + - Job