Skip to content

Commit 630e257

Browse files
committed
Stabilize lock signal tests on macOS
1 parent 15bdff2 commit 630e257

2 files changed

Lines changed: 41 additions & 22 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ They should be mentioned in any package files, copyright notices, etc.
9797
- Snapshot tests for plans and diffs
9898
- Fuzz tests for regex generation to prevent backtracking issues
9999
- Cross platform tests including Windows path edge cases
100+
- Lock signal integration tests rely on the `wait_for_lock_state` polling helper (see `renamify-cli/src/test_lock_signals.rs`); avoid reintroducing fixed sleeps that cause macOS flakes
100101

101102
## CLI contract
102103

renamify-cli/src/test_lock_signals.rs

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
mod signal_tests {
55
use assert_cmd::Command as AssertCommand;
66
use std::fs;
7-
use std::path::PathBuf;
7+
use std::path::{Path, PathBuf};
88
use std::process::{Command, Stdio};
99
use std::thread;
1010
use std::time::{Duration, Instant};
@@ -26,6 +26,28 @@ mod signal_tests {
2626
(temp_dir, lock_file)
2727
}
2828

29+
fn wait_for_condition<F>(mut condition: F, timeout: Duration) -> bool
30+
where
31+
F: FnMut() -> bool,
32+
{
33+
let start = Instant::now();
34+
loop {
35+
if condition() {
36+
return true;
37+
}
38+
39+
if start.elapsed() >= timeout {
40+
return false;
41+
}
42+
43+
thread::sleep(Duration::from_millis(25));
44+
}
45+
}
46+
47+
fn wait_for_lock_state(lock_file: &Path, should_exist: bool, timeout: Duration) -> bool {
48+
wait_for_condition(|| lock_file.exists() == should_exist, timeout)
49+
}
50+
2951
#[test]
3052
fn test_lock_normal_completion() {
3153
let (temp_dir, lock_file) = create_test_env();
@@ -60,10 +82,9 @@ mod signal_tests {
6082
.spawn()
6183
.expect("Failed to spawn first command");
6284

63-
// Wait a bit for first command to acquire lock
64-
thread::sleep(Duration::from_millis(200));
85+
// Wait for first command to acquire lock
6586
assert!(
66-
lock_file.exists(),
87+
wait_for_lock_state(&lock_file, true, Duration::from_secs(2)),
6788
"Lock file should exist while first command runs"
6889
);
6990

@@ -99,8 +120,10 @@ mod signal_tests {
99120
.expect("Failed to spawn command");
100121

101122
// Wait for lock to be acquired
102-
thread::sleep(Duration::from_millis(200));
103-
assert!(lock_file.exists(), "Lock file should exist");
123+
assert!(
124+
wait_for_lock_state(&lock_file, true, Duration::from_secs(2)),
125+
"Lock file should exist"
126+
);
104127

105128
// Send SIGINT (Ctrl-C)
106129
unsafe {
@@ -118,11 +141,8 @@ mod signal_tests {
118141
);
119142

120143
// Give it a moment for cleanup
121-
thread::sleep(Duration::from_millis(100));
122-
123-
// Lock file should be cleaned up
124144
assert!(
125-
!lock_file.exists(),
145+
wait_for_lock_state(&lock_file, false, Duration::from_secs(2)),
126146
"Lock file should be removed after SIGINT"
127147
);
128148

@@ -142,8 +162,10 @@ mod signal_tests {
142162
.expect("Failed to spawn command");
143163

144164
// Wait for lock to be acquired
145-
thread::sleep(Duration::from_millis(200));
146-
assert!(lock_file.exists(), "Lock file should exist");
165+
assert!(
166+
wait_for_lock_state(&lock_file, true, Duration::from_secs(2)),
167+
"Lock file should exist"
168+
);
147169

148170
// Send SIGTERM
149171
unsafe {
@@ -161,11 +183,8 @@ mod signal_tests {
161183
);
162184

163185
// Give it a moment for cleanup
164-
thread::sleep(Duration::from_millis(100));
165-
166-
// Lock file should be cleaned up
167186
assert!(
168-
!lock_file.exists(),
187+
wait_for_lock_state(&lock_file, false, Duration::from_secs(2)),
169188
"Lock file should be removed after SIGTERM"
170189
);
171190

@@ -185,8 +204,10 @@ mod signal_tests {
185204
.expect("Failed to spawn command");
186205

187206
// Wait for lock to be acquired
188-
thread::sleep(Duration::from_millis(200));
189-
assert!(lock_file.exists(), "Lock file should exist");
207+
assert!(
208+
wait_for_lock_state(&lock_file, true, Duration::from_secs(2)),
209+
"Lock file should exist"
210+
);
190211

191212
// Send SIGKILL (kill -9) - no cleanup possible
192213
unsafe {
@@ -200,11 +221,8 @@ mod signal_tests {
200221
assert!(output.status.code().is_none(), "Should be killed by signal");
201222

202223
// Give it a moment, but lock file should still exist since no cleanup
203-
thread::sleep(Duration::from_millis(100));
204-
205-
// Lock file should still exist since SIGKILL prevents cleanup
206224
assert!(
207-
lock_file.exists(),
225+
wait_for_lock_state(&lock_file, true, Duration::from_secs(2)),
208226
"Lock file should still exist after SIGKILL"
209227
);
210228

0 commit comments

Comments
 (0)