Skip to content

Commit 5c3ffd4

Browse files
Merge pull request #475 from ScriptedAlchemy/codex/windows-integrity-followup
fix(tests): harden Windows daemon fixtures
2 parents 5e88523 + 2312244 commit 5c3ffd4

7 files changed

Lines changed: 59 additions & 43 deletions

File tree

src/branch/admin/tests.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ fn metadata_commit_before_deleted_promotion_recovers_as_committed() {
454454
assert!(crate::db::database_path_is_tombstoned(&db).unwrap());
455455
}
456456

457+
#[cfg(unix)]
457458
#[test]
458459
fn committed_recovery_syncs_metadata_before_tombstone_transition() {
459460
let (_temp, project_root, tracedecay_dir) = fixture();
@@ -549,6 +550,7 @@ fn orphan_commit_before_deleted_promotion_recovers_as_committed() {
549550
assert!(crate::db::database_path_is_tombstoned(&orphan).unwrap());
550551
}
551552

553+
#[cfg(unix)]
552554
#[test]
553555
fn committed_recovery_syncs_store_directory_before_tombstone_transition() {
554556
let (_temp, project_root, tracedecay_dir) = fixture();

src/branch/admin/transaction.rs

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -241,19 +241,7 @@ where
241241
if expected_present {
242242
require_regular_file(&source, "branch store family member")?;
243243
require_missing(&quarantine, "branch deletion quarantine")?;
244-
std::fs::rename(&source, &quarantine).map_err(|error| {
245-
config_error(format!(
246-
"failed to quarantine branch store file '{}' as '{}': {error}",
247-
source.display(),
248-
quarantine.display()
249-
))
250-
})?;
251-
sync_directory(source.parent().ok_or_else(|| {
252-
config_error(format!(
253-
"branch store path '{}' has no parent",
254-
source.display()
255-
))
256-
})?)?;
244+
move_file_durably(&source, &quarantine, "branch store quarantine")?;
257245
moved += 1;
258246
hook(TransactionPhase::AfterMove(moved))?;
259247
} else {
@@ -590,6 +578,24 @@ fn quarantine_family_paths(database: &Path, transaction_id: &str) -> Result<[Pat
590578
Ok([database, PathBuf::from(wal), PathBuf::from(shm)])
591579
}
592580

581+
fn move_file_durably(source: &Path, destination: &Path, record_name: &str) -> Result<()> {
582+
crate::db::DatabaseAuthority::replace_file_atomically(source, destination, record_name)
583+
.map_err(|error| {
584+
config_error(format!(
585+
"failed to move '{}' to '{}' for {record_name}: {error}",
586+
source.display(),
587+
destination.display()
588+
))
589+
})?;
590+
let parent = destination.parent().ok_or_else(|| {
591+
config_error(format!(
592+
"branch store path '{}' has no parent",
593+
destination.display()
594+
))
595+
})?;
596+
sync_directory(parent)
597+
}
598+
593599
fn rollback_files(tracedecay_dir: &Path, journal: &DeletionJournal) -> Result<()> {
594600
let states = journal
595601
.entries
@@ -615,19 +621,7 @@ fn rollback_files(tracedecay_dir: &Path, journal: &DeletionJournal) -> Result<()
615621
for family in states.into_iter().rev() {
616622
for (source, quarantine, expected_present) in family.into_iter().rev() {
617623
if expected_present && quarantine.exists() {
618-
std::fs::rename(&quarantine, &source).map_err(|error| {
619-
config_error(format!(
620-
"failed to restore quarantined branch store '{}' to '{}': {error}",
621-
quarantine.display(),
622-
source.display()
623-
))
624-
})?;
625-
sync_directory(source.parent().ok_or_else(|| {
626-
config_error(format!(
627-
"branch store path '{}' has no parent",
628-
source.display()
629-
))
630-
})?)?;
624+
move_file_durably(&quarantine, &source, "branch store quarantine rollback")?;
631625
}
632626
}
633627
}

src/global_db/tests.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,13 +166,16 @@ async fn try_open_at_preserves_authority_error() {
166166
"{message}"
167167
);
168168
assert!(message.contains("open global database"), "{message}");
169+
let displayed = path.display().to_string();
169170
#[cfg(windows)]
170171
assert!(
171-
message.contains(&format!(r"\\?\{}", path.display())),
172+
message
173+
.replace('\\', "/")
174+
.contains(&displayed.replace('\\', "/")),
172175
"{message}"
173176
);
174177
#[cfg(not(windows))]
175-
assert!(message.contains(&path.display().to_string()), "{message}");
178+
assert!(message.contains(&displayed), "{message}");
176179
}
177180

178181
#[tokio::test]

src/tracedecay/locking.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,7 @@ fn is_pid_alive(pid: u32) -> bool {
466466
mod tests {
467467
use super::*;
468468

469+
#[cfg(not(windows))]
469470
fn legacy_parser_classifies_stale(path: &Path) -> bool {
470471
std::fs::read_to_string(path)
471472
.ok()
@@ -474,16 +475,20 @@ mod tests {
474475
}
475476

476477
#[test]
477-
fn live_new_owner_is_not_stale_to_legacy_pid_parser() {
478+
fn live_new_owner_blocks_legacy_create_and_new_lockers() {
478479
let dir = tempfile::tempdir().unwrap();
479480
let path = dir.path().join("sync.lock");
480481
let guard = try_acquire_sync_lock_at(&path).unwrap();
481482

482-
assert!(!legacy_parser_classifies_stale(&path));
483-
assert_eq!(
484-
std::fs::read_to_string(&path).unwrap(),
485-
std::process::id().to_string()
486-
);
483+
assert!(is_pid_alive(std::process::id()));
484+
#[cfg(not(windows))]
485+
{
486+
assert!(!legacy_parser_classifies_stale(&path));
487+
assert_eq!(
488+
std::fs::read_to_string(&path).unwrap(),
489+
std::process::id().to_string()
490+
);
491+
}
487492
let legacy_create = OpenOptions::new().write(true).create_new(true).open(&path);
488493
assert_eq!(
489494
legacy_create.unwrap_err().kind(),

tests/core_cli_suite/cli_non_interactive_test.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ fn init_skips_gitignore_prompt_when_stdin_not_a_terminal() {
393393
std::fs::create_dir_all(project.path().join("src")).unwrap();
394394
std::fs::write(project.path().join("src/lib.rs"), "pub fn marker() {}\n").unwrap();
395395

396-
let mut command = tracedecay_command_without_daemon(home.path(), project.path());
396+
let mut command = tracedecay_command(home.path(), project.path());
397397
command.arg("init");
398398
let output = run_with_timeout(command, cli_timeout());
399399

@@ -1782,7 +1782,6 @@ fn migrate_registry_gc_cleans_stale_storage_metadata_and_preserves_live_and_bloc
17821782
std::fs::create_dir_all(&live_project).expect("live project dir");
17831783
std::fs::write(live_project.join("lib.rs"), "pub fn live() {}\n").expect("live source");
17841784

1785-
#[cfg(unix)]
17861785
let daemon = crate::common::spawn_tracedecay_daemon(home.path());
17871786
let init = tracedecay_command_without_daemon(home.path(), &live_project)
17881787
.args(["init", "."])
@@ -1794,7 +1793,6 @@ fn migrate_registry_gc_cleans_stale_storage_metadata_and_preserves_live_and_bloc
17941793
String::from_utf8_lossy(&init.stdout),
17951794
String::from_utf8_lossy(&init.stderr)
17961795
);
1797-
#[cfg(unix)]
17981796
drop(daemon);
17991797

18001798
let stale_project = canonical_temp_path(home.path()).join("gone-project");

tests/hooks_lsp_suite/hook_replay_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ async fn replayed_provider_hooks_record_attributed_rows_and_bridge_to_analytics_
253253
.success()
254254
);
255255
}
256+
let daemon = spawn_tracedecay_daemon(&home_root);
256257
let init = tracedecay_command_with_home(&home_root)
257258
.arg("init")
258259
.current_dir(&project_root)
@@ -315,7 +316,6 @@ async fn replayed_provider_hooks_record_attributed_rows_and_bridge_to_analytics_
315316
);
316317

317318
// Bridge: `analytics sync` imports the JSONL rows into the durable table.
318-
let daemon = spawn_tracedecay_daemon(&home_root);
319319
let sync = tracedecay_command_with_home(&home_root)
320320
.args(["analytics", "sync"])
321321
.current_dir(&project_root)

tests/memory_suite/memory_eval_test.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,8 @@ struct FixtureSnapshot {
229229

230230
#[cfg(windows)]
231231
impl FixtureSnapshot {
232-
fn capture(fixture: &Fixture) -> Self {
232+
fn capture(fixture: &mut Fixture) -> Self {
233+
fixture.stop_daemon();
233234
let dir = TempDir::new().expect("fixture snapshot tempdir");
234235
let profile_path = dir.path().join(".tracedecay");
235236
std::fs::create_dir_all(&profile_path).unwrap_or_else(|e| {
@@ -239,13 +240,15 @@ impl FixtureSnapshot {
239240
)
240241
});
241242
copy_dir_contents(&fixture.home_path.join(".tracedecay"), &profile_path);
243+
fixture.start_daemon();
242244
Self {
243245
_dir: dir,
244246
profile_path,
245247
}
246248
}
247249

248-
fn restore_into(&self, fixture: &Fixture) {
250+
fn restore_into(&self, fixture: &mut Fixture) {
251+
fixture.stop_daemon();
249252
let profile_path = fixture.home_path.join(".tracedecay");
250253
if profile_path.exists() {
251254
std::fs::remove_dir_all(&profile_path).unwrap_or_else(|e| {
@@ -262,10 +265,21 @@ impl FixtureSnapshot {
262265
)
263266
});
264267
copy_dir_contents(&self.profile_path, &profile_path);
268+
fixture.start_daemon();
265269
}
266270
}
267271

268272
impl Fixture {
273+
fn start_daemon(&mut self) {
274+
assert!(self._daemon.is_none(), "fixture daemon already running");
275+
self._daemon = Some(common::spawn_tracedecay_daemon(&self.home_path));
276+
}
277+
278+
#[cfg(windows)]
279+
fn stop_daemon(&mut self) {
280+
drop(self._daemon.take());
281+
}
282+
269283
fn db_path(&self) -> PathBuf {
270284
tracedecay::storage::resolve_layout(&self.project_path, &self.home_path.join(".tracedecay"))
271285
.expect("resolve fixture storage layout")
@@ -534,7 +548,7 @@ fn build_fixture(setup: &Setup) -> Fixture {
534548
}
535549
initialize_fixture_project(&fixture);
536550
seed_setup_facts(&fixture, &setup.facts);
537-
fixture._daemon = Some(common::spawn_tracedecay_daemon(&fixture.home_path));
551+
fixture.start_daemon();
538552
fixture
539553
}
540554

@@ -816,7 +830,7 @@ fn run_scenario(id: &str) {
816830
#[cfg(windows)]
817831
let baseline_snapshot =
818832
if !well_behaved_steps.is_empty() && scenario.deterministic.violation.is_some() {
819-
Some(FixtureSnapshot::capture(&fixture))
833+
Some(FixtureSnapshot::capture(&mut fixture))
820834
} else {
821835
None
822836
};
@@ -854,7 +868,7 @@ fn run_scenario(id: &str) {
854868
if !well_behaved_steps.is_empty() {
855869
#[cfg(windows)]
856870
if let Some(snapshot) = &baseline_snapshot {
857-
snapshot.restore_into(&fixture);
871+
snapshot.restore_into(&mut fixture);
858872
}
859873
#[cfg(not(windows))]
860874
{

0 commit comments

Comments
 (0)