|
6 | 6 | "net/http" |
7 | 7 | "net/http/httptest" |
8 | 8 | "path/filepath" |
| 9 | + "strconv" |
9 | 10 | "strings" |
10 | 11 | "testing" |
11 | 12 | "time" |
@@ -277,3 +278,128 @@ func TestClaims_RenewLeaseRenewsClaim(t *testing.T) { |
277 | 278 | t.Fatalf("claim not renewed with lease: before=%v after=%+v", first.ExpiresAt, after) |
278 | 279 | } |
279 | 280 | } |
| 281 | + |
| 282 | +// A lease that ends while the holder's own open PR covers the issue keeps the |
| 283 | +// claim as a PR-held claim for the same principal; once that PR stops |
| 284 | +// covering the issue the sweep releases it (hivecommons/hive#17300). |
| 285 | +func TestClaims_LeaseRevokeKeepsClaimCoveredByHoldersPR(t *testing.T) { |
| 286 | + hub, s, l := claimsHub(t) |
| 287 | + const identity = "c-karibhou#a" |
| 288 | + covering := ghpkg.IssueClaim{Repo: "myorg/repo1", Issue: 1, PRRepo: "myorg/repo1", PRNumber: 5, |
| 289 | + PRURL: "https://github.com/myorg/repo1/pull/5", PRAuthor: "Karibhou", ExternalAuthor: true, Closing: true} |
| 290 | + open := true |
| 291 | + s.deps.IssueClaimed = func(repo string, number int) (ghpkg.IssueClaim, bool) { |
| 292 | + if repo == "myorg/repo1" && number == 1 && open { |
| 293 | + return covering, true |
| 294 | + } |
| 295 | + return ghpkg.IssueClaim{}, false |
| 296 | + } |
| 297 | + if err := hub.recordLease(identity, "t-1", "myorg/repo1", 1, "contributor", 3, time.Now()); err != nil { |
| 298 | + t.Fatal(err) |
| 299 | + } |
| 300 | + if _, err := l.Claim(claims.Request{Repo: "myorg/repo1", Issue: 1, Holder: "karibhou", HolderID: identity, Kind: claims.KindContributor, Session: "a"}); err != nil { |
| 301 | + t.Fatal(err) |
| 302 | + } |
| 303 | + |
| 304 | + hub.revokeLease(identity, "t-1") |
| 305 | + c, ok := l.Lookup("myorg/repo1", 1) |
| 306 | + if !ok || c.HolderID != identity || c.Holder != "karibhou" || !c.PRStarted || c.PRRepo != "myorg/repo1" || c.PRNumber != 5 { |
| 307 | + t.Fatalf("claim after revoke = %+v ok=%v, want PR-held claim by the same contributor", c, ok) |
| 308 | + } |
| 309 | + if hub.sweepPRHeldClaims() != 0 { |
| 310 | + t.Fatal("sweep released a claim whose PR still covers the issue") |
| 311 | + } |
| 312 | + if _, ok := l.Lookup("myorg/repo1", 1); !ok { |
| 313 | + t.Fatal("PR-held claim lost while the PR is open") |
| 314 | + } |
| 315 | + |
| 316 | + open = false |
| 317 | + if n := hub.sweepPRHeldClaims(); n != 1 { |
| 318 | + t.Fatalf("sweep released %d claims once the PR closed, want 1", n) |
| 319 | + } |
| 320 | + if _, ok := l.Lookup("myorg/repo1", 1); ok { |
| 321 | + t.Fatal("PR-held claim survived its PR closing") |
| 322 | + } |
| 323 | +} |
| 324 | + |
| 325 | +// Without a covering PR by the holder — none at all, or one by somebody |
| 326 | +// else — a revoked lease still releases the claim. |
| 327 | +func TestClaims_LeaseRevokeWithoutHoldersPRReleasesClaim(t *testing.T) { |
| 328 | + hub, s, l := claimsHub(t) |
| 329 | + const identity = "c-worker#a" |
| 330 | + s.deps.IssueClaimed = func(repo string, number int) (ghpkg.IssueClaim, bool) { |
| 331 | + if repo == "myorg/repo1" && number == 2 { |
| 332 | + return ghpkg.IssueClaim{Repo: repo, Issue: number, PRNumber: 8, PRAuthor: "someone-else", ExternalAuthor: true}, true |
| 333 | + } |
| 334 | + return ghpkg.IssueClaim{}, false |
| 335 | + } |
| 336 | + for n, task := range map[int]string{1: "t-1", 2: "t-2"} { |
| 337 | + if err := hub.recordLease(identity, task, "myorg/repo1", n, "contributor", 3, time.Now()); err != nil { |
| 338 | + t.Fatal(err) |
| 339 | + } |
| 340 | + if _, err := l.Claim(claims.Request{Repo: "myorg/repo1", Issue: n, Holder: "worker", HolderID: identity, Kind: claims.KindContributor, Session: "a"}); err != nil { |
| 341 | + t.Fatal(err) |
| 342 | + } |
| 343 | + hub.revokeLease(identity, task) |
| 344 | + if c, ok := l.Lookup("myorg/repo1", n); ok { |
| 345 | + t.Fatalf("issue %d: claim survived an abandon with no covering PR by its holder: %+v", n, c) |
| 346 | + } |
| 347 | + } |
| 348 | +} |
| 349 | + |
| 350 | +// The primary #17300 path: the relay opened its PR with its own token and |
| 351 | +// reports task_complete before any scan saw it, so the duplicate-PR ledger is |
| 352 | +// empty at revoke time. The verified completion PR is recorded first and the |
| 353 | +// claim is held for it, not released with the lease. |
| 354 | +func TestClaims_CompletionPRHoldsClaimBeforeScanSeesIt(t *testing.T) { |
| 355 | + hub, s, l := claimsHub(t) |
| 356 | + const identity = "c-karibhou#a" |
| 357 | + ledger := map[string]ghpkg.IssueClaim{} |
| 358 | + s.deps.IssueClaimed = func(repo string, number int) (ghpkg.IssueClaim, bool) { |
| 359 | + c, ok := ledger[repo+"#"+strconv.Itoa(number)] |
| 360 | + return c, ok |
| 361 | + } |
| 362 | + s.deps.RecordIssueClaim = func(c ghpkg.IssueClaim) error { |
| 363 | + ledger[c.Repo+"#"+strconv.Itoa(c.Issue)] = c |
| 364 | + return nil |
| 365 | + } |
| 366 | + if err := hub.recordLease(identity, "t-1", "myorg/repo1", 1, "contributor", 3, time.Now()); err != nil { |
| 367 | + t.Fatal(err) |
| 368 | + } |
| 369 | + if _, err := l.Claim(claims.Request{Repo: "myorg/repo1", Issue: 1, Holder: "karibhou", HolderID: identity, Kind: claims.KindContributor, Session: "a"}); err != nil { |
| 370 | + t.Fatal(err) |
| 371 | + } |
| 372 | + if _, ok := s.deps.IssueClaimed("myorg/repo1", 1); ok { |
| 373 | + t.Fatal("precondition: scan ledger must not know the PR yet") |
| 374 | + } |
| 375 | + |
| 376 | + task := &WSTaskAssign{TaskID: "t-1", Kind: "issue", Repo: "myorg/repo1", Number: 1} |
| 377 | + hub.recordCompletionPRClaim(task, "https://github.com/myorg/repo1/pull/9", "Karibhou", ghpkg.PRVerification{Verified: true}) |
| 378 | + hub.revokeLease(identity, "t-1") |
| 379 | + |
| 380 | + c, ok := l.Lookup("myorg/repo1", 1) |
| 381 | + if !ok || c.HolderID != identity || !c.PRStarted || c.PRRepo != "myorg/repo1" || c.PRNumber != 9 { |
| 382 | + t.Fatalf("claim after completion = %+v ok=%v, want held for the verified PR #9", c, ok) |
| 383 | + } |
| 384 | + if hub.sweepPRHeldClaims() != 0 { |
| 385 | + t.Fatal("sweep released a claim held for the verified completion PR") |
| 386 | + } |
| 387 | + |
| 388 | + // An unverified or merged completion PR records nothing, so the claim |
| 389 | + // goes with the lease as before. |
| 390 | + for i, pr := range []ghpkg.PRVerification{{}, {Verified: true, Merged: true}} { |
| 391 | + n := 2 + i |
| 392 | + task := &WSTaskAssign{TaskID: "t-x", Kind: "issue", Repo: "myorg/repo1", Number: n} |
| 393 | + if err := hub.recordLease(identity, "t-x", "myorg/repo1", n, "contributor", 3, time.Now()); err != nil { |
| 394 | + t.Fatal(err) |
| 395 | + } |
| 396 | + if _, err := l.Claim(claims.Request{Repo: "myorg/repo1", Issue: n, Holder: "karibhou", HolderID: identity, Kind: claims.KindContributor, Session: "a"}); err != nil { |
| 397 | + t.Fatal(err) |
| 398 | + } |
| 399 | + hub.recordCompletionPRClaim(task, "https://github.com/myorg/repo1/pull/10", "Karibhou", pr) |
| 400 | + hub.revokeLease(identity, "t-x") |
| 401 | + if c, ok := l.Lookup("myorg/repo1", n); ok { |
| 402 | + t.Fatalf("issue %d: claim kept for an unverified/merged completion PR: %+v", n, c) |
| 403 | + } |
| 404 | + } |
| 405 | +} |
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?v=4&size=40){kind=avatar}
0 commit comments