feat: complete assigned pull requests from mobile (#161)
This commit is contained in:
timmy committed
1 parent
35e46f81f1
commit
745c3779c2
7 files changed
+648
-5
No files matched your search
@@ -11,6 +11,7 @@ MY_WORK = Path(__file__).parents[1] / "frontend" / "my-work.js"
|
||||
REVIEW_SHEET = Path(__file__).parents[1] / "frontend" / "review-sheet.js"
|
||||
ISSUE_SHEET = Path(__file__).parents[1] / "frontend" / "issue-sheet.js"
|
||||
CREATE_ISSUE_SHEET = Path(__file__).parents[1] / "frontend" / "create-issue-sheet.js"
|
||||
PULL_SHEET = Path(__file__).parents[1] / "frontend" / "pull-sheet.js"
|
||||
|
||||
|
||||
def test_my_work_queue_prioritizes_labels_then_reviews_and_keeps_repo_identity():
|
||||
@@ -691,6 +692,80 @@ Promise.all([first, duplicate]).then(results =>
|
||||
}
|
||||
|
||||
|
||||
def test_pull_sheet_preserves_comment_draft_and_single_flights_mutations():
|
||||
script = f"""
|
||||
const createPullSheet = require({json.dumps(str(PULL_SHEET))});
|
||||
const values = new Map();
|
||||
const storage = {{getItem:k => values.get(k) || null, setItem:(k,v) => values.set(k,v), removeItem:k => values.delete(k)}};
|
||||
const calls = [];
|
||||
let releaseComment;
|
||||
const controller = createPullSheet({{
|
||||
storage,
|
||||
fetchJson: (url, options={{}}) => {{
|
||||
calls.push({{url, method:options.method || 'GET', body:options.body ? JSON.parse(options.body) : null}});
|
||||
if (url.endsWith('/comments')) return new Promise(resolve => {{ releaseComment = () => resolve({{id:91, body:'Ship it'}}); }});
|
||||
return Promise.resolve({{number:7, merged:true, state:'closed'}});
|
||||
}},
|
||||
}});
|
||||
const item = {{repository:'stackchain/api', number:7}};
|
||||
controller.saveDraft(item, 'Ship it');
|
||||
const first = controller.comment(item, 'Ship it');
|
||||
const duplicate = controller.comment(item, 'Ship it');
|
||||
const during = controller.loadDraft(item);
|
||||
releaseComment();
|
||||
Promise.all([first, duplicate]).then(async comments => {{
|
||||
const merges = await Promise.all([controller.merge(item, 'abc123'), controller.merge(item, 'abc123')]);
|
||||
process.stdout.write(JSON.stringify({{calls, during, after:controller.loadDraft(item), comments, merges}}));
|
||||
}});
|
||||
"""
|
||||
result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
output = json.loads(result.stdout)
|
||||
|
||||
assert output["during"] == "Ship it"
|
||||
assert output["after"] == ""
|
||||
assert output["calls"] == [
|
||||
{"url": "api/v1/repos/stackchain/api/pulls/7/comments", "method": "POST", "body": {"body": "Ship it"}},
|
||||
{"url": "api/v1/repos/stackchain/api/pulls/7/merge", "method": "POST", "body": {"expected_head_sha": "abc123"}},
|
||||
]
|
||||
assert len(output["comments"]) == 2 and len(output["merges"]) == 2
|
||||
|
||||
|
||||
def test_pull_sheet_enables_merge_only_for_current_safe_state():
|
||||
script = f"""
|
||||
const createPullSheet = require({json.dumps(str(PULL_SHEET))});
|
||||
const states = [
|
||||
{{state:'open', draft:false, mergeable:true, merged:false, ci_state:'success', head_sha:'abc'}},
|
||||
{{state:'open', draft:true, mergeable:true, merged:false, ci_state:'success', head_sha:'abc'}},
|
||||
{{state:'open', draft:false, mergeable:true, merged:false, ci_state:'failure', head_sha:'abc'}},
|
||||
{{state:'open', draft:false, mergeable:false, merged:false, ci_state:'success', head_sha:'abc'}},
|
||||
];
|
||||
process.stdout.write(JSON.stringify(states.map(createPullSheet.mergeEligibility)));
|
||||
"""
|
||||
result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
output = json.loads(result.stdout)
|
||||
assert output[0] == {"allowed": True, "reason": "Ready to merge"}
|
||||
assert output[1]["allowed"] is False and "draft" in output[1]["reason"].lower()
|
||||
assert output[2]["allowed"] is False and "CI" in output[2]["reason"]
|
||||
assert output[3]["allowed"] is False and "conflict" in output[3]["reason"].lower()
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_assigned_pulls_open_accessible_mobile_completion_sheet():
|
||||
html = await dashboard()
|
||||
|
||||
assert 'id="pull-sheet"' in html and 'aria-modal="true"' in html
|
||||
assert 'class="my-work-card-main pull-trigger"' in html
|
||||
assert 'id="pull-sheet-status"' in html
|
||||
assert 'id="pull-files"' in html and 'id="pull-comments"' in html
|
||||
assert 'id="pull-comment"' in html and 'maxlength="10000"' in html
|
||||
assert 'id="merge-pull"' in html and 'id="open-pull-gitea"' in html
|
||||
assert '<script src="static/pull-sheet.js"></script>' in html
|
||||
assert "pullController.load(item)" in html
|
||||
assert "window.confirm('Merge ' + selectedPull.key" in html
|
||||
assert "expected_head_sha" in html
|
||||
assert "item.kind === 'pull'" in html and "pull-trigger" in html
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_assigned_issues_open_accessible_mobile_action_sheet_with_safe_mutations():
|
||||
html = await dashboard()
|
||||
|
||||
@@ -0,0 +1,149 @@
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from src import gitea_proxy, main
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_assigned_pull_detail_reports_completion_state(monkeypatch):
|
||||
async def assigned(repository, number):
|
||||
return (repository, number) == ("stackchain/api", 7)
|
||||
|
||||
async def detail(repository, number):
|
||||
assert (repository, number) == ("stackchain/api", 7)
|
||||
return {
|
||||
"repository": repository,
|
||||
"number": number,
|
||||
"title": "Ship mobile flow",
|
||||
"body": "Ready to merge",
|
||||
"url": "https://forge.example/stackchain/api/pulls/7",
|
||||
"author": "alex",
|
||||
"head_sha": "abc123",
|
||||
"state": "open",
|
||||
"draft": False,
|
||||
"mergeable": True,
|
||||
"merged": False,
|
||||
"ci_state": "success",
|
||||
"files": [{"filename": "src/api.py", "additions": 8, "deletions": 2}],
|
||||
"comments": [{"id": 9, "author": "sam", "body": "Ship it"}],
|
||||
}
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned, raising=False)
|
||||
monkeypatch.setattr(main.gitea_proxy, "pull_completion_detail", detail, raising=False)
|
||||
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/detail")
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.headers["cache-control"] == "no-store"
|
||||
assert response.json()["head_sha"] == "abc123"
|
||||
assert response.json()["mergeable"] is True
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_assigned_pull_detail_rejects_unassigned_pull(monkeypatch):
|
||||
async def assigned(repository, number):
|
||||
return False
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned, raising=False)
|
||||
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/private/secret/pulls/9/detail")
|
||||
|
||||
assert response.status_code == 404
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_assigned_pull_comment_posts_only_after_assignment_check(monkeypatch):
|
||||
calls = []
|
||||
|
||||
async def assigned(repository, number):
|
||||
return True
|
||||
|
||||
async def comment(repository, number, body):
|
||||
calls.append((repository, number, body))
|
||||
return {"id": 91, "author": "timmy", "body": body}
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned)
|
||||
monkeypatch.setattr(main.gitea_proxy, "comment_on_issue", comment)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
response = await client.post(
|
||||
"/api/v1/repos/stackchain/api/pulls/7/comments",
|
||||
json={"body": " Ready to ship. "},
|
||||
)
|
||||
|
||||
assert response.status_code == 201
|
||||
assert calls == [("stackchain/api", 7, "Ready to ship.")]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_assigned_pull_merge_requires_current_eligible_head(monkeypatch):
|
||||
calls = []
|
||||
|
||||
async def assigned(repository, number):
|
||||
return True
|
||||
|
||||
async def merge(repository, number, expected_head_sha):
|
||||
calls.append((repository, number, expected_head_sha))
|
||||
return {"number": number, "merged": True, "state": "closed"}
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned)
|
||||
monkeypatch.setattr(main.gitea_proxy, "merge_assigned_pull", merge, raising=False)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
response = await client.post(
|
||||
"/api/v1/repos/stackchain/api/pulls/7/merge",
|
||||
json={"expected_head_sha": "abc123"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"number": 7, "merged": True, "state": "closed"}
|
||||
assert calls == [("stackchain/api", 7, "abc123")]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_assigned_pull_merge_returns_conflict_without_mutating_stale_head(monkeypatch):
|
||||
async def assigned(repository, number):
|
||||
return True
|
||||
|
||||
async def merge(*args):
|
||||
raise gitea_proxy.StalePullError("changed")
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned)
|
||||
monkeypatch.setattr(main.gitea_proxy, "merge_assigned_pull", merge, raising=False)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
response = await client.post(
|
||||
"/api/v1/repos/stackchain/api/pulls/7/merge",
|
||||
json={"expected_head_sha": "abc123"},
|
||||
)
|
||||
|
||||
assert response.status_code == 409
|
||||
assert "New commits" in response.json()["error"]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_gitea_merge_rejects_failed_ci_without_upstream_mutation():
|
||||
requests = []
|
||||
|
||||
async def handler(request):
|
||||
requests.append((request.method, request.url.path))
|
||||
if request.url.path.endswith("/pulls/7"):
|
||||
return httpx.Response(200, json={
|
||||
"number": 7, "state": "open", "draft": False, "mergeable": True,
|
||||
"merged": False, "head": {"sha": "abc123"},
|
||||
})
|
||||
return httpx.Response(200, json={"state": "failure"})
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
with pytest.raises(gitea_proxy.PullNotMergeableError):
|
||||
await gitea_proxy.merge_assigned_pull("stackchain/api", 7, "abc123")
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert requests == [
|
||||
("GET", "/api/v1/repos/stackchain/api/pulls/7"),
|
||||
("GET", "/api/v1/repos/stackchain/api/commits/abc123/status"),
|
||||
]
|
||||
Reference in new issue
Block a user