Merge pull request 'Show actionable CI checks in mobile pull reviews' (#518) from timmy/517-actionable-ci-checks into main
This commit is contained in:
commit
5bef6291e8
|
|
@ -232,6 +232,18 @@ textarea { resize: vertical; min-height: 120px; }
|
|||
.review-diff-note, .review-diff-empty { display:block; padding:8px; color:#fcd34d; white-space:normal; }
|
||||
.review-action { min-height:44px; }
|
||||
.review-retry { min-height:44px; margin-top:10px; }
|
||||
.ci-checks { max-width:100%; margin:10px 0; border:1px solid #2a496e; border-radius:10px; overflow:hidden; }
|
||||
.ci-checks > summary { min-height:44px; display:flex; align-items:center; justify-content:space-between; gap:8px; padding:0 10px; cursor:pointer; }
|
||||
.ci-checks > summary .small { min-width:0; overflow-wrap:anywhere; text-align:right; }
|
||||
.ci-check-refresh { width:100%; min-height:44px; margin-bottom:6px; }
|
||||
.ci-check-list { display:grid; gap:6px; min-width:0; padding:0 8px 8px; }
|
||||
.ci-check { min-width:0; display:grid; grid-template-columns:minmax(0,1fr) auto; align-items:center; gap:8px; padding:8px; border:1px solid #203a5c; border-radius:8px; }
|
||||
.ci-check-failure, .ci-check-error { border-color:#dc2626; background:rgba(127,29,29,.22); }
|
||||
.ci-check-pending { border-color:#d97706; background:rgba(120,53,15,.2); }
|
||||
.ci-check-success { border-color:#15803d; }
|
||||
.ci-check-copy { min-width:0; display:grid; gap:3px; overflow-wrap:anywhere; }
|
||||
.ci-check-copy strong, .ci-check-copy span { min-width:0; overflow-wrap:anywhere; }
|
||||
.ci-check-link { min-height:44px; display:flex; align-items:center; justify-content:center; padding:0 10px; border:1px solid #60a5fa; border-radius:8px; white-space:nowrap; }
|
||||
.update-sheet { position:fixed; inset:0; z-index:55; display:none; justify-content:flex-end; background:rgba(5,12,21,.72); backdrop-filter:blur(4px); }
|
||||
.update-sheet.open { display:flex; }
|
||||
.update-sheet-panel { width:min(560px,100%); height:100%; overflow:auto; padding:18px; background:#0b1526; border-left:1px solid #2a496e; }
|
||||
|
|
|
|||
|
|
@ -2293,7 +2293,15 @@
|
|||
if (issueTrigger?.isConnected) issueTrigger.focus();
|
||||
}
|
||||
|
||||
function renderCheckSection(prefix, detail, offline = false) {
|
||||
const rendered = createReviewController.renderChecks(detail?.checks, escapeHtml, { offline });
|
||||
qs('#' + prefix + '-checks-summary').textContent = rendered.summary;
|
||||
qs('#' + prefix + '-check-list').innerHTML = rendered.html || '<div class="muted">No individual checks were reported.</div>';
|
||||
qs('#' + prefix + '-checks').open = rendered.expanded;
|
||||
}
|
||||
|
||||
function renderPullReview(detail, focusFilename = null) {
|
||||
renderCheckSection('pull', detail);
|
||||
pullReviewState = pullController.reviewState(selectedPull, detail);
|
||||
qs('#pull-review-progress').textContent = pullReviewState.total ?
|
||||
pullReviewState.reviewed.length + ' of ' + pullReviewState.total + ' files reviewed' : 'No changed files to review';
|
||||
|
|
@ -2340,7 +2348,7 @@
|
|||
}
|
||||
}
|
||||
|
||||
async function loadPullReview() {
|
||||
async function loadPullReview({ refresh = false } = {}) {
|
||||
if (!selectedPull || !selectedPullDetail?.head_sha) return;
|
||||
const item = selectedPull;
|
||||
const readDetail = selectedPullDetail;
|
||||
|
|
@ -2348,7 +2356,7 @@
|
|||
qs('#pull-review-retry').hidden = true;
|
||||
qs('#merge-pull').disabled = true;
|
||||
try {
|
||||
const review = await pullController.loadReview(item, readDetail.head_sha);
|
||||
const review = await pullController.loadReview(item, readDetail.head_sha, { refresh });
|
||||
if (selectedPull !== item) return;
|
||||
selectedPullDetail = { ...readDetail, ...review };
|
||||
qs('#pull-ci-state').textContent = 'CI ' + (review.ci_state || 'unknown');
|
||||
|
|
@ -2396,6 +2404,9 @@
|
|||
qs('#pull-comment').value = pullController.loadDraft(item);
|
||||
qs('#pull-comment-status').textContent = '';
|
||||
qs('#pull-ci-state').textContent = 'CI unknown';
|
||||
qs('#pull-checks-summary').textContent = 'Not loaded';
|
||||
qs('#pull-check-list').textContent = '';
|
||||
qs('#pull-checks').open = false;
|
||||
qs('#pull-merge-state').textContent = 'Review data not loaded';
|
||||
qs('#merge-pull').disabled = true;
|
||||
qs('#merge-pull').textContent = workSession.active() ? 'Merge & next' : 'Merge';
|
||||
|
|
@ -2946,6 +2957,11 @@
|
|||
qs('#review-handoff-status').textContent = '';
|
||||
qs('#review-submit-status').textContent = '';
|
||||
qs('#continue-review-to-merge').hidden = true;
|
||||
qs('#review-ci-state').textContent = 'CI unknown';
|
||||
qs('#review-checks-summary').textContent = cachedDetail ? 'Last known · loading' : 'Loading';
|
||||
qs('#review-check-list').textContent = '';
|
||||
qs('#review-checks').open = false;
|
||||
qs('#refresh-review-checks').disabled = offlineReview;
|
||||
qs('#submit-review').disabled = true;
|
||||
qs('#submit-review').textContent = offlineReview && reviewingActiveTodayItem() ? 'Queue review & next' :
|
||||
(offlineReview ? 'Queue review for reconnect' : 'Submit review');
|
||||
|
|
@ -2959,6 +2975,7 @@
|
|||
if (selectedReview !== item) return;
|
||||
qs('#review-sheet-body').innerHTML = renderMarkdown(detail.body || 'No description provided.');
|
||||
qs('#review-ci-state').textContent = 'CI ' + (detail.ci_state || 'unknown');
|
||||
renderCheckSection('review', detail, offlineReview);
|
||||
qs('#review-files').innerHTML = (detail.files || []).length ? detail.files.map((file, index) =>
|
||||
createReviewController.renderDiffFile(file, index, escapeHtml)
|
||||
).join('') : '<div>No changed files reported.</div>';
|
||||
|
|
@ -4262,6 +4279,13 @@
|
|||
if (event.currentTarget.open) loadPullReview();
|
||||
});
|
||||
qs('#pull-review-retry').addEventListener('click', loadPullReview);
|
||||
qs('#refresh-pull-checks').addEventListener('click', async () => {
|
||||
const button = qs('#refresh-pull-checks');
|
||||
button.disabled = true;
|
||||
qs('#pull-checks-summary').textContent = 'Refreshing…';
|
||||
try { await loadPullReview({ refresh: true }); }
|
||||
finally { button.disabled = false; }
|
||||
});
|
||||
qs('#next-unreviewed-pull-file').addEventListener('click', focusNextUnreviewedPullFile);
|
||||
qs('#load-older-pull-comments').addEventListener('click', async () => {
|
||||
if (!pullConversation) return;
|
||||
|
|
@ -4459,6 +4483,25 @@
|
|||
qs('#retry-review-load').addEventListener('click', () => {
|
||||
if (selectedReview) openReviewSheet(selectedReview, reviewTrigger);
|
||||
});
|
||||
qs('#refresh-review-checks').addEventListener('click', async () => {
|
||||
if (!selectedReview || offlineReview) return;
|
||||
const item = selectedReview;
|
||||
const button = qs('#refresh-review-checks');
|
||||
button.disabled = true;
|
||||
qs('#review-checks-summary').textContent = 'Refreshing…';
|
||||
try {
|
||||
const detail = await reviewController.load(item);
|
||||
if (selectedReview !== item) return;
|
||||
if (selectedReviewHead && detail.head_sha !== selectedReviewHead) {
|
||||
qs('#review-sheet-status').textContent = 'New commits detected. Reload the review before submitting feedback.';
|
||||
qs('#submit-review').disabled = true;
|
||||
}
|
||||
qs('#review-ci-state').textContent = 'CI ' + (detail.ci_state || 'unknown');
|
||||
renderCheckSection('review', detail);
|
||||
} catch (error) {
|
||||
if (selectedReview === item) qs('#review-checks-summary').textContent = 'Refresh failed · retry';
|
||||
} finally { button.disabled = false; }
|
||||
});
|
||||
qs('#next-unreviewed-review').addEventListener('click', () => {
|
||||
if (progress) openNextUnreviewed(progress.snapshot());
|
||||
});
|
||||
|
|
|
|||
|
|
@ -578,6 +578,11 @@
|
|||
<div id="pull-review-status" class="small" aria-live="polite">Expand to load changed files and merge readiness.</div>
|
||||
<button class="pull-retry" id="pull-review-retry" type="button" hidden>Retry review data</button>
|
||||
<div class="row"><span class="pill" id="pull-ci-state">CI unknown</span><span class="pill" id="pull-merge-state">Review data not loaded</span></div>
|
||||
<details class="ci-checks" id="pull-checks">
|
||||
<summary><span>Checks</span><span class="small" id="pull-checks-summary">Not loaded</span></summary>
|
||||
<button class="ci-check-refresh" id="refresh-pull-checks" type="button">Refresh checks</button>
|
||||
<div class="ci-check-list" id="pull-check-list"></div>
|
||||
</details>
|
||||
<div class="pull-review-tools"><span id="pull-review-progress" class="small" aria-live="polite">Review progress unavailable.</span><button id="next-unreviewed-pull-file" type="button" disabled>Next unreviewed</button></div>
|
||||
<div id="pull-files"></div>
|
||||
<button id="merge-pull" type="button" disabled>Merge</button>
|
||||
|
|
@ -609,6 +614,11 @@
|
|||
<button class="review-retry" id="retry-review-load" hidden>Retry loading review</button>
|
||||
<div class="review-sheet-body markdown-content" id="review-sheet-body"></div>
|
||||
<div class="row"><span class="pill" id="review-ci-state">CI unknown</span><button class="share-work-route" type="button">Share</button><a id="open-review-gitea" href="#" target="_blank" rel="noopener noreferrer">Open in Gitea</a></div>
|
||||
<details class="ci-checks" id="review-checks">
|
||||
<summary><span>Checks</span><span class="small" id="review-checks-summary">Not loaded</span></summary>
|
||||
<button class="ci-check-refresh" id="refresh-review-checks" type="button">Refresh checks</button>
|
||||
<div class="ci-check-list" id="review-check-list"></div>
|
||||
</details>
|
||||
<h2>Changed files</h2>
|
||||
<div class="review-display-tools">
|
||||
<button id="review-wrap-lines" type="button" aria-pressed="false" aria-controls="review-files">Wrap lines</button>
|
||||
|
|
|
|||
|
|
@ -5,7 +5,11 @@ function mergeEligibility(detail, reviewState) {
|
|||
if (detail.draft) return { allowed: false, reason: 'Draft pull requests cannot be merged' };
|
||||
if (!detail.mergeable) return { allowed: false, reason: 'Resolve conflicts before merging' };
|
||||
if (detail.ci_state !== 'success') {
|
||||
return { allowed: false, reason: 'CI must succeed before merging' };
|
||||
const blockers = (detail.checks || []).filter(check =>
|
||||
['failure', 'error', 'pending'].includes(check.state)
|
||||
).map(check => check.name).filter(Boolean).slice(0, 3);
|
||||
return { allowed: false, reason: blockers.length ?
|
||||
'CI blocked by ' + blockers.join(', ') : 'CI must succeed before merging' };
|
||||
}
|
||||
if (!detail.head_sha) return { allowed: false, reason: 'Current head is unavailable' };
|
||||
if (reviewState && !reviewState.complete) {
|
||||
|
|
@ -64,9 +68,9 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT
|
|||
load(item) {
|
||||
return fetchJson(pathFor(item) + '/detail', { headers: { Accept: 'application/json' } });
|
||||
},
|
||||
loadReview(item, headSha) {
|
||||
loadReview(item, headSha, { refresh = false } = {}) {
|
||||
const key = item.repository + '#' + item.number + ':' + headSha;
|
||||
if (reviewCache.has(key)) return Promise.resolve(reviewCache.get(key));
|
||||
if (!refresh && reviewCache.has(key)) return Promise.resolve(reviewCache.get(key));
|
||||
if (reviewRequests.has(key)) return reviewRequests.get(key);
|
||||
const request = fetchJson(pathFor(item) + '/review-data', {
|
||||
headers: { Accept: 'application/json' },
|
||||
|
|
|
|||
|
|
@ -355,6 +355,34 @@ function prepareMergeContinuation({ storage, item, headSha, reviewed, decision }
|
|||
return true;
|
||||
}
|
||||
|
||||
function renderChecks(checks, escapeHtml, { offline = false } = {}) {
|
||||
const rank = { error: 0, failure: 0, pending: 1, warning: 2, unknown: 2, success: 3 };
|
||||
const items = (Array.isArray(checks) ? checks : []).filter(check =>
|
||||
check && typeof check.name === 'string' && check.name
|
||||
).map((check, index) => ({ ...check, index })).sort((left, right) =>
|
||||
(rank[left.state] ?? 2) - (rank[right.state] ?? 2) || left.index - right.index
|
||||
);
|
||||
const failed = items.filter(check => ['failure', 'error'].includes(check.state)).length;
|
||||
const pending = items.filter(check => check.state === 'pending').length;
|
||||
const passed = items.filter(check => check.state === 'success').length;
|
||||
const other = items.length - failed - pending - passed;
|
||||
const parts = [
|
||||
failed ? failed + ' failed' : '', pending ? pending + ' pending' : '',
|
||||
passed ? passed + ' passed' : '', other ? other + ' other' : '',
|
||||
].filter(Boolean);
|
||||
const summary = (offline ? 'Last known · ' : '') + (parts.join(' · ') || 'No checks reported');
|
||||
const html = items.map(check => {
|
||||
const url = typeof check.url === 'string' ? check.url : '';
|
||||
const link = url ? '<a class="ci-check-link" href="' + escapeHtml(url) +
|
||||
'" target="_blank" rel="noopener noreferrer">Open job</a>' : '';
|
||||
return '<article class="ci-check ci-check-' + escapeHtml(check.state || 'unknown') + '">' +
|
||||
'<div class="ci-check-copy"><strong>' + escapeHtml(check.name) + '</strong>' +
|
||||
'<span class="small">' + escapeHtml(check.state || 'unknown') +
|
||||
(check.description ? ' · ' + escapeHtml(check.description) : '') + '</span></div>' + link + '</article>';
|
||||
}).join('');
|
||||
return { summary, html, expanded: failed > 0 };
|
||||
}
|
||||
|
||||
createReviewController.renderDiffFile = renderDiffFile;
|
||||
createReviewController.createWrapPreference = createWrapPreference;
|
||||
createReviewController.parseDiffLines = parseDiffLines;
|
||||
|
|
@ -364,6 +392,7 @@ createReviewController.createDraft = createDraft;
|
|||
createReviewController.formatFeedback = formatFeedback;
|
||||
createReviewController.copyAndContinue = copyAndContinue;
|
||||
createReviewController.prepareMergeContinuation = prepareMergeContinuation;
|
||||
createReviewController.renderChecks = renderChecks;
|
||||
|
||||
if (typeof module !== 'undefined' && module.exports) {
|
||||
module.exports = createReviewController;
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import re
|
|||
import shlex
|
||||
from contextlib import asynccontextmanager
|
||||
from typing import Any
|
||||
from urllib.parse import urlsplit
|
||||
from urllib.parse import urljoin, urlsplit
|
||||
|
||||
import httpx
|
||||
|
||||
|
|
@ -600,6 +600,51 @@ def _safe_web_url(value: Any) -> str:
|
|||
return value if parsed.scheme in {"http", "https"} and parsed.netloc else ""
|
||||
|
||||
|
||||
def _safe_gitea_web_url(value: Any) -> str:
|
||||
if not isinstance(value, str) or not value.strip():
|
||||
return ""
|
||||
resolved = urljoin(f"{GITEA_URL}/", value.strip())
|
||||
parsed = urlsplit(resolved)
|
||||
configured = urlsplit(GITEA_URL)
|
||||
base_path = configured.path.rstrip("/")
|
||||
if (
|
||||
parsed.scheme not in {"http", "https"}
|
||||
or parsed.scheme != configured.scheme
|
||||
or parsed.netloc != configured.netloc
|
||||
or (base_path and parsed.path != base_path and not parsed.path.startswith(f"{base_path}/"))
|
||||
):
|
||||
return ""
|
||||
return resolved
|
||||
|
||||
|
||||
def _normalize_commit_checks(status: Any) -> list[dict]:
|
||||
entries = status.get("statuses") if isinstance(status, dict) else None
|
||||
if not isinstance(entries, list):
|
||||
return []
|
||||
rank = {"error": 0, "failure": 0, "pending": 1, "warning": 2, "success": 3}
|
||||
checks = []
|
||||
for index, entry in enumerate(entries):
|
||||
if not isinstance(entry, dict):
|
||||
continue
|
||||
name = entry.get("context")
|
||||
if not isinstance(name, str) or not name.strip():
|
||||
continue
|
||||
state = entry.get("status", entry.get("state", "unknown"))
|
||||
state = state.lower() if isinstance(state, str) else "unknown"
|
||||
if state not in rank:
|
||||
state = "unknown"
|
||||
description = entry.get("description")
|
||||
checks.append({
|
||||
"name": name.strip()[:120],
|
||||
"state": state,
|
||||
"description": description.strip()[:240] if isinstance(description, str) else "",
|
||||
"url": _safe_gitea_web_url(entry.get("target_url")),
|
||||
"_index": index,
|
||||
})
|
||||
checks.sort(key=lambda check: (rank.get(check["state"], 2), check["_index"]))
|
||||
return [{key: value for key, value in check.items() if key != "_index"} for check in checks[:20]]
|
||||
|
||||
|
||||
def _normalize_notifications(threads: Any) -> list[dict]:
|
||||
if not isinstance(threads, list):
|
||||
raise ValueError("Gitea notification response was not a list")
|
||||
|
|
@ -1741,6 +1786,7 @@ async def pull_completion_review(repository: str, number: int) -> dict:
|
|||
"mergeable": pull.get("mergeable") is True,
|
||||
"merged": pull.get("merged") is True,
|
||||
"ci_state": status.get("state", "unknown") if isinstance(status, dict) else "unknown",
|
||||
"checks": _normalize_commit_checks(status),
|
||||
"files": [
|
||||
{
|
||||
"filename": item.get("filename", ""),
|
||||
|
|
@ -1873,6 +1919,7 @@ async def pull_review_detail(repository: str, number: int) -> dict:
|
|||
"author": user.get("login", ""),
|
||||
"head_sha": sha,
|
||||
"ci_state": status.get("state", "unknown") if isinstance(status, dict) else "unknown",
|
||||
"checks": _normalize_commit_checks(status),
|
||||
"files": normalized_files,
|
||||
"reviews": normalized_reviews,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3965,7 +3965,9 @@ 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:true, merged:false, ci_state:'failure', head_sha:'abc', checks:[
|
||||
{{name:'lint', state:'failure'}}, {{name:'release', state:'pending'}}, {{name:'build', state:'success'}}
|
||||
]}},
|
||||
{{state:'open', draft:false, mergeable:false, merged:false, ci_state:'success', head_sha:'abc'}},
|
||||
];
|
||||
process.stdout.write(JSON.stringify(states.map(createPullSheet.mergeEligibility)));
|
||||
|
|
@ -3974,7 +3976,7 @@ process.stdout.write(JSON.stringify(states.map(createPullSheet.mergeEligibility)
|
|||
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[2] == {"allowed": False, "reason": "CI blocked by lint, release"}
|
||||
assert output[3]["allowed"] is False and "conflict" in output[3]["reason"].lower()
|
||||
|
||||
|
||||
|
|
@ -4053,6 +4055,34 @@ Promise.allSettled([first, concurrent]).then(async failed => {{
|
|||
]
|
||||
|
||||
|
||||
def test_pull_sheet_refreshes_checks_without_clearing_head_scoped_progress():
|
||||
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)}};
|
||||
const calls = [];
|
||||
const sheet = createPullSheet({{storage, fetchJson: url => {{
|
||||
calls.push(url);
|
||||
return Promise.resolve({{head_sha:'abc123', ci_state:calls.length === 1 ? 'pending' : 'success',
|
||||
checks:[{{name:'lint', state:calls.length === 1 ? 'pending' : 'success'}}],
|
||||
files:[{{filename:'src/api.py'}}]}});
|
||||
}}}});
|
||||
const item = {{repository:'stackchain/api', number:7}};
|
||||
sheet.loadReview(item, 'abc123').then(first => {{
|
||||
sheet.toggleReviewed(item, first, 'src/api.py');
|
||||
return sheet.loadReview(item, 'abc123', {{refresh:true}}).then(refreshed => {{
|
||||
process.stdout.write(JSON.stringify({{calls, refreshed, progress:sheet.reviewState(item, refreshed)}}));
|
||||
}});
|
||||
}});
|
||||
"""
|
||||
result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
output = json.loads(result.stdout)
|
||||
|
||||
assert len(output["calls"]) == 2
|
||||
assert output["refreshed"]["ci_state"] == "success"
|
||||
assert output["progress"] == {"reviewed": ["src/api.py"], "total": 1, "complete": True}
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_mobile_pull_sheet_puts_reading_before_collapsed_review_controls():
|
||||
html = await dashboard()
|
||||
|
|
@ -4493,6 +4523,50 @@ process.stdout.write(JSON.stringify({{ html, expanded: button.attrs['aria-expand
|
|||
assert output["hidden"] is False
|
||||
|
||||
|
||||
def test_review_checks_render_blockers_first_with_safe_touch_links():
|
||||
script = f"""
|
||||
const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
|
||||
const escapeHtml = value => String(value)
|
||||
.replaceAll('&', '&').replaceAll('<', '<').replaceAll('>', '>').replaceAll('"', '"');
|
||||
const result = reviewSheet.renderChecks([
|
||||
{{name:'build', state:'success', description:'Passed'}},
|
||||
{{name:'lint <mobile>', state:'failure', description:'Open & fix', url:'https://forge.example/jobs/4'}},
|
||||
{{name:'release', state:'pending', description:'Waiting'}},
|
||||
], escapeHtml, {{offline:false}});
|
||||
const offline = reviewSheet.renderChecks([
|
||||
{{name:'lint', state:'success', description:'Passed'}},
|
||||
], escapeHtml, {{offline:true}});
|
||||
process.stdout.write(JSON.stringify({{result, offline}}));
|
||||
"""
|
||||
result = subprocess.run(
|
||||
["node", "-e", script], check=True, capture_output=True, text=True
|
||||
)
|
||||
output = json.loads(result.stdout)
|
||||
|
||||
assert output["result"]["summary"] == "1 failed · 1 pending · 1 passed"
|
||||
assert output["result"]["expanded"] is True
|
||||
assert output["result"]["html"].index("lint <mobile>") < output["result"]["html"].index("build")
|
||||
assert 'href="https://forge.example/jobs/4"' in output["result"]["html"]
|
||||
assert 'target="_blank" rel="noopener noreferrer"' in output["result"]["html"]
|
||||
assert "Open job" in output["result"]["html"]
|
||||
assert output["offline"]["summary"].startswith("Last known · ")
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_mobile_pull_review_sheets_expose_actionable_refreshable_checks():
|
||||
html = await dashboard()
|
||||
|
||||
assert html.index('id="pull-checks"') < html.index('id="pull-files"')
|
||||
assert html.index('id="review-checks"') < html.index('id="review-files"')
|
||||
assert 'id="refresh-pull-checks"' in html
|
||||
assert 'id="refresh-review-checks"' in html
|
||||
assert "createReviewController.renderChecks" in html
|
||||
assert "{ refresh: true }" in html
|
||||
assert "qs('#refresh-review-checks').disabled = offlineReview" in html
|
||||
assert ".ci-check-link" in html and "min-height:44px" in html
|
||||
assert ".ci-check-copy" in html and "overflow-wrap:anywhere" in html
|
||||
|
||||
|
||||
def test_review_wrap_preference_defaults_to_phone_layout_and_persists_explicit_choice():
|
||||
script = f"""
|
||||
const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
|
||||
|
|
|
|||
|
|
@ -36,6 +36,57 @@ async def test_review_detail_endpoint_returns_normalized_no_store_payload(monkey
|
|||
assert response.json()["files"][0]["filename"] == "src/api.py"
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_gitea_review_detail_returns_bounded_actionable_checks(monkeypatch):
|
||||
monkeypatch.setattr(gitea_proxy, "GITEA_URL", "https://forge.example/git")
|
||||
|
||||
async def handler(request):
|
||||
path = request.url.path
|
||||
if path.endswith("/pulls/7"):
|
||||
return httpx.Response(200, json={
|
||||
"title": "Review API", "head": {"sha": "abc123"},
|
||||
"user": {"login": "alex"},
|
||||
})
|
||||
if path.endswith("/pulls/7/files") or path.endswith("/pulls/7/reviews"):
|
||||
return httpx.Response(200, json=[])
|
||||
if path.endswith("/commits/abc123/status"):
|
||||
return httpx.Response(200, json={
|
||||
"state": "failure",
|
||||
"statuses": [
|
||||
{
|
||||
"context": "lint/<unsafe>", "status": "failure",
|
||||
"description": "Formatting failed " + "x" * 300,
|
||||
"target_url": "https://forge.example/git/stackchain/api/actions/runs/9/jobs/4",
|
||||
},
|
||||
{
|
||||
"context": "external", "status": "pending",
|
||||
"description": "Waiting", "target_url": "https://evil.example/job/1",
|
||||
},
|
||||
{"context": "build-release", "status": "success", "description": "Passed"},
|
||||
] + [{"context": f"extra-{index}", "status": "success"} for index in range(30)],
|
||||
})
|
||||
if path.endswith("/pulls/7.diff"):
|
||||
return httpx.Response(200, text="")
|
||||
raise AssertionError(f"unexpected request: {request.method} {path}")
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
detail = await gitea_proxy.pull_review_detail("stackchain/api", 7)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert len(detail["checks"]) == 20
|
||||
assert detail["checks"][:3] == [
|
||||
{
|
||||
"name": "lint/<unsafe>", "state": "failure",
|
||||
"description": "Formatting failed " + "x" * 222,
|
||||
"url": "https://forge.example/git/stackchain/api/actions/runs/9/jobs/4",
|
||||
},
|
||||
{"name": "external", "state": "pending", "description": "Waiting", "url": ""},
|
||||
{"name": "build-release", "state": "success", "description": "Passed", "url": ""},
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_review_detail_rejects_pulls_not_requested_from_service_user(monkeypatch):
|
||||
async def requested(repository, number):
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user