refactor: remove negative scratch assertions and rename child dir to stages/

Remove test assertions that verified legacy files (final.patch,
workflow_bundle.json, manifest.json, cache/artifacts/values/) do not
exist in scratch directories — these are a test smell since the code
that wrote them is long gone.

Also rename child workflow scratch path from nodes/{id}_{visit}/child
to stages/{id}@{visit}/child to align with stage_id convention.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-08 16:23:02 -04:00
parent a54e27f42e
commit 13bcac88dc
No known key found for this signature in database
6 changed files with 6 additions and 194 deletions

View file

@ -239,16 +239,6 @@ mod tests {
assert!(scratch.worktree_dir().exists());
assert!(scratch.runtime_dir().exists());
assert!(scratch.artifact_files_dir().exists());
assert!(
!scratch
.root()
.join("cache")
.join("artifacts")
.join("values")
.exists()
);
assert!(!scratch.root().join("final.patch").exists());
scratch.remove().unwrap();
assert!(!scratch.root().exists());
}

View file

@ -5934,32 +5934,6 @@ mod tests {
run_id
}
async fn create_direct_run(state: &Arc<AppState>, settings: &Settings) -> RunId {
operations::create(
state.store.as_ref(),
operations::CreateRunInput {
workflow: operations::WorkflowInput::DotSource {
source: MINIMAL_DOT.to_string(),
base_dir: None,
},
settings: settings.clone(),
cwd: PathBuf::from("/tmp"),
workflow_slug: None,
workflow_path: None,
workflow_bundle: None,
submitted_manifest_bytes: None,
run_id: None,
host_repo_path: None,
repo_origin_url: None,
base_branch: None,
provenance: None,
},
)
.await
.unwrap()
.run_id
}
async fn create_durable_run_with_events(
state: &Arc<AppState>,
run_id: RunId,
@ -6535,15 +6509,9 @@ mod tests {
assert_eq!(accepted_definition["workflow_path"], "workflow.fabro");
assert!(accepted_definition["workflows"]["workflow.fabro"].is_object());
let run_dir = PathBuf::from(
created["properties"]["run_dir"]
.as_str()
.expect("run.created should include run_dir"),
);
assert!(
!run_dir.join("workflow_bundle.json").exists(),
"run scratch should no longer persist workflow_bundle.json"
);
created["properties"]["run_dir"]
.as_str()
.expect("run.created should include run_dir");
}
#[tokio::test]
@ -6863,86 +6831,6 @@ mod tests {
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
}
#[tokio::test]
async fn api_created_runs_do_not_fallback_to_scratch_artifacts() {
let temp = tempfile::tempdir().unwrap();
let mut settings = dry_run_settings();
settings.storage_dir = Some(temp.path().join("storage"));
let state = create_app_state_with_options(settings.clone(), 5);
let app = build_router(Arc::clone(&state), AuthMode::Disabled);
let run_id = create_run(&app, MINIMAL_DOT)
.await
.parse::<RunId>()
.unwrap();
let artifact_path = Storage::new(settings.storage_dir())
.run_scratch(&run_id)
.artifact_files_dir()
.join("code")
.join("retry_2")
.join("src/lib.rs");
std::fs::create_dir_all(artifact_path.parent().unwrap()).unwrap();
std::fs::write(&artifact_path, "legacy scratch only").unwrap();
let req = Request::builder()
.method("GET")
.uri(api(&format!("/runs/{run_id}/stages/code@2/artifacts")))
.body(Body::empty())
.unwrap();
let response = app.clone().oneshot(req).await.unwrap();
assert_eq!(response.status(), StatusCode::OK);
let body = body_json(response.into_body()).await;
assert_eq!(body["data"].as_array().unwrap().len(), 0);
let req = Request::builder()
.method("GET")
.uri(api(&format!(
"/runs/{run_id}/stages/code@2/artifacts/download?filename=src/lib.rs"
)))
.body(Body::empty())
.unwrap();
let response = app.oneshot(req).await.unwrap();
assert_eq!(response.status(), StatusCode::NOT_FOUND);
}
#[tokio::test]
async fn directly_created_runs_do_not_fallback_to_scratch_artifacts() {
let temp = tempfile::tempdir().unwrap();
let mut settings = dry_run_settings();
settings.storage_dir = Some(temp.path().join("storage"));
let state = create_app_state_with_options(settings.clone(), 5);
let app = build_router(Arc::clone(&state), AuthMode::Disabled);
let run_id = create_direct_run(&state, &settings).await;
let artifact_path = Storage::new(settings.storage_dir())
.run_scratch(&run_id)
.artifact_files_dir()
.join("code")
.join("retry_2")
.join("src/lib.rs");
std::fs::create_dir_all(artifact_path.parent().unwrap()).unwrap();
std::fs::write(&artifact_path, "legacy scratch only").unwrap();
let req = Request::builder()
.method("GET")
.uri(api(&format!("/runs/{run_id}/stages/code@2/artifacts")))
.body(Body::empty())
.unwrap();
let response = app.clone().oneshot(req).await.unwrap();
assert_eq!(response.status(), StatusCode::OK);
let body = body_json(response.into_body()).await;
assert_eq!(body["data"].as_array().unwrap().len(), 0);
let req = Request::builder()
.method("GET")
.uri(api(&format!(
"/runs/{run_id}/stages/code@2/artifacts/download?filename=src/lib.rs"
)))
.body(Body::empty())
.unwrap();
let response = app.oneshot(req).await.unwrap();
assert_eq!(response.status(), StatusCode::NOT_FOUND);
}
#[tokio::test]
async fn create_run_returns_submitted() {
let state = create_app_state();

View file

@ -194,7 +194,7 @@ impl Handler for SubWorkflowHandler {
// Build child RunOptions
let visit = visit_from_context(context) as u64;
let child_logs = run_dir.join(format!("nodes/{}_{visit}/child", node.id));
let child_logs = run_dir.join(format!("stages/{}@{visit}/child", node.id));
let _ = std::fs::create_dir_all(&child_logs);
let cancel_token = Arc::new(AtomicBool::new(false));
@ -399,7 +399,7 @@ mod tests {
.contains("Child completed")
);
assert!(
dir.path().join("nodes/manager_1/child").exists(),
dir.path().join("stages/manager@1/child").exists(),
"child logs should default to first-visit directory naming"
);
}

View file

@ -1054,12 +1054,6 @@ mod tests {
.await
.unwrap();
let bundle_file = created.run_dir.join("workflow_bundle.json");
assert!(
!bundle_file.exists(),
"run scratch should not persist workflow_bundle.json"
);
let started = start(
&run_dir,
test_start_services(&store, &run_dir, emitter, registry).await,

View file

@ -728,13 +728,6 @@ async fn daytona_git_checkpoint_remote_emits_events() {
"checkpoint should have git_commit_sha"
);
// Assert scratch final.patch is no longer written
let final_patch = dir.path().join("final.patch");
assert!(
!final_patch.exists(),
"final.patch should not be written to scratch"
);
env.cleanup().await.unwrap();
}
@ -1243,13 +1236,6 @@ async fn daytona_git_checkpoint_with_shadow_branch() {
"sandbox commit should have Fabro-Run trailer, got:\n{commit_msg}"
);
// Assert scratch final.patch is no longer written
let final_patch = dir.path().join("final.patch");
assert!(
!final_patch.exists(),
"final.patch should not be written to scratch"
);
env.cleanup().await.unwrap();
}
@ -1367,9 +1353,6 @@ async fn daytona_asset_collection() {
let content = std::fs::read_to_string(&report_path).unwrap();
assert!(content.contains("testsuites"));
let manifest_path = artifacts_dir.join("manifest.json");
assert!(!manifest_path.exists(), "manifest.json should not exist");
env.cleanup().await.unwrap();
}

View file

@ -8665,17 +8665,6 @@ async fn large_context_values_are_offloaded_to_artifact_store() {
"value should be a durable blob ref"
);
assert!(
!RunScratch::new(dir.path())
.root()
.join("cache")
.join("artifacts")
.join("values")
.join(format!("{expected_blob_id}.json"))
.exists(),
"legacy host blob cache file should not exist"
);
// WorkflowRunCompleted artifact_count now tracks captured artifacts, not offloaded values.
let evts = events.lock().unwrap();
let completed_event = evts
@ -10387,13 +10376,6 @@ async fn git_checkpoint_host_emits_events_and_diff_patch() {
"checkpoint should have git_commit_sha"
);
// 9. Assert scratch final.patch is no longer written
let final_patch = run_dir.path().join("final.patch");
assert!(
!final_patch.exists(),
"final.patch should not be written to scratch"
);
// Cleanup worktree
let _ = std::process::Command::new("git")
.args(["worktree", "remove", "--force"])
@ -10828,14 +10810,7 @@ async fn parallel_git_branching_host_e2e() {
"parallel branch ref should still exist for debugging"
);
// 11. Verify scratch final.patch is no longer written
let final_patch = run_dir.path().join("final.patch");
assert!(
!final_patch.exists(),
"final.patch should not be written to scratch"
);
// 12. Verify events
// 11. Verify events
let events = events.lock().unwrap();
let parallel_started: Vec<_> = events
.iter()
@ -10972,13 +10947,6 @@ async fn git_checkpoint_host_skips_empty_diff_patch() {
.expect("pipeline should succeed");
assert_eq!(outcome.status, StageStatus::Success);
// final.patch should NOT exist either
let final_patch = run_dir.path().join("final.patch");
assert!(
!final_patch.exists(),
"final.patch should not exist when there are no changes"
);
// Cleanup
let _ = std::process::Command::new("git")
.args(["worktree", "remove", "--force"])
@ -12683,14 +12651,6 @@ async fn asset_collection_local_sandbox_success() {
let report_content = std::fs::read_to_string(&report_path).unwrap();
assert!(report_content.contains("testsuites"));
// Check manifest.json is no longer written
let manifest_path = artifacts_dir.join("manifest.json");
assert!(
!manifest_path.exists(),
"manifest.json should not exist at {}",
manifest_path.display()
);
// Check that ArtifactCaptured events were emitted
let captured_events = events.lock().unwrap();
let asset_events: Vec<&RunEvent> = captured_events
@ -12889,9 +12849,6 @@ async fn asset_collection_docker_sandbox() {
let content = std::fs::read_to_string(&report_path).unwrap();
assert!(content.contains("testsuites"));
let manifest_path = artifacts_dir.join("manifest.json");
assert!(!manifest_path.exists(), "manifest.json should not exist");
sandbox.cleanup().await.unwrap();
}