Watch
0
0
Fork
You've already forked hyperhive
0

review try-out: test the branch push request; name the open-PR gating

push_classifier_branch's request moves into push_branch, which takes the
forge client, so a test can run it against a stand-in forge. The tests
pin the POST to /repos/o/r/contents and its body: base and new branch,
no force overwrite, the message, the operator as author with the
service account as committer, the base64 content, and update with the
blob sha or create without one.

The page's open-a-PR gating becomes rowsProblem in TryOutTypes.ts,
beside the other form checks. swarm-ui has no test runner, so the
typecheck is the only check on it.

Refs #5259
This commit is contained in:
atlas 2026-10-14 16:13:57 +02:00 • committed by mara
commit 915364c918
3 changed files with 140 additions and 48 deletions

View file

@ -37,11 +37,10 @@ import { ClassificationEditor } from "./ClassificationEditor.js";
import { TryOutResult } from "./TryOutResult.js";
import {
draftFromRule,
draftProblem,
duplicateName,
hasTypedText,
prNumber,
repoProblem,
rowsProblem,
toBody,
type ClassificationDraft,
type OpenPrAnswer,
@ -218,15 +217,10 @@ export function ReviewTryOutPage() {
};
}, [runId]);
// What keeps the rows from going to the repo; a PR number is not needed.
const rowsProblem =
repoProblem(repo) ??
drafts.map(draftProblem).find((p) => p !== null) ??
duplicateName(drafts) ??
null;
const openPrProblem = rowsProblem(repo, drafts);
const problem =
(prNumber(pr) === null ? "a PR number is a whole number above 0" : null) ??
rowsProblem;
openPrProblem;
async function run(e: Event) {
e.preventDefault();
@ -410,7 +404,7 @@ export function ReviewTryOutPage() {
</Button>
<Button
onClick={() => void openPr()}
disabled={opening || rowsProblem !== null}
disabled={opening || openPrProblem !== null}
>
{opening ? "pushing…" : "open a PR"}
</Button>

View file

@ -150,6 +150,20 @@ export function duplicateName(drafts: ClassificationDraft[]): string | null {
return null;
}
/** What keeps the rows from going to the repo as a PR, or null: the repo, then
* each row, then a shared name. Unlike a run, it needs no PR number. */
export function rowsProblem(
repo: string,
drafts: ClassificationDraft[],
): string | null {
return (
repoProblem(repo) ??
drafts.map(draftProblem).find((p) => p !== null) ??
duplicateName(drafts) ??
null
);
}
export function toBody(d: ClassificationDraft): ClassificationBody {
return {
name: d.name.trim(),

View file

@ -181,44 +181,7 @@ impl Client {
repo: &str,
commit: BranchCommit,
) -> Result<()> {
use base64::Engine as _;
let branch = commit.branch.clone();
self.as_classifier()
.await?
.repo_change_files(
owner,
repo,
ChangeFilesOptions {
author: Some(Identity {
name: Some(commit.author.name),
email: Some(commit.author.email),
}),
branch: Some(commit.base),
committer: None,
dates: None,
files: vec![ChangeFileOperation {
content: Some(
base64::engine::general_purpose::STANDARD.encode(&commit.content),
),
from_path: None,
operation: if commit.sha.is_some() {
ChangeFileOperationOperation::Update
} else {
ChangeFileOperationOperation::Create
},
path: commit.path,
sha: commit.sha,
}],
force_overwrite_new_branch: Some(false),
message: Some(commit.message),
new_branch: Some(commit.branch),
signoff: None,
},
)
.await
.with_context(|| format!("pushing {LOGIN}'s branch {branch} to {owner}/{repo}"))?;
Ok(())
push_branch(&self.as_classifier().await?, owner, repo, commit).await
}
/// The forge account `login` as a commit author: its full name, or its
@ -338,6 +301,47 @@ impl Client {
}
}
/// Make `commit` with `forge`'s identity as committer, through the contents
/// API.
async fn push_branch(forge: &Forgejo, owner: &str, repo: &str, commit: BranchCommit) -> Result<()> {
use base64::Engine as _;
let branch = commit.branch.clone();
forge
.repo_change_files(
owner,
repo,
ChangeFilesOptions {
author: Some(Identity {
name: Some(commit.author.name),
email: Some(commit.author.email),
}),
branch: Some(commit.base),
committer: None,
dates: None,
files: vec![ChangeFileOperation {
content: Some(
base64::engine::general_purpose::STANDARD.encode(&commit.content),
),
from_path: None,
operation: if commit.sha.is_some() {
ChangeFileOperationOperation::Update
} else {
ChangeFileOperationOperation::Create
},
path: commit.path,
sha: commit.sha,
}],
force_overwrite_new_branch: Some(false),
message: Some(commit.message),
new_branch: Some(commit.branch),
signoff: None,
},
)
.await
.with_context(|| format!("pushing {LOGIN}'s branch {branch} to {owner}/{repo}"))?;
Ok(())
}
/// Check the account now and every [`RECONCILE_INTERVAL`] after, and call
/// `enqueue`, which inserts a `MintClassifierForgeToken` node, when it needs
/// a job. A pass that fails is logged and retried on the next tick.
@ -419,6 +423,86 @@ mod tests {
assert!(lockdown_patch(&seen, LOGIN).is_none());
}
/// The one request `push_branch` sends for `sha`, as a stand-in forge
/// saw it: method, path and JSON body.
async fn pushed(sha: Option<&str>) -> (String, String, serde_json::Value) {
use axum::http::{Method, StatusCode, Uri, header::CONTENT_TYPE};
let seen = Arc::new(std::sync::Mutex::new(Vec::new()));
let recorded = Arc::clone(&seen);
let app = axum::Router::new().fallback(
move |method: Method, uri: Uri, body: axum::body::Bytes| {
let recorded = Arc::clone(&recorded);
async move {
recorded.lock().unwrap().push((
method.to_string(),
uri.path().to_owned(),
serde_json::from_slice(&body).unwrap_or_default(),
));
(
StatusCode::CREATED,
[(CONTENT_TYPE, "application/json")],
r#"{"commit":{"sha":"c1"}}"#,
)
}
},
);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let url = url::Url::parse(&format!("http://{}/", listener.local_addr().unwrap())).unwrap();
tokio::spawn(async move { axum::serve(listener, app).await });
let forge = Forgejo::new(Auth::Token("stub-token"), url).unwrap();
let commit = BranchCommit {
base: "main".to_owned(),
branch: "review-classifier/alice-0a1b2c3d".to_owned(),
path: ".hyperhive/review-classifier.yml".to_owned(),
sha: sha.map(str::to_owned),
content: b"classifications: []\n".to_vec(),
message: "review classifier: add no-ads".to_owned(),
author: CommitAuthor {
name: "Alice".to_owned(),
email: "alice@example.test".to_owned(),
},
};
push_branch(&forge, "o", "r", commit).await.expect("pushed");
let mut seen = seen.lock().unwrap();
assert_eq!(seen.len(), 1, "{seen:?}");
seen.remove(0)
}
#[tokio::test]
async fn a_push_is_one_commit_on_a_new_branch_authored_by_the_operator() {
let (method, path, body) = pushed(Some("b1")).await;
assert_eq!(method, "POST");
assert_eq!(path, "/api/v1/repos/o/r/contents");
assert_eq!(body["branch"], "main");
assert_eq!(body["new_branch"], "review-classifier/alice-0a1b2c3d");
assert_eq!(body["force_overwrite_new_branch"], false);
assert_eq!(body["message"], "review classifier: add no-ads");
assert_eq!(
body["author"],
serde_json::json!({ "name": "Alice", "email": "alice@example.test" })
);
assert_eq!(body["committer"], serde_json::Value::Null);
assert_eq!(
body["files"],
serde_json::json!([{
"operation": "update",
"path": ".hyperhive/review-classifier.yml",
"sha": "b1",
// base64 of "classifications: []\n"
"content": "Y2xhc3NpZmljYXRpb25zOiBbXQo=",
"from_path": null,
}])
);
}
#[tokio::test]
async fn a_push_without_a_blob_sha_creates_the_file() {
let (_, _, body) = pushed(None).await;
assert_eq!(body["files"][0]["operation"], "create");
assert_eq!(body["files"][0]["sha"], serde_json::Value::Null);
}
#[tokio::test]
async fn a_commit_author_is_the_accounts_name_and_email_or_none() {
use super::super::tests::stub_forge;