Skip to content

Commit 71d3cc6

Browse files
PLeVasseurPLeVasseur
committed
test(reviewer-bot): cover intent routing and git-ref lock behavior
1 parent 3227581 commit 71d3cc6

1 file changed

Lines changed: 113 additions & 39 deletions

File tree

‎.github/reviewer-bot-tests/test_reviewer_bot.py‎

Lines changed: 113 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -336,22 +336,26 @@ def test_acquire_state_issue_lease_lock_success(monkeypatch):
336336
monkeypatch.setenv("WORKFLOW_JOB_NAME", "reviewer-bot")
337337
monkeypatch.setattr(reviewer_bot.random, "uniform", lambda a, b: 0.0)
338338
monkeypatch.setattr(reviewer_bot.time, "sleep", lambda _: None)
339-
340-
body = reviewer_bot.render_state_issue_body(make_state(), reviewer_bot.clear_lock_metadata())
341339
monkeypatch.setattr(
342340
reviewer_bot,
343-
"get_state_issue_snapshot",
344-
lambda: reviewer_bot.StateIssueSnapshot(
345-
body=body,
346-
etag='"etag"',
347-
html_url="https://example.com/issues/314",
341+
"get_lock_ref_snapshot",
342+
lambda: ("parent-sha", "tree-sha", reviewer_bot.clear_lock_metadata()),
343+
)
344+
monkeypatch.setattr(
345+
reviewer_bot,
346+
"create_lock_commit",
347+
lambda parent_sha, tree_sha, lock_meta: reviewer_bot.GitHubApiResult(
348+
status_code=201,
349+
payload={"sha": "new-lock-commit-sha"},
350+
headers={},
351+
text="",
352+
ok=True,
348353
),
349354
)
350-
351355
monkeypatch.setattr(
352356
reviewer_bot,
353-
"conditional_patch_state_issue",
354-
lambda body, etag: reviewer_bot.GitHubApiResult(
357+
"cas_update_lock_ref",
358+
lambda new_sha: reviewer_bot.GitHubApiResult(
355359
status_code=200,
356360
payload={"ok": True},
357361
headers={},
@@ -366,30 +370,36 @@ def test_acquire_state_issue_lease_lock_success(monkeypatch):
366370
assert ctx.lock_owner_workflow == "Reviewer Bot"
367371
assert ctx.lock_owner_job == "reviewer-bot"
368372
assert reviewer_bot.ACTIVE_LEASE_CONTEXT is not None
373+
assert ctx.lock_ref == "refs/heads/reviewer-bot-state-lock"
374+
assert isinstance(ctx.lock_expires_at, str)
369375

370376

371377
def test_acquire_state_issue_lease_lock_retries_on_conflict(monkeypatch):
372378
monkeypatch.setattr(reviewer_bot, "ACTIVE_LEASE_CONTEXT", None)
373379
monkeypatch.setattr(reviewer_bot.random, "uniform", lambda a, b: 0.0)
374380
monkeypatch.setattr(reviewer_bot.time, "sleep", lambda _: None)
375381

376-
body = reviewer_bot.render_state_issue_body(make_state(), reviewer_bot.clear_lock_metadata())
377382
monkeypatch.setattr(
378383
reviewer_bot,
379-
"get_state_issue_snapshot",
380-
lambda: reviewer_bot.StateIssueSnapshot(
381-
body=body,
382-
etag='"etag"',
383-
html_url="https://example.com/issues/314",
384+
"get_lock_ref_snapshot",
385+
lambda: ("parent-sha", "tree-sha", reviewer_bot.clear_lock_metadata()),
386+
)
387+
monkeypatch.setattr(
388+
reviewer_bot,
389+
"create_lock_commit",
390+
lambda parent_sha, tree_sha, lock_meta: reviewer_bot.GitHubApiResult(
391+
status_code=201,
392+
payload={"sha": "new-lock-commit-sha"},
393+
headers={},
394+
text="",
395+
ok=True,
384396
),
385397
)
386-
387-
statuses = iter([412, 200])
388-
398+
statuses = iter([409, 200])
389399
monkeypatch.setattr(
390400
reviewer_bot,
391-
"conditional_patch_state_issue",
392-
lambda body, etag: reviewer_bot.GitHubApiResult(
401+
"cas_update_lock_ref",
402+
lambda new_sha: reviewer_bot.GitHubApiResult(
393403
status_code=next(statuses),
394404
payload={"ok": True},
395405
headers={},
@@ -413,27 +423,33 @@ def test_acquire_state_issue_lease_lock_takes_over_expired_lock(monkeypatch):
413423
"lock_owner_run_id": "123",
414424
"lock_owner_workflow": "Reviewer Bot",
415425
"lock_owner_job": "reviewer-bot",
426+
"lock_state": "locked",
416427
"lock_token": "stale-lock",
417428
"lock_acquired_at": "2020-01-01T00:00:00+00:00",
418429
"lock_expires_at": "2020-01-01T00:01:00+00:00",
419430
}
420431
)
421-
body = reviewer_bot.render_state_issue_body(make_state(), expired_lock)
432+
monkeypatch.setattr(
433+
reviewer_bot,
434+
"get_lock_ref_snapshot",
435+
lambda: ("parent-sha", "tree-sha", expired_lock),
436+
)
422437

423438
monkeypatch.setattr(
424439
reviewer_bot,
425-
"get_state_issue_snapshot",
426-
lambda: reviewer_bot.StateIssueSnapshot(
427-
body=body,
428-
etag='"etag"',
429-
html_url="https://example.com/issues/314",
440+
"create_lock_commit",
441+
lambda parent_sha, tree_sha, lock_meta: reviewer_bot.GitHubApiResult(
442+
status_code=201,
443+
payload={"sha": "new-lock-commit-sha"},
444+
headers={},
445+
text="",
446+
ok=True,
430447
),
431448
)
432-
433449
monkeypatch.setattr(
434450
reviewer_bot,
435-
"conditional_patch_state_issue",
436-
lambda body, etag: reviewer_bot.GitHubApiResult(
451+
"cas_update_lock_ref",
452+
lambda new_sha: reviewer_bot.GitHubApiResult(
437453
status_code=200,
438454
payload={"ok": True},
439455
headers={},
@@ -458,24 +474,19 @@ def test_acquire_state_issue_lease_lock_times_out(monkeypatch):
458474
"lock_owner_run_id": "other-run",
459475
"lock_owner_workflow": "Reviewer Bot",
460476
"lock_owner_job": "reviewer-bot",
477+
"lock_state": "locked",
461478
"lock_token": "active-lock",
462479
"lock_acquired_at": "2999-01-01T00:00:00+00:00",
463480
"lock_expires_at": "2999-01-01T00:10:00+00:00",
464481
}
465482
)
466-
body = reviewer_bot.render_state_issue_body(make_state(), valid_lock)
467-
468483
monkeypatch.setattr(
469484
reviewer_bot,
470-
"get_state_issue_snapshot",
471-
lambda: reviewer_bot.StateIssueSnapshot(
472-
body=body,
473-
etag='"etag"',
474-
html_url="https://example.com/issues/314",
475-
),
485+
"get_lock_ref_snapshot",
486+
lambda: ("parent-sha", "tree-sha", valid_lock),
476487
)
477488

478-
monotonic_values = iter([0.0, 0.0, 2.0, 2.0])
489+
monotonic_values = iter([0.0, 0.0, 2.0])
479490
monkeypatch.setattr(reviewer_bot.time, "monotonic", lambda: next(monotonic_values))
480491

481492
with pytest.raises(RuntimeError, match="Timed out waiting for reviewer-bot lease lock"):
@@ -1683,6 +1694,69 @@ def test_handle_comment_event_unknown_command(stub_api, captured_comments):
16831694
assert "Unknown command" in captured_comments[0]["body"]
16841695

16851696

1697+
def test_classify_event_intent_cross_repo_review_is_non_mutating_defer(monkeypatch):
1698+
monkeypatch.setenv("PR_IS_CROSS_REPOSITORY", "true")
1699+
intent = reviewer_bot.classify_event_intent("pull_request_review", "submitted")
1700+
assert intent == reviewer_bot.EVENT_INTENT_NON_MUTATING_DEFER
1701+
1702+
1703+
def test_classify_event_intent_same_repo_review_is_mutating(monkeypatch):
1704+
monkeypatch.setenv("PR_IS_CROSS_REPOSITORY", "false")
1705+
intent = reviewer_bot.classify_event_intent("pull_request_review", "submitted")
1706+
assert intent == reviewer_bot.EVENT_INTENT_MUTATING
1707+
1708+
1709+
def test_main_cross_repo_review_does_not_acquire_lock(monkeypatch):
1710+
monkeypatch.setenv("EVENT_NAME", "pull_request_review")
1711+
monkeypatch.setenv("EVENT_ACTION", "submitted")
1712+
monkeypatch.setenv("PR_IS_CROSS_REPOSITORY", "true")
1713+
1714+
acquire_called = {"value": False}
1715+
1716+
def fail_if_called():
1717+
acquire_called["value"] = True
1718+
raise AssertionError("acquire_state_issue_lease_lock should not be called")
1719+
1720+
monkeypatch.setattr(reviewer_bot, "acquire_state_issue_lease_lock", fail_if_called)
1721+
monkeypatch.setattr(reviewer_bot, "load_state", lambda: make_state())
1722+
monkeypatch.setattr(reviewer_bot, "handle_pull_request_review_event", lambda state: False)
1723+
1724+
reviewer_bot.main()
1725+
1726+
assert acquire_called["value"] is False
1727+
1728+
1729+
def test_main_same_repo_review_acquires_lock(monkeypatch):
1730+
monkeypatch.setenv("EVENT_NAME", "pull_request_review")
1731+
monkeypatch.setenv("EVENT_ACTION", "submitted")
1732+
monkeypatch.setenv("PR_IS_CROSS_REPOSITORY", "false")
1733+
1734+
acquire_called = {"value": False}
1735+
1736+
def fake_acquire():
1737+
acquire_called["value"] = True
1738+
return reviewer_bot.LeaseContext(
1739+
lock_token="token",
1740+
lock_owner_run_id="run",
1741+
lock_owner_workflow="workflow",
1742+
lock_owner_job="job",
1743+
state_issue_url="https://example.com/issues/314",
1744+
lock_ref="refs/heads/reviewer-bot-state-lock",
1745+
lock_expires_at="2999-01-01T00:00:00+00:00",
1746+
)
1747+
1748+
monkeypatch.setattr(reviewer_bot, "acquire_state_issue_lease_lock", fake_acquire)
1749+
monkeypatch.setattr(reviewer_bot, "release_state_issue_lease_lock", lambda: True)
1750+
monkeypatch.setattr(reviewer_bot, "load_state", lambda: make_state())
1751+
monkeypatch.setattr(reviewer_bot, "process_pass_until_expirations", lambda state: (state, []))
1752+
monkeypatch.setattr(reviewer_bot, "sync_members_with_queue", lambda state: (state, []))
1753+
monkeypatch.setattr(reviewer_bot, "handle_pull_request_review_event", lambda state: False)
1754+
1755+
reviewer_bot.main()
1756+
1757+
assert acquire_called["value"] is True
1758+
1759+
16861760
def test_main_fails_when_save_state_fails(monkeypatch):
16871761
monkeypatch.setenv("EVENT_NAME", "issue_comment")
16881762
monkeypatch.setenv("EVENT_ACTION", "created")

0 commit comments

Comments
 (0)