mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-11-02 07:12:34 +00:00
fix(cli): show the source branch in the Base header
This commit is contained in:
parent
d8a43b13f4
commit
b2b3140df0
7 changed files with 153 additions and 12 deletions
|
|
@ -218,7 +218,7 @@ async fn attach_petri_run_with_client(
|
|||
printer: Printer,
|
||||
) -> Result<ExitCode> {
|
||||
let is_tty = std::io::stderr().is_terminal();
|
||||
let mut progress_ui = run_progress::ProgressUI::new(is_tty, opts.verbose);
|
||||
let mut progress_ui = run_progress::ProgressUI::new(is_tty, opts.verbose, &state.spec);
|
||||
let ctrl_c_signal = ctrl_c();
|
||||
tokio::pin!(ctrl_c_signal);
|
||||
|
||||
|
|
|
|||
|
|
@ -27,7 +27,6 @@ pub(super) enum ProgressEvent {
|
|||
},
|
||||
WorkflowStarted {
|
||||
worktree_dir: Option<String>,
|
||||
base_branch: Option<String>,
|
||||
base_sha: Option<String>,
|
||||
},
|
||||
StageStarted {
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@
|
|||
reason = "sync CLI run-progress renderer: writes to std::io::stderr directly"
|
||||
)]
|
||||
|
||||
use fabro_types::{RunNoticeCode, RunStreamItem};
|
||||
use fabro_types::{RunNoticeCode, RunSpec, RunStreamItem, RunTarget};
|
||||
|
||||
mod event;
|
||||
mod info_display;
|
||||
|
|
@ -26,10 +26,11 @@ pub(crate) struct ProgressUI {
|
|||
info: InfoDisplay,
|
||||
saw_metadata_snapshot_failure: bool,
|
||||
petri: PetriProgressState,
|
||||
source_branch: Option<String>,
|
||||
}
|
||||
|
||||
impl ProgressUI {
|
||||
pub(crate) fn new(is_tty: bool, verbose: bool) -> Self {
|
||||
pub(crate) fn new(is_tty: bool, verbose: bool, spec: &RunSpec) -> Self {
|
||||
let renderer = if is_tty {
|
||||
ProgressRenderer::new_tty()
|
||||
} else {
|
||||
|
|
@ -38,7 +39,7 @@ impl ProgressUI {
|
|||
console::colors_enabled_stderr(),
|
||||
)
|
||||
};
|
||||
Self::with_renderer(renderer, verbose)
|
||||
Self::with_renderer(renderer, verbose).with_source_spec(spec)
|
||||
}
|
||||
|
||||
fn with_renderer(renderer: ProgressRenderer, verbose: bool) -> Self {
|
||||
|
|
@ -48,9 +49,25 @@ impl ProgressUI {
|
|||
info: InfoDisplay::new(verbose),
|
||||
saw_metadata_snapshot_failure: false,
|
||||
petri: PetriProgressState::default(),
|
||||
source_branch: None,
|
||||
}
|
||||
}
|
||||
|
||||
fn with_source_spec(mut self, spec: &RunSpec) -> Self {
|
||||
// A fork starts at a checkpoint, and a tag need not name the input
|
||||
// branch's history. Keep those bases commit-only instead of pairing
|
||||
// the inherited working branch with an unrelated revision.
|
||||
if spec.fork_source_ref.is_none()
|
||||
&& !matches!(&spec.target, Some(RunTarget::Git(target)) if target.tag.is_some())
|
||||
{
|
||||
self.source_branch = spec
|
||||
.base_branch()
|
||||
.filter(|branch| !branch.trim().is_empty())
|
||||
.map(str::to_owned);
|
||||
}
|
||||
self
|
||||
}
|
||||
|
||||
pub(crate) fn hide_bars(&self) {
|
||||
self.renderer.hide();
|
||||
}
|
||||
|
|
@ -82,14 +99,13 @@ impl ProgressUI {
|
|||
}
|
||||
ProgressEvent::WorkflowStarted {
|
||||
worktree_dir,
|
||||
base_branch,
|
||||
base_sha,
|
||||
} => {
|
||||
if let Some(worktree_dir) = worktree_dir {
|
||||
InfoDisplay::show_worktree(renderer, std::path::Path::new(&worktree_dir));
|
||||
}
|
||||
if let Some(base_sha) = base_sha {
|
||||
InfoDisplay::show_base_info(renderer, base_branch.as_deref(), &base_sha);
|
||||
InfoDisplay::show_base_info(renderer, self.source_branch.as_deref(), &base_sha);
|
||||
}
|
||||
}
|
||||
ProgressEvent::StageStarted {
|
||||
|
|
|
|||
|
|
@ -221,10 +221,6 @@ fn platform_progress_event(record: &Value) -> Option<ProgressEvent> {
|
|||
}),
|
||||
"run.branch" => Some(ProgressEvent::WorkflowStarted {
|
||||
worktree_dir: None,
|
||||
base_branch: record
|
||||
.get("run_branch")
|
||||
.and_then(Value::as_str)
|
||||
.map(str::to_string),
|
||||
base_sha: record
|
||||
.get("base_sha")
|
||||
.and_then(Value::as_str)
|
||||
|
|
|
|||
|
|
@ -6,7 +6,9 @@
|
|||
use std::io::{self, Write};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
use fabro_types::{RunStreamItemKind, fixtures};
|
||||
use fabro_types::{
|
||||
DirtyStatus, ForkSourceRef, GitContext, GitRunTarget, RunStreamItemKind, fixtures, test_support,
|
||||
};
|
||||
use fabro_util::terminal::Styles;
|
||||
use serde_json::{Value, json};
|
||||
|
||||
|
|
@ -203,3 +205,82 @@ fn skipped_stage_is_dim_in_both_renderers_and_visible_in_plain_replay_without_a_
|
|||
" ⊘ work skipped 0ms\n"
|
||||
);
|
||||
}
|
||||
|
||||
fn branch_record(base_sha: Option<&str>) -> RunStreamItem {
|
||||
RunStreamItem {
|
||||
run_id: fixtures::RUN_1,
|
||||
stream_seq: 2,
|
||||
kind: RunStreamItemKind::Platform,
|
||||
id: "2".to_string(),
|
||||
recorded_at: 1_789_706_579_000,
|
||||
item: json!({"record": {
|
||||
"kind": "run.branch",
|
||||
"run_branch": format!("fabro/run/{}", fixtures::RUN_1),
|
||||
"base_sha": base_sha
|
||||
}}),
|
||||
}
|
||||
}
|
||||
|
||||
fn source_spec() -> RunSpec {
|
||||
RunSpec {
|
||||
git: Some(GitContext {
|
||||
origin_url: String::new(),
|
||||
branch: "main".to_string(),
|
||||
// The branch-point record, not this creation-time observation,
|
||||
// supplies the SHA displayed next to the source branch.
|
||||
sha: Some("1111111111111111111111111111111111111111".to_string()),
|
||||
dirty: DirtyStatus::Clean,
|
||||
}),
|
||||
..test_support::test_run_spec()
|
||||
}
|
||||
}
|
||||
|
||||
fn render_base(spec: &RunSpec, base_sha: Option<&str>, verbose: bool) -> String {
|
||||
let output = Output::default();
|
||||
let renderer = ProgressRenderer::new_plain(Box::new(output.clone()), false);
|
||||
let mut ui = ProgressUI::with_renderer(renderer, verbose).with_source_spec(spec);
|
||||
let record = branch_record(base_sha);
|
||||
let raw = petri_stream::raw_line(&record).unwrap();
|
||||
ui.handle_stream_item(&record);
|
||||
ui.finish();
|
||||
assert_eq!(petri_stream::raw_line(&record).unwrap(), raw);
|
||||
output.text()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn base_header_uses_saved_source_branch_and_recorded_base_commit() {
|
||||
let spec = source_spec();
|
||||
for verbose in [false, true] {
|
||||
insta::allow_duplicates! {
|
||||
insta::assert_snapshot!(render_base(&spec, Some("abcdef0123456789"), verbose), @" Base: main (abcdef012345)");
|
||||
}
|
||||
assert_eq!(render_base(&spec, None, verbose), "");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn base_header_uses_only_the_commit_when_source_branch_is_unknown_or_ambiguous() {
|
||||
let mut absent = source_spec();
|
||||
absent.git = None;
|
||||
let mut empty = source_spec();
|
||||
empty.git.as_mut().unwrap().branch = " ".to_string();
|
||||
let mut fork = source_spec();
|
||||
fork.fork_source_ref = Some(ForkSourceRef {
|
||||
source_run_id: fixtures::RUN_2,
|
||||
checkpoint_sha: "abcdef0123456789".to_string(),
|
||||
});
|
||||
let mut tag = source_spec();
|
||||
tag.target = Some(RunTarget::Git(GitRunTarget {
|
||||
repo: "acme/widgets".to_string(),
|
||||
branch: "main".to_string(),
|
||||
tag: Some("v1".to_string()),
|
||||
sha: None,
|
||||
}));
|
||||
for spec in [absent, empty, fork, tag] {
|
||||
for verbose in [false, true] {
|
||||
insta::allow_duplicates! {
|
||||
insta::assert_snapshot!(render_base(&spec, Some("abcdef0123456789"), verbose), @" Base: abcdef012345");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2905,3 +2905,51 @@ fn attach_json_errors_without_prompting_for_human_input() {
|
|||
});
|
||||
wait_for_status(&run.run_dir, &["succeeded"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn attach_base_header_preserves_the_saved_source_branch() {
|
||||
let context = test_context!();
|
||||
let workflow = super::support::write_workflow(&context.temp_dir, "base-header", "BaseHeader");
|
||||
let workspace = context.temp_dir.join("source");
|
||||
context.write_temp("source/README.md", "source commit\n");
|
||||
let source_sha = super::support::init_remote_fixture(&workspace, "main");
|
||||
let run = context
|
||||
.run_cmd()
|
||||
.args([
|
||||
"--auto-approve",
|
||||
"--detach",
|
||||
"--environment",
|
||||
"local",
|
||||
"--target-from",
|
||||
])
|
||||
.arg(&workspace)
|
||||
.arg(&workflow)
|
||||
.timeout(SHARED_DAEMON_TIMEOUT)
|
||||
.assert()
|
||||
.success();
|
||||
let run_id = created_run_id(run.get_output());
|
||||
context
|
||||
.command()
|
||||
.args(["wait", &run_id])
|
||||
.timeout(SHARED_DAEMON_TIMEOUT)
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
// Reattach must use the stored source, even if the caller changes branches.
|
||||
super::support::run_git(&workspace, &["checkout", "-b", "later"]);
|
||||
let output = context
|
||||
.command()
|
||||
.args(["attach", &run_id])
|
||||
.current_dir(&workspace)
|
||||
.timeout(SHARED_DAEMON_TIMEOUT)
|
||||
.assert()
|
||||
.success();
|
||||
let stderr = String::from_utf8_lossy(&output.get_output().stderr);
|
||||
let base_lines: Vec<_> = stderr
|
||||
.lines()
|
||||
.map(str::trim)
|
||||
.filter(|line| line.starts_with("Base:"))
|
||||
.collect();
|
||||
// Do not apply the shared Base filter: it would hide a wrong branch name.
|
||||
assert_eq!(base_lines, [format!("Base: main ({})", &source_sha[..12])]);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -74,6 +74,7 @@ static INSTA_FILTERS: &[(&str, &str)] = &[
|
|||
"Duration: [DURATION]",
|
||||
),
|
||||
(r"Base: [^\n]+ \([0-9a-f]{7,40}\)", "Base: [BASE]"),
|
||||
(r"Base: [0-9a-f]{7,40}\b", "Base: [BASE]"),
|
||||
(r"(Branch: [^\n]+ from )[0-9a-f]{7,40}", "${1}[SHA]"),
|
||||
// The sandbox driver's events: per-process event source ids, operation
|
||||
// ids, sub-second durations, and a local sandbox's path-derived id.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue