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.

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

Use non-deprecated lazily evaluated iterateEnclosingBlocks #133

Merged
merged 2 commits into from
Aug 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import org.jenkinsci.plugins.workflow.actions.ThreadNameAction;
import org.jenkinsci.plugins.workflow.flow.FlowExecution;
import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner;
import org.jenkinsci.plugins.workflow.graph.BlockStartNode;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.graphanalysis.FlowScanningUtils;
import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
Expand Down Expand Up @@ -638,10 +639,8 @@ private String computeEnclosingLabel(FlowNode executorStepNode, List<FlowNode> h
// See if this step is inside our node {} block, and track the associated label.
boolean match = false;
String enclosingLabel = null;
Iterator<FlowNode> it = FlowScanningUtils.fetchEnclosingBlocks(runningNode);
int count = 0;
while (it.hasNext()) {
FlowNode n = it.next();
for (FlowNode n : runningNode.iterateEnclosingBlocks()) {
if (enclosingLabel == null) {
ThreadNameAction tna = n.getPersistentAction(ThreadNameAction.class);
if (tna != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import javax.annotation.Nullable;

import jenkins.model.Jenkins;
Expand Down Expand Up @@ -777,7 +778,7 @@ private List<WorkspaceAction> getWorkspaceActions(WorkflowRun workflowRun) {
* @return Map containing node names as key and the log text for all steps executed on that very node as value
* @throws java.io.IOException Will be thrown in case there something went wrong while reading the log
*/
private Map<String, StringWriter> mapNodeNameToLogText(WorkflowRun workflowRun) throws java.io.IOException{
private Map<String, String> mapNodeNameToLogText(WorkflowRun workflowRun) throws java.io.IOException{
FlowGraphWalker walker = new FlowGraphWalker(workflowRun.getExecution());
Map<String, StringWriter> workspaceActionToLogText = new HashMap<>();
for (FlowNode n : walker) {
Expand All @@ -804,7 +805,8 @@ private Map<String, StringWriter> mapNodeNameToLogText(WorkflowRun workflowRun)
}
}
}
return workspaceActionToLogText;
return workspaceActionToLogText.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().toString()));
}


Expand All @@ -826,20 +828,18 @@ private Map<String, StringWriter> mapNodeNameToLogText(WorkflowRun workflowRun)
"}\n" +
"", true));
WorkflowRun run1 = r.buildAndAssertSuccess(p);
Map<String, StringWriter> nodeMapping1 = mapNodeNameToLogText(run1);
Map<String, String> nodeMapping1 = mapNodeNameToLogText(run1);

WorkflowRun run2 = r.buildAndAssertSuccess(p);
Map<String, StringWriter> nodeMapping2 = mapNodeNameToLogText(run2);
Map<String, String> nodeMapping2 = mapNodeNameToLogText(run2);

for (String nodeName: nodeMapping1.keySet()) {
assertEquals(nodeMapping1.get(nodeName).toString(), nodeMapping2.get(nodeName).toString());
}
assertEquals(nodeMapping1, nodeMapping2);
});
}

/**
* Please note that any change to the node allocation algorithm may need an increase or decrease
* of the number of slaves in order to get a pass
* of the number of agents in order to get a pass
*/
@Issue("JENKINS-36547")
@Test public void reuseNodesWithSameLabelsInDifferentReorderedStages() {
Expand All @@ -863,9 +863,9 @@ private Map<String, StringWriter> mapNodeNameToLogText(WorkflowRun workflowRun)
"}\n" +
"", true));
WorkflowRun run1 = r.buildAndAssertSuccess(p);
Map<String, StringWriter> nodeMapping1 = mapNodeNameToLogText(run1);
Map<String, String> nodeMapping1 = mapNodeNameToLogText(run1);
// if nodeMapping contains only one entry this test actually will not test anything reasonable
// possibly the number of dumb slaves has to be adjusted in that case
// possibly the number of agents has to be adjusted in that case
assertEquals(nodeMapping1.size(), 2);

p.setDefinition(new CpsFlowDefinition("" +
Expand All @@ -881,18 +881,16 @@ private Map<String, StringWriter> mapNodeNameToLogText(WorkflowRun workflowRun)
"}\n" +
"", true));
WorkflowRun run2 = r.buildAndAssertSuccess(p);
Map<String, StringWriter> nodeMapping2 = mapNodeNameToLogText(run2);
Map<String, String> nodeMapping2 = mapNodeNameToLogText(run2);

for (String nodeName: nodeMapping1.keySet()) {
assertEquals(nodeMapping1.get(nodeName).toString(), nodeMapping2.get(nodeName).toString());
}
assertEquals(nodeMapping1, nodeMapping2);
});
}

/**
* Ensure node reuse works from within parallel block without using stages
* Please note that any change to the node allocation algorithm may need an increase or decrease
* of the number of slaves in order to get a pass
* of the number of agents in order to get a pass
*/
@Issue("JENKINS-36547")
@Test public void reuseNodesWithSameLabelsInParallelStages() {
Expand Down Expand Up @@ -924,10 +922,10 @@ private Map<String, StringWriter> mapNodeNameToLogText(WorkflowRun workflowRun)
"})\n" +
"", true));
WorkflowRun run1 = r.buildAndAssertSuccess(p);
Map<String, StringWriter> nodeMapping1 = mapNodeNameToLogText(run1);
Map<String, String> nodeMapping1 = mapNodeNameToLogText(run1);

// if nodeMapping contains only one entry this test actually will not test anything reasonable
// possibly the number of dumb slaves has to be adjusted in that case
// possibly the number of agents has to be adjusted in that case
assertEquals(nodeMapping1.size(), 2);

// 2: update script to force reversed order for node blocks; shall still pick the same nodes
Expand All @@ -949,18 +947,15 @@ private Map<String, StringWriter> mapNodeNameToLogText(WorkflowRun workflowRun)
"})\n" +
"", true));
WorkflowRun run2 = r.buildAndAssertSuccess(p);
Map<String, StringWriter> nodeMapping2 = mapNodeNameToLogText(run2);

for (String nodeName: nodeMapping1.keySet()) {
assertEquals(nodeMapping1.get(nodeName).toString(), nodeMapping2.get(nodeName).toString());
}
Map<String, String> nodeMapping2 = mapNodeNameToLogText(run2);
assertEquals(nodeMapping1, nodeMapping2);
});
}

/**
* Ensure node reuse works from within parallel blocks which use the same stage names
* Please note that any change to the node allocation algorithm may need an increase or decrease
* of the number of slaves in order to get a pass
* of the number of agents in order to get a pass
*/
@Issue("JENKINS-36547")
@Test public void reuseNodesWithSameLabelsInStagesWrappedInsideParallelStages() {
Expand Down Expand Up @@ -993,10 +988,10 @@ private Map<String, StringWriter> mapNodeNameToLogText(WorkflowRun workflowRun)
"})\n" +
"", true));
WorkflowRun run1 = r.buildAndAssertSuccess(p);
Map<String, StringWriter> nodeMapping1 = mapNodeNameToLogText(run1);
Map<String, String> nodeMapping1 = mapNodeNameToLogText(run1);

// if nodeMapping contains only one entry this test actually will not test anything reasonable
// possibly the number of dumb slaves has to be adjusted in that case
// possibly the number of agents has to be adjusted in that case
assertEquals(nodeMapping1.size(), 2);

// update script to force reversed order for node blocks; shall still pick the same nodes
Expand All @@ -1023,11 +1018,8 @@ private Map<String, StringWriter> mapNodeNameToLogText(WorkflowRun workflowRun)
"", true));

WorkflowRun run2 = r.buildAndAssertSuccess(p);
Map<String, StringWriter> nodeMapping2 = mapNodeNameToLogText(run2);

for (String nodeName: nodeMapping1.keySet()) {
assertEquals(nodeMapping1.get(nodeName).toString(), nodeMapping2.get(nodeName).toString());
}
Map<String, String> nodeMapping2 = mapNodeNameToLogText(run2);
assertEquals(nodeMapping1, nodeMapping2);
});
}

Expand All @@ -1042,7 +1034,7 @@ private Map<String, StringWriter> mapNodeNameToLogText(WorkflowRun workflowRun)
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "demo");
p.setDefinition(new CpsFlowDefinition("for (int i = 0; i < 20; ++i) {node('foo') {echo \"ran node block ${i}\"}}", true));
WorkflowRun run = r.buildAndAssertSuccess(p);
Map<String, StringWriter> nodeMapping = mapNodeNameToLogText(run);
Map<String, String> nodeMapping = mapNodeNameToLogText(run);

// if the node was reused every time we'll only have one node mapping entry
assertEquals(nodeMapping.size(), 1);
Expand Down