Skip to content

Commit 84d4840

Browse files
committed
fix(tests): make CI deterministic — hermetic fixtures + serial test run --patch
The remaining CI reds were environment/parallelism issues invisible on a local macOS run: - import_config: the test set the process-global HOME to point build_import_preview at a fixture. That doesn't redirect dirs::home_dir() on Windows (Known-Folder API) AND raced with every parallel test reading the home dir. Added an injected-paths variant (build_import_preview_with_paths) so the test stages a fixture with zero env mutation — works on all platforms. - stats fixtures: wrote each JSONL line via the async write_transcript_entry, then read back synchronously — a write/read visibility gap dropped trailing lines on the loaded Linux runner (flaking turn counts / last-prompt). Now written in one synchronous std::fs::write. - ci.yml: run tests with --test-threads=1. Several tests across the suite mutate global env (HOME, ANTHROPIC_API_KEY, …); serial execution removes the parallel races deterministically. Verified: cargo test --workspace --no-fail-fast -- --test-threads=1 → 30/30 binaries green (incl doctests); release build clean, 0 warnings. Ships as a patch to v0.1.6.
1 parent c3c796d commit 84d4840

3 files changed

Lines changed: 49 additions & 18 deletions

File tree

.github/workflows/ci.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,10 @@ jobs:
5858

5959
- name: Run tests
6060
working-directory: src-rust
61-
run: cargo test --workspace --locked
61+
# Run serially: a number of tests mutate process-global state (env vars
62+
# like HOME / ANTHROPIC_API_KEY, etc.) and would otherwise race under
63+
# parallel execution, flaking non-deterministically across runners.
64+
run: cargo test --workspace --locked -- --test-threads=1
6265

6366
# Advisory for now: the codebase has pre-existing clippy warnings and
6467
# rustfmt drift. Flip continue-on-error off once they are cleaned up.

src-rust/crates/commands/src/stats.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,8 +1216,7 @@ pub fn run(raw: &[&str], ctx: &CommandContext) -> CommandResult {
12161216
mod tests {
12171217
use super::*;
12181218
use claurst_core::session_storage::{
1219-
write_transcript_entry, AiTitleEntry, CustomTitleEntry, LastPromptEntry,
1220-
TranscriptMessage,
1219+
AiTitleEntry, CustomTitleEntry, LastPromptEntry, TranscriptMessage,
12211220
};
12221221
use claurst_core::types::{Message, MessageContent, MessageCost, Role};
12231222
use tempfile::TempDir;
@@ -1292,9 +1291,19 @@ mod tests {
12921291
entries: Vec<TranscriptEntry>,
12931292
) -> PathBuf {
12941293
let path = dir.join(format!("{session_id}.jsonl"));
1295-
for e in entries {
1296-
write_transcript_entry(&path, &e).await.unwrap();
1294+
// Write the whole fixture synchronously in one shot. The reader
1295+
// (`aggregate_from_dir` / `parse_jsonl_sync`) uses blocking `std::fs`,
1296+
// so writing each line via the async `write_transcript_entry`
1297+
// (open/append/close per entry on the blocking pool) left a
1298+
// write-then-read visibility gap that dropped trailing lines on the
1299+
// loaded Linux CI runner — non-deterministically failing the turn count
1300+
// and last-prompt assertions. A single sync write removes that race.
1301+
let mut buf = String::new();
1302+
for e in &entries {
1303+
buf.push_str(&serde_json::to_string(e).unwrap());
1304+
buf.push('\n');
12971305
}
1306+
std::fs::write(&path, buf).unwrap();
12981307
path
12991308
}
13001309

src-rust/crates/core/src/import_config.rs

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,18 @@ pub fn build_import_preview(selection: ImportSelection) -> Result<ImportPreview>
124124
Ok(prepare_import(selection)?.preview)
125125
}
126126

127+
/// Like [`build_import_preview`] but against an explicit [`ImportPaths`] instead
128+
/// of the detected home directory. Lets tests stage a fixture without mutating
129+
/// the process-global `HOME` env var (which races with every other test that
130+
/// reads the home directory in parallel).
131+
#[cfg(test)]
132+
pub(crate) fn build_import_preview_with_paths(
133+
selection: ImportSelection,
134+
paths: ImportPaths,
135+
) -> Result<ImportPreview> {
136+
Ok(prepare_import_with_paths(selection, paths)?.preview)
137+
}
138+
127139
pub fn execute_import(selection: ImportSelection) -> Result<ImportExecutionResult> {
128140
let prepared = prepare_import(selection)?;
129141
let paths = ImportPaths::detect();
@@ -193,7 +205,13 @@ pub fn summarize_import_result(result: &ImportExecutionResult, paths: &ImportPat
193205
}
194206

195207
fn prepare_import(selection: ImportSelection) -> Result<PreparedImport> {
196-
let paths = ImportPaths::detect();
208+
prepare_import_with_paths(selection, ImportPaths::detect())
209+
}
210+
211+
fn prepare_import_with_paths(
212+
selection: ImportSelection,
213+
paths: ImportPaths,
214+
) -> Result<PreparedImport> {
197215
let mut preview = ImportPreview {
198216
selection,
199217
claude_md: None,
@@ -857,10 +875,13 @@ mod tests {
857875

858876
#[test]
859877
fn build_import_preview_maps_settings_and_doc() {
878+
// Stage the fixture against an explicit ImportPaths instead of mutating
879+
// the process-global HOME env var. The old approach set_var("HOME"),
880+
// which (a) doesn't redirect dirs::home_dir() on Windows and (b) raced
881+
// with every other test reading the home dir in parallel.
860882
let tmp = TempDir::new().unwrap();
861-
let home = tmp.path();
862-
let claude_dir = home.join(".claude");
863-
let claurst_dir = home.join(".claurst");
883+
let claude_dir = tmp.path().join(".claude");
884+
let claurst_dir = tmp.path().join(".claurst");
864885
std::fs::create_dir_all(&claude_dir).unwrap();
865886
std::fs::create_dir_all(&claurst_dir).unwrap();
866887
std::fs::write(claude_dir.join("CLAUDE.md"), "hello\nworld").unwrap();
@@ -883,22 +904,20 @@ mod tests {
883904
)
884905
.unwrap();
885906

886-
let old_home = std::env::var("HOME").ok();
887-
std::env::set_var("HOME", home);
907+
let paths = ImportPaths {
908+
source_claude_md: claude_dir.join("CLAUDE.md"),
909+
source_settings_json: claude_dir.join("settings.json"),
910+
target_claude_md: claurst_dir.join("CLAUDE.md"),
911+
target_settings_json: claurst_dir.join("settings.json"),
912+
};
888913

889-
let preview = build_import_preview(ImportSelection::Both).unwrap();
914+
let preview = build_import_preview_with_paths(ImportSelection::Both, paths).unwrap();
890915
assert!(preview.claude_md.is_some());
891916
let settings = preview.settings.unwrap();
892917
assert!(settings.fields.iter().any(|f| f.name == "model" && f.action == PreviewAction::Skip));
893918
assert!(settings.fields.iter().any(|f| f.name == "theme"));
894919
assert!(settings.fields.iter().any(|f| f.name.starts_with("hooks")));
895920
assert!(settings.fields.iter().any(|f| f.name.starts_with("mcpServers")));
896921
assert!(settings.fields.iter().any(|f| f.name == "env" && f.action == PreviewAction::Skip));
897-
898-
if let Some(old) = old_home {
899-
std::env::set_var("HOME", old);
900-
} else {
901-
std::env::remove_var("HOME");
902-
}
903922
}
904923
}

0 commit comments

Comments
 (0)