Skip to content

Commit ca852cc

Browse files
committed
refactor: Move inversePrecedence tests to separate test class
Extract lockInverseOrderWithLabel and lockInverseOrderMixedDifferentJobs into LockStepInversePrecedenceTest to keep LockStepTest smaller and avoid CI timeouts.
1 parent 0b67e95 commit ca852cc

2 files changed

Lines changed: 168 additions & 149 deletions

File tree

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
package org.jenkins.plugins.lockableresources;
2+
3+
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
4+
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
5+
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
6+
import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
7+
import org.junit.jupiter.api.Test;
8+
import org.jvnet.hudson.test.Issue;
9+
import org.jvnet.hudson.test.JenkinsRule;
10+
import org.jvnet.hudson.test.junit.jupiter.WithJenkins;
11+
12+
/**
13+
* Tests for inversePrecedence queue ordering (issues #861 and #864).
14+
*
15+
* <p>Extracted from {@link LockStepTest} to keep test classes small and avoid CI timeouts.
16+
*/
17+
@WithJenkins
18+
class LockStepInversePrecedenceTest extends LockStepTestBase {
19+
20+
/**
21+
* Verify that inversePrecedence=true grants the lock to the newest build
22+
* when locking by <b>label</b> (not named resource).
23+
*
24+
* <pre>
25+
* start time | build | label | inversePrecedence
26+
* -----------|-------|--------|-------------------
27+
* 00:01 | b1 | label1 | true (acquires)
28+
* 00:02 | b2 | label1 | true (waits)
29+
* 00:03 | b3 | label1 | true (waits)
30+
*
31+
* expected lock order: b1 -> b3 -> b2
32+
* </pre>
33+
*/
34+
@Test
35+
@Issue({"JENKINS-40787", "GITHUB-861"})
36+
void lockInverseOrderWithLabel(JenkinsRule j) throws Exception {
37+
LockableResourcesManager.get().createResourceWithLabel("resource1", "label1");
38+
WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
39+
p.setDefinition(new CpsFlowDefinition("""
40+
lock(label: 'label1', inversePrecedence: true) {
41+
semaphore 'wait-inside'
42+
}
43+
echo 'Finish'""", true));
44+
45+
WorkflowRun b1 = p.scheduleBuild2(0).waitForStart();
46+
SemaphoreStep.waitForStart("wait-inside/1", b1);
47+
48+
WorkflowRun b2 = p.scheduleBuild2(0).waitForStart();
49+
j.waitForMessage("[Label: label1] is locked by build " + b1.getFullDisplayName(), b2);
50+
isPaused(b2, 1, 1);
51+
52+
WorkflowRun b3 = p.scheduleBuild2(0).waitForStart();
53+
j.waitForMessage("[Label: label1] is locked by build " + b1.getFullDisplayName(), b3);
54+
isPaused(b3, 1, 1);
55+
56+
// Release b1 — b3 (newest) must acquire before b2
57+
SemaphoreStep.success("wait-inside/1", null);
58+
j.waitForMessage("Lock released on resource", b1);
59+
j.assertBuildStatusSuccess(j.waitForCompletion(b1));
60+
61+
SemaphoreStep.waitForStart("wait-inside/2", b3);
62+
j.waitForMessage("Trying to acquire lock on [Label: label1]", b3);
63+
64+
SemaphoreStep.success("wait-inside/2", null);
65+
j.waitForMessage("Lock released on resource", b3);
66+
j.assertBuildStatusSuccess(j.waitForCompletion(b3));
67+
68+
SemaphoreStep.waitForStart("wait-inside/3", b2);
69+
j.waitForMessage("Trying to acquire lock on [Label: label1]", b2);
70+
71+
SemaphoreStep.success("wait-inside/3", null);
72+
j.assertBuildStatusSuccess(j.waitForCompletion(b2));
73+
}
74+
75+
/**
76+
* Verify that each waiting job's own {@code inversePrecedence} flag controls
77+
* queue ordering, not the releasing job's flag. Uses <b>separate</b> pipeline
78+
* jobs to match the original report.
79+
*
80+
* <pre>
81+
* start time | job | resource | inversePrecedence
82+
* -----------|------|-----------|-------------------
83+
* 00:01 | pA#1 | resource1 | true (acquires)
84+
* 00:02 | pB#1 | resource1 | false (waits — FIFO)
85+
* 00:03 | pA#2 | resource1 | true (waits — inversePrecedence, front)
86+
* 00:04 | pB#2 | resource1 | false (waits — FIFO, behind pB#1)
87+
*
88+
* expected lock order: pA#1 -> pA#2 -> pB#1 -> pB#2
89+
* </pre>
90+
*/
91+
@Test
92+
@Issue({"JENKINS-41070", "GITHUB-864"})
93+
void lockInverseOrderMixedDifferentJobs(JenkinsRule j) throws Exception {
94+
LockableResourcesManager.get().createResourceWithLabel("resource1", "label1");
95+
96+
// Job A — inversePrecedence = true
97+
WorkflowJob pA = j.jenkins.createProject(WorkflowJob.class, "pA");
98+
pA.setDefinition(new CpsFlowDefinition("""
99+
lock(resource: 'resource1', inversePrecedence: true) {
100+
echo 'locked-pA'
101+
semaphore 'wait-inside'
102+
}
103+
echo 'Finish'""", true));
104+
105+
// Job B — inversePrecedence = false
106+
WorkflowJob pB = j.jenkins.createProject(WorkflowJob.class, "pB");
107+
pB.setDefinition(new CpsFlowDefinition("""
108+
lock(resource: 'resource1', inversePrecedence: false) {
109+
echo 'locked-pB'
110+
semaphore 'wait-inside'
111+
}
112+
echo 'Finish'""", true));
113+
114+
// pA#1 acquires the lock
115+
WorkflowRun a1 = pA.scheduleBuild2(0).waitForStart();
116+
SemaphoreStep.waitForStart("wait-inside/1", a1);
117+
j.assertLogContains("locked-pA", a1);
118+
119+
// pB#1 waits (inversePrecedence=false → back of queue)
120+
WorkflowRun b1 = pB.scheduleBuild2(0).waitForStart();
121+
j.waitForMessage("[resource1] is locked by build " + a1.getFullDisplayName(), b1);
122+
123+
// pA#2 waits (inversePrecedence=true → front of queue)
124+
WorkflowRun a2 = pA.scheduleBuild2(0).waitForStart();
125+
j.waitForMessage("[resource1] is locked by build " + a1.getFullDisplayName(), a2);
126+
127+
// pB#2 waits (inversePrecedence=false → back of queue, behind pB#1)
128+
WorkflowRun b2 = pB.scheduleBuild2(0).waitForStart();
129+
j.waitForMessage("[resource1] is locked by build " + a1.getFullDisplayName(), b2);
130+
131+
// Verify only a1 has the lock so far
132+
j.assertLogNotContains("locked-pA", a2);
133+
j.assertLogNotContains("locked-pB", b1);
134+
j.assertLogNotContains("locked-pB", b2);
135+
136+
// Release pA#1 — pA#2 (inversePrecedence=true) must acquire next
137+
SemaphoreStep.success("wait-inside/1", null);
138+
j.waitForMessage("Lock released on resource", a1);
139+
140+
SemaphoreStep.waitForStart("wait-inside/2", a2);
141+
j.assertLogContains("locked-pA", a2);
142+
j.assertLogNotContains("locked-pB", b1);
143+
j.assertLogNotContains("locked-pB", b2);
144+
145+
// Release pA#2 — pB#1 (FIFO among false) must acquire next
146+
SemaphoreStep.success("wait-inside/2", null);
147+
j.waitForMessage("Lock released on resource", a2);
148+
149+
SemaphoreStep.waitForStart("wait-inside/3", b1);
150+
j.assertLogContains("locked-pB", b1);
151+
j.assertLogNotContains("locked-pB", b2);
152+
153+
// Release pB#1 — pB#2 gets the lock last
154+
SemaphoreStep.success("wait-inside/3", null);
155+
j.waitForMessage("Lock released on resource", b1);
156+
157+
SemaphoreStep.waitForStart("wait-inside/4", b2);
158+
j.assertLogContains("locked-pB", b2);
159+
160+
// Release pB#2 and verify all succeed
161+
SemaphoreStep.success("wait-inside/4", null);
162+
163+
j.assertBuildStatusSuccess(j.waitForCompletion(a1));
164+
j.assertBuildStatusSuccess(j.waitForCompletion(a2));
165+
j.assertBuildStatusSuccess(j.waitForCompletion(b1));
166+
j.assertBuildStatusSuccess(j.waitForCompletion(b2));
167+
}
168+
}

src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java

Lines changed: 0 additions & 149 deletions
Original file line numberDiff line numberDiff line change
@@ -557,155 +557,6 @@ void lockInverseOrder2(JenkinsRule j) throws Exception {
557557
j.assertBuildStatusSuccess(j.waitForCompletion(b6));
558558
}
559559

560-
/**
561-
* Verify that inversePrecedence=true grants the lock to the newest build
562-
* when locking by <b>label</b> (not named resource).
563-
*
564-
* <pre>
565-
* start time | build | label | inversePrecedence
566-
* -----------|-------|--------|-------------------
567-
* 00:01 | b1 | label1 | true (acquires)
568-
* 00:02 | b2 | label1 | true (waits)
569-
* 00:03 | b3 | label1 | true (waits)
570-
*
571-
* expected lock order: b1 -> b3 -> b2
572-
* </pre>
573-
*/
574-
@Test
575-
@Issue({"JENKINS-40787", "GITHUB-861"})
576-
void lockInverseOrderWithLabel(JenkinsRule j) throws Exception {
577-
LockableResourcesManager.get().createResourceWithLabel("resource1", "label1");
578-
WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
579-
p.setDefinition(new CpsFlowDefinition("""
580-
lock(label: 'label1', inversePrecedence: true) {
581-
semaphore 'wait-inside'
582-
}
583-
echo 'Finish'""", true));
584-
585-
WorkflowRun b1 = p.scheduleBuild2(0).waitForStart();
586-
SemaphoreStep.waitForStart("wait-inside/1", b1);
587-
588-
WorkflowRun b2 = p.scheduleBuild2(0).waitForStart();
589-
j.waitForMessage("[Label: label1] is locked by build " + b1.getFullDisplayName(), b2);
590-
isPaused(b2, 1, 1);
591-
592-
WorkflowRun b3 = p.scheduleBuild2(0).waitForStart();
593-
j.waitForMessage("[Label: label1] is locked by build " + b1.getFullDisplayName(), b3);
594-
isPaused(b3, 1, 1);
595-
596-
// Release b1 — b3 (newest) must acquire before b2
597-
SemaphoreStep.success("wait-inside/1", null);
598-
j.waitForMessage("Lock released on resource", b1);
599-
j.assertBuildStatusSuccess(j.waitForCompletion(b1));
600-
601-
SemaphoreStep.waitForStart("wait-inside/2", b3);
602-
j.waitForMessage("Trying to acquire lock on [Label: label1]", b3);
603-
604-
SemaphoreStep.success("wait-inside/2", null);
605-
j.waitForMessage("Lock released on resource", b3);
606-
j.assertBuildStatusSuccess(j.waitForCompletion(b3));
607-
608-
SemaphoreStep.waitForStart("wait-inside/3", b2);
609-
j.waitForMessage("Trying to acquire lock on [Label: label1]", b2);
610-
611-
SemaphoreStep.success("wait-inside/3", null);
612-
j.assertBuildStatusSuccess(j.waitForCompletion(b2));
613-
}
614-
615-
/**
616-
* Verify that each waiting job's own {@code inversePrecedence} flag controls
617-
* queue ordering, not the releasing job's flag. Uses <b>separate</b> pipeline
618-
* jobs to match the original report.
619-
*
620-
* <pre>
621-
* start time | job | resource | inversePrecedence
622-
* -----------|------|-----------|-------------------
623-
* 00:01 | pA#1 | resource1 | true (acquires)
624-
* 00:02 | pB#1 | resource1 | false (waits — FIFO)
625-
* 00:03 | pA#2 | resource1 | true (waits — inversePrecedence, front)
626-
* 00:04 | pB#2 | resource1 | false (waits — FIFO, behind pB#1)
627-
*
628-
* expected lock order: pA#1 -> pA#2 -> pB#1 -> pB#2
629-
* </pre>
630-
*/
631-
@Test
632-
@Issue({"JENKINS-41070", "GITHUB-864"})
633-
void lockInverseOrderMixedDifferentJobs(JenkinsRule j) throws Exception {
634-
LockableResourcesManager.get().createResourceWithLabel("resource1", "label1");
635-
636-
// Job A — inversePrecedence = true
637-
WorkflowJob pA = j.jenkins.createProject(WorkflowJob.class, "pA");
638-
pA.setDefinition(new CpsFlowDefinition("""
639-
lock(resource: 'resource1', inversePrecedence: true) {
640-
echo 'locked-pA'
641-
semaphore 'wait-inside'
642-
}
643-
echo 'Finish'""", true));
644-
645-
// Job B — inversePrecedence = false
646-
WorkflowJob pB = j.jenkins.createProject(WorkflowJob.class, "pB");
647-
pB.setDefinition(new CpsFlowDefinition("""
648-
lock(resource: 'resource1', inversePrecedence: false) {
649-
echo 'locked-pB'
650-
semaphore 'wait-inside'
651-
}
652-
echo 'Finish'""", true));
653-
654-
// pA#1 acquires the lock
655-
WorkflowRun a1 = pA.scheduleBuild2(0).waitForStart();
656-
SemaphoreStep.waitForStart("wait-inside/1", a1);
657-
j.assertLogContains("locked-pA", a1);
658-
659-
// pB#1 waits (inversePrecedence=false → back of queue)
660-
WorkflowRun b1 = pB.scheduleBuild2(0).waitForStart();
661-
j.waitForMessage("[resource1] is locked by build " + a1.getFullDisplayName(), b1);
662-
663-
// pA#2 waits (inversePrecedence=true → front of queue)
664-
WorkflowRun a2 = pA.scheduleBuild2(0).waitForStart();
665-
j.waitForMessage("[resource1] is locked by build " + a1.getFullDisplayName(), a2);
666-
667-
// pB#2 waits (inversePrecedence=false → back of queue, behind pB#1)
668-
WorkflowRun b2 = pB.scheduleBuild2(0).waitForStart();
669-
j.waitForMessage("[resource1] is locked by build " + a1.getFullDisplayName(), b2);
670-
671-
// Verify only a1 has the lock so far
672-
j.assertLogNotContains("locked-pA", a2);
673-
j.assertLogNotContains("locked-pB", b1);
674-
j.assertLogNotContains("locked-pB", b2);
675-
676-
// Release pA#1 — pA#2 (inversePrecedence=true) must acquire next
677-
SemaphoreStep.success("wait-inside/1", null);
678-
j.waitForMessage("Lock released on resource", a1);
679-
680-
SemaphoreStep.waitForStart("wait-inside/2", a2);
681-
j.assertLogContains("locked-pA", a2);
682-
j.assertLogNotContains("locked-pB", b1);
683-
j.assertLogNotContains("locked-pB", b2);
684-
685-
// Release pA#2 — pB#1 (FIFO among false) must acquire next
686-
SemaphoreStep.success("wait-inside/2", null);
687-
j.waitForMessage("Lock released on resource", a2);
688-
689-
SemaphoreStep.waitForStart("wait-inside/3", b1);
690-
j.assertLogContains("locked-pB", b1);
691-
j.assertLogNotContains("locked-pB", b2);
692-
693-
// Release pB#1 — pB#2 gets the lock last
694-
SemaphoreStep.success("wait-inside/3", null);
695-
j.waitForMessage("Lock released on resource", b1);
696-
697-
SemaphoreStep.waitForStart("wait-inside/4", b2);
698-
j.assertLogContains("locked-pB", b2);
699-
700-
// Release pB#2 and verify all succeed
701-
SemaphoreStep.success("wait-inside/4", null);
702-
703-
j.assertBuildStatusSuccess(j.waitForCompletion(a1));
704-
j.assertBuildStatusSuccess(j.waitForCompletion(a2));
705-
j.assertBuildStatusSuccess(j.waitForCompletion(b1));
706-
j.assertBuildStatusSuccess(j.waitForCompletion(b2));
707-
}
708-
709560
/**
710561
* start time | job | resource | priority
711562
* ------ |--- |--- |---

0 commit comments

Comments
 (0)