feat: make mobile review loading retryable (#127)
This commit is contained in:
1 parent
3bf635fb89
commit
173af9f698
6 files changed
+133
-23
No files matched your search
@@ -1,6 +1,8 @@
|
||||
import pytest
|
||||
import asyncio
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
from src import gitea_proxy
|
||||
from src import main
|
||||
|
||||
@@ -54,26 +56,32 @@ async def test_pull_requests_merge_assignment_and_review_responsibilities(monkey
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_requested_review_guard_matches_repository_number_and_reason(monkeypatch):
|
||||
async def pulls():
|
||||
async def test_requested_review_guard_uses_only_dedicated_review_search(monkeypatch):
|
||||
requested_paths = []
|
||||
|
||||
async def fake_fetch(path):
|
||||
requested_paths.append(path)
|
||||
return [
|
||||
{
|
||||
"number": 7,
|
||||
"repository": {"full_name": "stackchain/api"},
|
||||
"work_reasons": ["review_requested"],
|
||||
},
|
||||
{
|
||||
"number": 8,
|
||||
"repository": {"full_name": "stackchain/api"},
|
||||
"work_reasons": ["assigned_to_me"],
|
||||
},
|
||||
]
|
||||
|
||||
monkeypatch.setattr(gitea_proxy, "pull_requests", pulls)
|
||||
monkeypatch.setattr(gitea_proxy, "fetch", fake_fetch)
|
||||
|
||||
assert await gitea_proxy.is_requested_review("stackchain/api", 7) is True
|
||||
assert await gitea_proxy.is_requested_review("stackchain/api", 8) is False
|
||||
assert await gitea_proxy.is_requested_review("stackchain/api", 9) is False
|
||||
assert await gitea_proxy.is_requested_review("stackchain/private", 7) is False
|
||||
assert requested_paths == [
|
||||
"repos/issues/search?state=open&review_requested=true&type=pulls&limit=50",
|
||||
"repos/issues/search?state=open&review_requested=true&type=pulls&limit=50",
|
||||
"repos/issues/search?state=open&review_requested=true&type=pulls&limit=50",
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@@ -183,6 +191,45 @@ async def test_pull_review_detail_combines_pr_files_status_and_reviews(monkeypat
|
||||
assert len(detail["reviews"]) == 1
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_pull_review_detail_fetches_independent_head_resources_concurrently(monkeypatch):
|
||||
started = {name: asyncio.Event() for name in ("files", "status", "reviews", "diff")}
|
||||
release = asyncio.Event()
|
||||
|
||||
async def wait_for_release(name, result):
|
||||
started[name].set()
|
||||
await release.wait()
|
||||
return result
|
||||
|
||||
async def fake_fetch(path):
|
||||
if path.endswith("/pulls/7"):
|
||||
return {"head": {"sha": "abc123"}, "user": {"login": "alex"}}
|
||||
if path.endswith("/files"):
|
||||
return await wait_for_release("files", [])
|
||||
if path.endswith("/reviews"):
|
||||
return await wait_for_release("reviews", [])
|
||||
return await wait_for_release("status", {"state": "success"})
|
||||
|
||||
async def fake_fetch_text(path, max_bytes):
|
||||
return await wait_for_release("diff", ("", False))
|
||||
|
||||
monkeypatch.setattr(gitea_proxy, "fetch", fake_fetch)
|
||||
monkeypatch.setattr(gitea_proxy, "fetch_text", fake_fetch_text)
|
||||
|
||||
detail_task = asyncio.create_task(
|
||||
gitea_proxy.pull_review_detail("stackchain/api", 7)
|
||||
)
|
||||
try:
|
||||
await asyncio.wait_for(
|
||||
asyncio.gather(*(event.wait() for event in started.values())), timeout=1
|
||||
)
|
||||
finally:
|
||||
release.set()
|
||||
|
||||
detail = await detail_task
|
||||
assert detail["head_sha"] == "abc123"
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_pull_review_detail_attaches_bounded_per_file_diff_previews(monkeypatch):
|
||||
async def fake_fetch(path):
|
||||
|
||||
@@ -293,3 +293,15 @@ async def test_review_sheet_loads_details_and_preserves_safe_gitea_handoff():
|
||||
assert "review-history" in html
|
||||
assert "open-review-gitea" in html
|
||||
assert "reviewController.submit" not in html
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_mobile_review_failure_offers_an_in_place_retry_for_the_same_item():
|
||||
html = await dashboard()
|
||||
|
||||
assert 'id="retry-review-load"' in html
|
||||
assert '.review-retry' in html and 'min-height:44px' in html
|
||||
assert "qs('#retry-review-load').hidden = false" in html
|
||||
assert "qs('#retry-review-load').hidden = true" in html
|
||||
assert "openReviewSheet(selectedReview, reviewTrigger)" in html
|
||||
assert "qs('#retry-review-load').focus()" in html
|
||||
@@ -1,3 +1,5 @@
|
||||
import asyncio
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
@@ -48,6 +50,31 @@ async def test_review_detail_rejects_pulls_not_requested_from_service_user(monke
|
||||
assert response.headers["cache-control"] == "no-store"
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_review_detail_has_one_retryable_deadline_and_cancels_pending_work(monkeypatch):
|
||||
cancelled = asyncio.Event()
|
||||
|
||||
async def requested(repository, number):
|
||||
try:
|
||||
await asyncio.Event().wait()
|
||||
finally:
|
||||
cancelled.set()
|
||||
|
||||
monkeypatch.setattr(main, "is_requested_review", requested)
|
||||
monkeypatch.setattr(main, "REVIEW_DETAIL_TIMEOUT_SECONDS", 0.01)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
response = await client.get("/api/v1/repos/stackchain/api/pulls/7/review")
|
||||
|
||||
assert response.status_code == 503
|
||||
assert response.headers["cache-control"] == "no-store"
|
||||
assert response.headers["retry-after"] == "1"
|
||||
assert response.json() == {
|
||||
"error": "Pull request review details timed out. Please retry."
|
||||
}
|
||||
assert cancelled.is_set()
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_public_review_endpoint_does_not_expose_service_token_mutations():
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
|
||||
Reference in new issue
Block a user