Skip to content

Commit 7ab4e05

Browse files
Swopxvzf
authored andcommitted
fix: don't return stacked_pull_requests if the request status is not
1 parent 5ecdd56 commit 7ab4e05

File tree

2 files changed

+31
-25
lines changed

2 files changed

+31
-25
lines changed

e2e/api_test.go

+26-24
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,9 @@ var _ = Describe("API", Ordered, func() {
112112
})
113113
It("should return an non-empty list of requests", func() {
114114
leaseRequestsPayloadsJSON := buildExpectedRequestsContextPayloads(providerStateOpts.Known, map[string][]int{
115-
"xxx-1": rangeInt(1),
116-
"xxx-2": rangeInt(2),
117-
"xxx-3": rangeInt(3),
115+
"xxx-1": {},
116+
"xxx-2": {},
117+
"xxx-3": {},
118118
"xxx-4": rangeInt(4),
119119
})
120120
acquiredLeaseRequestPayloadJSON := buildExpectedRequestContextPayload(providerStateOpts.Acquired, rangeInt(4))
@@ -181,9 +181,9 @@ var _ = Describe("API", Ordered, func() {
181181
})
182182
It("should return an non-empty list of requests", func() {
183183
leaseRequestsPayloadsJSON := buildExpectedRequestsContextPayloads(providerStateOpts.Known, map[string][]int{
184-
"xxx-1": rangeInt(1),
185-
"xxx-2": rangeInt(2),
186-
"xxx-3": rangeInt(3),
184+
"xxx-1": {},
185+
"xxx-2": {},
186+
"xxx-3": {},
187187
"xxx-4": rangeInt(4),
188188
})
189189
acquiredLeaseRequestPayloadJSON := buildExpectedRequestContextPayload(providerStateOpts.Acquired, rangeInt(4))
@@ -309,7 +309,7 @@ var _ = Describe("API", Ordered, func() {
309309
HeadRef: headRef,
310310
Priority: priority,
311311
Status: pointer.String(lease.StatusPending),
312-
}, rangeInt(configHelper.DefaultConfigRepoExpectedRequestCount-1))
312+
}, []int{})
313313
Expect(acquireRespBody).To(MatchJSON(expectedPayload))
314314
})
315315
})
@@ -428,7 +428,7 @@ var _ = Describe("API", Ordered, func() {
428428
HeadRef: headRef,
429429
Priority: priority,
430430
Status: pointer.String(lease.StatusPending),
431-
}, rangeInt(1))
431+
}, []int{})
432432
Expect(acquireRespBody).To(MatchJSON(expectedPayload))
433433
})
434434
})
@@ -515,7 +515,7 @@ var _ = Describe("API", Ordered, func() {
515515
HeadRef: headRef,
516516
Priority: priority,
517517
Status: pointer.String(lease.StatusCompleted),
518-
}, rangeInt(2))
518+
}, []int{})
519519
Expect(releaseRespBody).To(MatchJSON(expectedPayload))
520520
})
521521
})
@@ -531,7 +531,7 @@ var _ = Describe("API", Ordered, func() {
531531
HeadRef: headRef,
532532
Priority: priority,
533533
Status: pointer.String(lease.StatusFailure),
534-
}, rangeInt(1))
534+
}, []int{})
535535
Expect(releaseRespBody).To(MatchJSON(expectedPayload))
536536
})
537537
})
@@ -555,7 +555,7 @@ var _ = Describe("API", Ordered, func() {
555555
HeadRef: ref(1),
556556
Priority: 1,
557557
Status: pointer.String(lease.StatusPending),
558-
}, rangeInt(1))))
558+
}, []int{})))
559559
})
560560
By("test acquire, request 2 => should be pending", func() {
561561
resp, body := apiCall(srv, acquireReq(owner, repo, baseRef, "xxx-2", 2))
@@ -565,7 +565,7 @@ var _ = Describe("API", Ordered, func() {
565565
HeadRef: ref(2),
566566
Priority: 2,
567567
Status: pointer.String(lease.StatusPending),
568-
}, rangeInt(2))))
568+
}, []int{})))
569569
})
570570
By("sleeping for stabilize duration", func() {
571571
currentTime := now
@@ -580,7 +580,7 @@ var _ = Describe("API", Ordered, func() {
580580
HeadRef: ref(1),
581581
Priority: 1,
582582
Status: pointer.String(lease.StatusPending),
583-
}, rangeInt(1))))
583+
}, []int{})))
584584
})
585585
By("test acquire, request 2 => should be acquired", func() {
586586
resp, body := apiCall(srv, acquireReq(owner, repo, baseRef, "xxx-2", 2))
@@ -600,7 +600,7 @@ var _ = Describe("API", Ordered, func() {
600600
HeadRef: ref(2),
601601
Priority: 2,
602602
Status: pointer.String(lease.StatusCompleted),
603-
}, rangeInt(2))))
603+
}, []int{})))
604604
})
605605
By("test acquire, request 1 => should be completed", func() {
606606
resp, body := apiCall(srv, acquireReq(owner, repo, baseRef, "xxx-1", 1))
@@ -629,7 +629,7 @@ var _ = Describe("API", Ordered, func() {
629629
HeadRef: ref(1),
630630
Priority: 1,
631631
Status: pointer.String(lease.StatusPending),
632-
}, rangeInt(1))))
632+
}, []int{})))
633633
})
634634
By("test acquire, request 2 => should be pending", func() {
635635
resp, body := apiCall(srv, acquireReq(owner, repo, baseRef, "xxx-2", 2))
@@ -639,7 +639,7 @@ var _ = Describe("API", Ordered, func() {
639639
HeadRef: ref(2),
640640
Priority: 2,
641641
Status: pointer.String(lease.StatusPending),
642-
}, rangeInt(2))))
642+
}, []int{})))
643643
})
644644
By("sleeping for stabilize duration", func() {
645645
currentTime := now
@@ -654,7 +654,7 @@ var _ = Describe("API", Ordered, func() {
654654
HeadRef: ref(1),
655655
Priority: 1,
656656
Status: pointer.String(lease.StatusPending),
657-
}, rangeInt(1))))
657+
}, []int{})))
658658
})
659659
By("test acquire, request 2 => should be acquired", func() {
660660
resp, body := apiCall(srv, acquireReq(owner, repo, baseRef, "xxx-2", 2))
@@ -674,7 +674,7 @@ var _ = Describe("API", Ordered, func() {
674674
HeadRef: ref(2),
675675
Priority: 2,
676676
Status: pointer.String(lease.StatusFailure),
677-
}, rangeInt(1))))
677+
}, []int{})))
678678
})
679679
By("test acquire, request 1 => should be acquired", func() {
680680
resp, body := apiCall(srv, acquireReq(owner, repo, baseRef, "xxx-1", 1))
@@ -705,7 +705,7 @@ var _ = Describe("API", Ordered, func() {
705705
HeadRef: ref(i),
706706
Priority: i,
707707
Status: pointer.String(lease.StatusPending),
708-
}, rangeInt(i))))
708+
}, []int{})))
709709
})
710710
}
711711
By(fmt.Sprintf("test acquire, request %d => should be acquired", max), func() {
@@ -726,7 +726,7 @@ var _ = Describe("API", Ordered, func() {
726726
HeadRef: ref(max),
727727
Priority: max,
728728
Status: pointer.String(lease.StatusCompleted),
729-
}, rangeInt(max))))
729+
}, []int{})))
730730
})
731731
for i := 1; i <= max-1; i++ {
732732
By(fmt.Sprintf("test acquire, request %d => should be completed", i), func() {
@@ -759,7 +759,7 @@ var _ = Describe("API", Ordered, func() {
759759
HeadRef: ref(i),
760760
Priority: i,
761761
Status: pointer.String(lease.StatusPending),
762-
}, rangeInt(i))))
762+
}, []int{})))
763763
})
764764
}
765765
By(fmt.Sprintf("test acquire, request %d => should be acquired", max), func() {
@@ -780,7 +780,7 @@ var _ = Describe("API", Ordered, func() {
780780
HeadRef: ref(max),
781781
Priority: max,
782782
Status: pointer.String(lease.StatusFailure),
783-
}, rangeInt(max-1))))
783+
}, []int{})))
784784
})
785785
for i := 1; i <= max-2; i++ {
786786
By(fmt.Sprintf("test acquire, request %d => should be pending", i), func() {
@@ -791,7 +791,7 @@ var _ = Describe("API", Ordered, func() {
791791
HeadRef: ref(i),
792792
Priority: i,
793793
Status: pointer.String(lease.StatusPending),
794-
}, rangeInt(i))))
794+
}, []int{})))
795795
})
796796
}
797797
By(fmt.Sprintf("test acquire, request %d => should be acquired", max-1), func() {
@@ -937,7 +937,9 @@ func buildExpectedRequestContextPayload(leaseRequest *lease.Request, expectedSta
937937
"priority": leaseRequest.Priority,
938938
"status": leaseRequest.Status,
939939
},
940-
"stacked_pull_requests": expectedStackedPulls,
940+
}
941+
if len(expectedStackedPulls) > 0 {
942+
raw["stacked_pull_requests"] = expectedStackedPulls
941943
}
942944
b, _ := json.Marshal(raw)
943945

internal/lease/leaseprovider.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ type StackedPullRequest struct {
5959

6060
type RequestContext struct {
6161
Request *Request `json:"request"`
62-
StackedPullRequests []*StackedPullRequest `json:"stacked_pull_requests"`
62+
StackedPullRequests []*StackedPullRequest `json:"stacked_pull_requests,omitempty"`
6363
}
6464

6565
func (lr *Request) UpdateLastSeenAt(t time.Time) {
@@ -612,6 +612,10 @@ func (lp *leaseProviderImpl) BuildRequestContext(ctx context.Context, leaseReque
612612
Request: leaseRequest,
613613
}
614614

615+
if pointer.StringDeref(leaseRequest.Status, StatusPending) != StatusAcquired {
616+
return requestContext, nil
617+
}
618+
615619
stackedPulls, err := lp.computeStackedPullRequests(leaseRequest)
616620
if err != nil {
617621
log.Ctx(ctx).

0 commit comments

Comments
 (0)