feat: delete merged source branches safely (Closes #1364)
This commit is contained in:
1 parent
f3876d359d
commit
07f5770aa8
9 files changed
+540
-7
No files matched your search
@@ -0,0 +1,67 @@
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
if os.getenv("STACKCHAIN_RUN_RELEASE_E2E") != "1":
|
||||
pytest.skip("packaged branch-cleanup journey runs only in the browser gate", allow_module_level=True)
|
||||
pytest.importorskip("playwright.sync_api")
|
||||
from playwright.sync_api import expect, sync_playwright
|
||||
|
||||
|
||||
ROOT = Path(__file__).parents[2]
|
||||
FRONTEND = ROOT / "frontend"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("viewport", [
|
||||
{"width": 320, "height": 568},
|
||||
{"width": 390, "height": 844},
|
||||
])
|
||||
def test_merged_source_branch_cleanup_is_phone_openable_and_exact(viewport):
|
||||
with sync_playwright() as playwright:
|
||||
browser = playwright.chromium.launch(headless=True)
|
||||
page = browser.new_page(viewport=viewport)
|
||||
page.set_content((FRONTEND / "index.html").read_text())
|
||||
page.add_style_tag(path=FRONTEND / "dashboard.css")
|
||||
page.add_script_tag(path=FRONTEND / "release-receipt.js")
|
||||
result = page.evaluate("""async () => {
|
||||
const calls=[];
|
||||
const values=new Map();
|
||||
const storage={
|
||||
getItem:key=>values.get(key)||null,
|
||||
setItem:(key,value)=>values.set(key,value),
|
||||
removeItem:key=>values.delete(key),
|
||||
};
|
||||
const receipt=createReleaseReceipt({
|
||||
storage,
|
||||
getLogin:()=> 'timmy',
|
||||
listNode:document.querySelector('#release-watchlist'),
|
||||
confirmAction:()=>true,
|
||||
fetchJson:async(path, options={})=>{
|
||||
calls.push({path, method:options.method, body:options.body});
|
||||
return {deleted:true, source_branch:'timmy/feature', source_head_sha:'abc1234'};
|
||||
},
|
||||
});
|
||||
receipt.capture(
|
||||
{repository:'stackchain/api', number:7, key:'stackchain/api#7'},
|
||||
{merge_commit_sha:'merge456', source_branch:'timmy/feature', source_head_sha:'abc1234', source_repository:'stackchain/api'},
|
||||
);
|
||||
document.querySelector('#release-receipt-sheet').showModal();
|
||||
const button=document.querySelector('.release-watchlist-actions button');
|
||||
const box=button.getBoundingClientRect();
|
||||
const label=button.getAttribute('aria-label');
|
||||
button.click();
|
||||
await new Promise(resolve=>setTimeout(resolve, 0));
|
||||
return {
|
||||
calls, label, height:box.height,
|
||||
branch:document.querySelector('.release-branch-cleanup-status').textContent,
|
||||
overflow:document.documentElement.scrollWidth > document.documentElement.clientWidth,
|
||||
};
|
||||
}""")
|
||||
expect(page.locator("#release-receipt-sheet")).to_be_visible()
|
||||
assert result["height"] >= 44
|
||||
assert result["label"] == "Delete merged source branch timmy/feature at abc1234"
|
||||
assert result["calls"][0]["method"] == "DELETE"
|
||||
assert result["branch"] == "Branch timmy/feature · deleted"
|
||||
assert result["overflow"] is False
|
||||
browser.close()
|
||||
@@ -102,6 +102,13 @@ def test_browser_job_gates_authored_pull_queue_journey():
|
||||
assert "tests/e2e/test_mobile_authored_pull_queue_release.py" in browser
|
||||
|
||||
|
||||
def test_browser_job_gates_mobile_source_branch_cleanup_journey():
|
||||
text = WORKFLOW.read_text()
|
||||
browser = text[text.index(" browser-journey:") : text.index(" release-candidate:")]
|
||||
|
||||
assert "tests/e2e/test_mobile_source_branch_cleanup_release.py" in browser
|
||||
|
||||
|
||||
def test_browser_job_gates_authored_pull_close_recovery_journey():
|
||||
text = WORKFLOW.read_text()
|
||||
browser = text[text.index(" browser-journey:") : text.index(" release-candidate:")]
|
||||
|
||||
+149
-2
@@ -1892,7 +1892,12 @@ async def test_gitea_merge_returns_the_exact_merge_commit_for_release_tracking()
|
||||
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"},
|
||||
"merged": False,
|
||||
"head": {
|
||||
"sha": "abc123",
|
||||
"ref": "timmy/feature",
|
||||
"repo": {"full_name": "stackchain/api"},
|
||||
},
|
||||
})
|
||||
if request.url.path.endswith("/commits/abc123/status"):
|
||||
return httpx.Response(200, json={"state": "success"})
|
||||
@@ -1909,10 +1914,117 @@ async def test_gitea_merge_returns_the_exact_merge_commit_for_release_tracking()
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert result == {
|
||||
"number": 7, "merged": True, "state": "closed", "merge_commit_sha": "merge456"
|
||||
"number": 7,
|
||||
"merged": True,
|
||||
"state": "closed",
|
||||
"merge_commit_sha": "merge456",
|
||||
"source_branch": "timmy/feature",
|
||||
"source_head_sha": "abc123",
|
||||
"source_repository": "stackchain/api",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_source_branch_cleanup_endpoint_audits_and_deletes_the_exact_merged_head(monkeypatch):
|
||||
lifecycle = []
|
||||
|
||||
class Journal:
|
||||
def reserve(self, kind, *, target):
|
||||
lifecycle.append(("reserve", kind, target))
|
||||
return "branch-cleanup"
|
||||
|
||||
def finalize(self, operation_id):
|
||||
lifecycle.append(("finalize", operation_id))
|
||||
|
||||
def discard(self, operation_id):
|
||||
lifecycle.append(("discard", operation_id))
|
||||
|
||||
async def cleanup(repository, number, source_branch, expected_head_sha):
|
||||
lifecycle.append(("delete", repository, number, source_branch, expected_head_sha))
|
||||
return {
|
||||
"number": number,
|
||||
"deleted": True,
|
||||
"source_branch": source_branch,
|
||||
"source_head_sha": expected_head_sha,
|
||||
}
|
||||
|
||||
monkeypatch.setattr(main, "_security_event_store", lambda: Journal())
|
||||
monkeypatch.setattr(main.gitea_proxy, "delete_merged_source_branch", cleanup, raising=False)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
response = await client.request(
|
||||
"DELETE",
|
||||
"/api/v1/repos/stackchain/api/pulls/7/source-branch",
|
||||
json={"source_branch": "timmy/feature", "expected_head_sha": "abc1234"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["deleted"] is True
|
||||
assert lifecycle == [
|
||||
("reserve", "source_branch_deleted", "stackchain/api#7@abc1234"),
|
||||
("delete", "stackchain/api", 7, "timmy/feature", "abc1234"),
|
||||
("finalize", "branch-cleanup"),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_gitea_deletes_only_the_merged_authors_same_repository_exact_source_branch():
|
||||
requests = []
|
||||
|
||||
async def handler(request):
|
||||
path = request.url.raw_path.decode()
|
||||
requests.append((request.method, path))
|
||||
if request.url.path.endswith("/user"):
|
||||
return httpx.Response(200, json={"login": "timmy"})
|
||||
if request.url.path.endswith("/pulls/7"):
|
||||
return httpx.Response(200, json={
|
||||
"state": "closed",
|
||||
"merged": True,
|
||||
"user": {"login": "timmy"},
|
||||
"head": {
|
||||
"ref": "timmy/feature",
|
||||
"sha": "abc123",
|
||||
"repo": {"full_name": "stackchain/api"},
|
||||
},
|
||||
})
|
||||
if request.url.path.endswith("/repos/stackchain/api"):
|
||||
return httpx.Response(200, json={"default_branch": "main"})
|
||||
if path.endswith("/branches/timmy%2Ffeature"):
|
||||
branch_reads = sum(
|
||||
method == "GET" and seen_path.endswith("/branches/timmy%2Ffeature")
|
||||
for method, seen_path in requests
|
||||
)
|
||||
if request.method == "DELETE":
|
||||
return httpx.Response(204)
|
||||
if branch_reads == 1:
|
||||
return httpx.Response(200, json={
|
||||
"name": "timmy/feature",
|
||||
"protected": False,
|
||||
"commit": {"id": "abc123"},
|
||||
})
|
||||
return httpx.Response(404)
|
||||
raise AssertionError(request.url.path)
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
result = await gitea_proxy.delete_merged_source_branch(
|
||||
"stackchain/api", 7, "timmy/feature", "abc123"
|
||||
)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert result == {
|
||||
"number": 7,
|
||||
"deleted": True,
|
||||
"source_branch": "timmy/feature",
|
||||
"source_head_sha": "abc123",
|
||||
}
|
||||
assert requests[-2:] == [
|
||||
("DELETE", "/api/v1/repos/stackchain/api/branches/timmy%2Ffeature"),
|
||||
("GET", "/api/v1/repos/stackchain/api/branches/timmy%2Ffeature"),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_release_receipt_matches_only_the_captured_commit_and_reports_checks(monkeypatch):
|
||||
async def receipt(repository, commit_sha):
|
||||
@@ -1979,3 +2091,38 @@ async def test_gitea_release_receipt_ignores_other_commits_and_normalizes_matchi
|
||||
"url": f"{forge}/stackchain/api/releases/download/rc-42/manifest.json",
|
||||
}],
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_source_branch_cleanup_resolves_a_lost_delete_response_with_one_absence_check():
|
||||
requests = []
|
||||
|
||||
async def handler(request):
|
||||
raw_path = request.url.raw_path.decode()
|
||||
requests.append((request.method, raw_path))
|
||||
if request.url.path.endswith("/user"):
|
||||
return httpx.Response(200, json={"login": "timmy"})
|
||||
if request.url.path.endswith("/pulls/7"):
|
||||
return httpx.Response(200, json={
|
||||
"state": "closed", "merged": True, "user": {"login": "timmy"},
|
||||
"head": {"ref": "timmy/feature", "sha": "abc123", "repo": {"full_name": "stackchain/api"}},
|
||||
})
|
||||
if request.url.path.endswith("/repos/stackchain/api"):
|
||||
return httpx.Response(200, json={"default_branch": "main"})
|
||||
if raw_path.endswith("/branches/timmy%2Ffeature"):
|
||||
if request.method == "DELETE":
|
||||
raise httpx.ReadTimeout("delete response was lost")
|
||||
reads = sum(method == "GET" and path.endswith("/branches/timmy%2Ffeature") for method, path in requests)
|
||||
return httpx.Response(200, json={"protected": False, "commit": {"id": "abc123"}}) if reads == 1 else httpx.Response(404)
|
||||
raise AssertionError(raw_path)
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
result = await gitea_proxy.delete_merged_source_branch(
|
||||
"stackchain/api", 7, "timmy/feature", "abc123"
|
||||
)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert result["deleted"] is True
|
||||
assert sum(method == "GET" and path.endswith("/branches/timmy%2Ffeature") for method, path in requests) == 2
|
||||
@@ -36,6 +36,40 @@ restored && first.refresh().then(status=>{
|
||||
assert output["keys"] == ["stackchain.release-receipt.v1:timmy"]
|
||||
|
||||
|
||||
def test_release_receipt_deletes_the_exact_captured_source_branch_without_stopping_release_tracking():
|
||||
output = run_node(r"""
|
||||
const values=new Map(), calls=[];
|
||||
const storage={getItem:key=>values.get(key)||null,setItem:(key,value)=>values.set(key,value),removeItem:key=>values.delete(key)};
|
||||
const receipt=createReleaseReceipt({storage,getLogin:()=> 'timmy',confirmAction:()=>true,fetchJson:async(path,options={})=>{
|
||||
calls.push({path,method:options.method||'GET',body:options.body||null});
|
||||
if ((options.method||'GET') === 'DELETE') return {number:7,deleted:true,source_branch:'timmy/feature',source_head_sha:'abc1234'};
|
||||
return {commit_sha:'merge456',ci_state:'pending',checks:[],release:null};
|
||||
}});
|
||||
const captured=receipt.capture(
|
||||
{repository:'stackchain/api',number:7,key:'stackchain/api#7'},
|
||||
{merge_commit_sha:'merge456',source_branch:'timmy/feature',source_head_sha:'abc1234',source_repository:'stackchain/api'}
|
||||
);
|
||||
(async()=>{
|
||||
await receipt.deleteBranch('stackchain/api','merge456');
|
||||
await receipt.refresh();
|
||||
process.stdout.write(JSON.stringify({captured,calls,restored:receipt.restore()}));
|
||||
})();
|
||||
""")
|
||||
|
||||
assert output["captured"]["source_branch"] == "timmy/feature"
|
||||
assert output["calls"][0] == {
|
||||
"path": "api/v1/repos/stackchain/api/pulls/7/source-branch",
|
||||
"method": "DELETE",
|
||||
"body": json.dumps({
|
||||
"source_branch": "timmy/feature",
|
||||
"expected_head_sha": "abc1234",
|
||||
}, separators=(",", ":")),
|
||||
}
|
||||
restored = output["restored"][0]
|
||||
assert restored["cleanup"] == {"state": "deleted", "message": "Source branch deleted."}
|
||||
assert restored["status"]["label"] == "Checks running"
|
||||
|
||||
|
||||
def test_receipt_distinguishes_failed_checks_waiting_for_release_and_exact_release():
|
||||
output = run_node(r"""
|
||||
const values=new Map();
|
||||
@@ -142,6 +176,25 @@ receipt.capture({repository:'stackchain/web',number:2,key:'stackchain/web#2'}, {
|
||||
assert [item["commit_sha"] for item in output["after"]] == ["pending"]
|
||||
|
||||
|
||||
def test_watchlist_renders_confirmed_touch_cleanup_for_the_exact_source_branch():
|
||||
output = run_node(r"""
|
||||
function node(tag='div') { return {tag,children:[],hidden:false,textContent:'',disabled:false,append(...xs){this.children.push(...xs)},replaceChildren(...xs){this.children=[...xs]},setAttribute(k,v){this[k]=v},addEventListener(k,fn){this[k]=fn}}; }
|
||||
global.document={createElement:tag=>node(tag)};
|
||||
const values=new Map(), calls=[], listNode=node();
|
||||
const storage={getItem:key=>values.get(key)||null,setItem:(key,value)=>values.set(key,value),removeItem:key=>values.delete(key)};
|
||||
const receipt=createReleaseReceipt({storage,getLogin:()=> 'timmy',listNode,confirmAction:()=>true,fetchJson:async(path,options={})=>{calls.push({path,method:options.method});return {deleted:true};}});
|
||||
receipt.capture({repository:'stackchain/api',number:7,key:'stackchain/api#7'}, {merge_commit_sha:'merge456',source_branch:'timmy/feature',source_head_sha:'abc1234',source_repository:'stackchain/api'});
|
||||
const row=listNode.children[0], copy=row.children[0], actions=row.children[1], cleanup=actions.children[0];
|
||||
(async()=>{ await cleanup.click(); process.stdout.write(JSON.stringify({branch:copy.children[2].textContent,cleanupText:cleanup.textContent,minLabel:cleanup['aria-label'],calls,after:receipt.restore()[0].cleanup})); })();
|
||||
""")
|
||||
|
||||
assert output["branch"] == "Branch timmy/feature · abc1234"
|
||||
assert output["cleanupText"] == "Delete source branch"
|
||||
assert output["minLabel"] == "Delete merged source branch timmy/feature at abc1234"
|
||||
assert output["calls"][0]["method"] == "DELETE"
|
||||
assert output["after"]["state"] == "deleted"
|
||||
|
||||
|
||||
def test_watchlist_polls_pending_merges_only_while_foregrounded():
|
||||
output = run_node(r"""
|
||||
const values=new Map(), scheduled=[], calls=[];
|
||||
@@ -219,3 +272,7 @@ def test_mobile_release_receipt_is_wired_into_the_merge_flow_and_phone_safe():
|
||||
assert "min-height:44px" in css[css.index(".release-receipt-sheet"):]
|
||||
assert "overflow-x:hidden" in css[css.index(".release-receipt-sheet"):]
|
||||
assert ".release-watchlist-item" in css
|
||||
assert ".release-watchlist-actions" in css
|
||||
cleanup_css = css[css.index(".release-watchlist-actions"):]
|
||||
assert "min-height:44px" in cleanup_css
|
||||
assert "grid-template-columns:1fr" in cleanup_css
|
||||
Reference in new issue
Block a user