From 1b276a4d95a4d6974b02ae6360a6ead7ace4a51b Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Wed, 1 May 2024 16:42:45 -0500 Subject: [PATCH 1/2] Fix non-determinism in nested cause tests For deeplyNestedCauses, quickly triggering a jobs with no quiet period runs the possibility that a job is triggered while it is still in the queue. Triggering and waiting for the jobs to execute synchronously avoids this and also simplifies the code. The broadlyNesstedCauses test suffers from the same problem, but is additionally not structured in the way the original issue describes to reproduce the issue. Using the quiet period argument to scheduleBuild2, we can reliably produce a graph where each build is triggered by the other two jobs. An additional final job outside of the loop cuts off the 1 second quiet period from the last loop's iteration so that the test is never waiting for the quiet period of any jobs even though half of the schedule operations specify one. --- .../src/test/java/hudson/model/CauseTest.java | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/test/src/test/java/hudson/model/CauseTest.java b/test/src/test/java/hudson/model/CauseTest.java index 0d4caf54f61b..2484a03c52ed 100644 --- a/test/src/test/java/hudson/model/CauseTest.java +++ b/test/src/test/java/hudson/model/CauseTest.java @@ -63,9 +63,9 @@ public class CauseTest { FreeStyleProject b = j.createFreeStyleProject("b"); FreeStyleBuild early = null; FreeStyleBuild last = null; - List> futures = new ArrayList<>(); for (int i = 1; i <= 15; i++) { - last = recordFuture(b.scheduleBuild2(0, new Cause.UpstreamCause(recordFuture(a.scheduleBuild2(0, last == null ? null : new Cause.UpstreamCause(last)), futures).get())), futures).get(); + last = j.waitForCompletion(a.scheduleBuild2(0, last == null ? null : new Cause.UpstreamCause(last)).get()); + last = j.waitForCompletion(b.scheduleBuild2(0, new Cause.UpstreamCause(last)).get()); if (i == 5) { early = last; } @@ -74,9 +74,6 @@ public class CauseTest { assertTrue("keeps full history:\n" + buildXml, buildXml.contains("1")); buildXml = new XmlFile(Run.XSTREAM, new File(last.getRootDir(), "build.xml")).asString(); assertFalse("too big:\n" + buildXml, buildXml.contains("1")); - for (QueueTaskFuture future : futures) { - j.assertBuildStatusSuccess(j.waitForCompletion(future.waitForStart())); - } } @Issue("JENKINS-15747") @@ -88,16 +85,16 @@ public class CauseTest { Run last = null; for (int i = 1; i <= 10; i++) { Cause cause = last == null ? null : new Cause.UpstreamCause(last); - QueueTaskFuture next1 = recordFuture(a.scheduleBuild2(0, cause), futures); - recordFuture(a.scheduleBuild2(0, cause), futures); - cause = new Cause.UpstreamCause(next1.get()); - QueueTaskFuture next2 = recordFuture(b.scheduleBuild2(0, cause), futures); - recordFuture(b.scheduleBuild2(0, cause), futures); - cause = new Cause.UpstreamCause(next2.get()); - QueueTaskFuture next3 = recordFuture(c.scheduleBuild2(0, cause), futures); - recordFuture(c.scheduleBuild2(0, cause), futures); - last = next3.get(); + last = j.waitForCompletion(a.scheduleBuild2(0, cause).get()); + recordFuture(b.scheduleBuild2(1, cause), futures); + cause = new Cause.UpstreamCause(last); + last = j.waitForCompletion(b.scheduleBuild2(0, cause).get()); + recordFuture(c.scheduleBuild2(1, cause), futures); + cause = new Cause.UpstreamCause(last); + last = j.waitForCompletion(c.scheduleBuild2(0, cause).get()); + recordFuture(a.scheduleBuild2(1, cause), futures); } + last = j.waitForCompletion(a.scheduleBuild2(0, new Cause.UpstreamCause(last)).get()); int count = new XmlFile(Run.XSTREAM, new File(last.getRootDir(), "build.xml")).asString().split(Pattern.quote(" 100); //j.interactiveBreak(); From 625abf4cfeaaa35a97503c3cd2f9984374b7c2bd Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Mon, 6 May 2024 17:01:05 -0500 Subject: [PATCH 2/2] Re-work nested cause trimming algorithm --- core/src/main/java/hudson/model/Cause.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/hudson/model/Cause.java b/core/src/main/java/hudson/model/Cause.java index 6990dd2b39e0..668587ffc8fc 100644 --- a/core/src/main/java/hudson/model/Cause.java +++ b/core/src/main/java/hudson/model/Cause.java @@ -182,6 +182,10 @@ public UpstreamCause(Run up) { upstreamCauses = new ArrayList<>(); Set traversed = new HashSet<>(); for (Cause c : up.getCauses()) { + if (traversed.size() >= MAX_LEAF) { + upstreamCauses.add(new DeeplyNestedUpstreamCause()); + break; + } upstreamCauses.add(trim(c, MAX_DEPTH, traversed)); } } @@ -239,14 +243,16 @@ public int hashCode() { } UpstreamCause uc = (UpstreamCause) c; List cs = new ArrayList<>(); - if (depth > 0) { - if (traversed.add(uc.upstreamUrl + uc.upstreamBuild)) { - for (Cause c2 : uc.upstreamCauses) { - cs.add(trim(c2, depth - 1, traversed)); + if (traversed.add(uc.upstreamUrl + uc.upstreamBuild)) { + for (Cause c2 : uc.upstreamCauses) { + if (depth <= 0 || traversed.size() >= MAX_LEAF) { + cs.add(new DeeplyNestedUpstreamCause()); + break; } + cs.add(trim(c2, depth - 1, traversed)); } - } else if (traversed.size() < MAX_LEAF) { - cs.add(new DeeplyNestedUpstreamCause()); + } else { + traversed.add(uc.upstreamUrl + uc.upstreamBuild + '#' + traversed.size()); } return new UpstreamCause(uc.upstreamProject, uc.upstreamBuild, uc.upstreamUrl, cs); }