Skip to content

Commit c7c7794

Browse files
committed
Fix failing tests in CI for windows
Sometimes in CI the file timestamps don't update after a write to the file. We don't know why exactly, but it probably has something to do with their virtualisation or some deduping filesystem. e.g. https://github.com/integrated-application-development/pasfmt/actions/runs/10590530822/job/29346465816#step:4:1159 Whatever the cause, it's clear we can't rely on timestamps to tell us when files are touched; a more robust method is to lock the file and assert that the operation fails as a result.
1 parent 4daac78 commit c7c7794

File tree

4 files changed

+49
-45
lines changed

4 files changed

+49
-45
lines changed

front-end/tests/idempotence.rs

+8-14
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use assert_fs::{prelude::*, TempDir};
22
use predicates::prelude::*;
33

4+
use crate::utils::windows::fmt_with_lock;
45
use crate::utils::*;
56

67
#[test]
@@ -10,30 +11,23 @@ fn file_is_only_written_when_necessary() -> TestResult {
1011
let to_leave = tmp.child("to_leave.pas");
1112
to_leave.write_str("a;")?;
1213
fmt(&*to_leave)?.success();
13-
let to_leave_last_written = to_leave.metadata()?.modified()?;
1414

1515
let to_change = tmp.child("to_change.pas");
1616
to_change.write_str("a ;")?;
17-
let to_change_last_written = to_change.metadata()?.modified()?;
1817

19-
fmt(&*tmp)?
18+
fmt_with_lock(&to_leave, 0)?
2019
.success()
2120
.stderr(predicate::str::contains(format!(
2221
"DEBUG Skipping writing to '{}' because it is already formatted.",
2322
to_leave.display()
2423
)));
2524

26-
assert_eq!(
27-
to_leave.metadata()?.modified()?,
28-
to_leave_last_written,
29-
"File already in formatted state should not be written to"
30-
);
31-
32-
assert_ne!(
33-
to_change.metadata()?.modified()?,
34-
to_change_last_written,
35-
"File not already in formatted state should be written to"
36-
);
25+
fmt_with_lock(&to_change, 0)?
26+
.failure()
27+
.stderr(predicate::str::contains(format!(
28+
"ERROR Failed to write to '{}'",
29+
to_change.display()
30+
)));
3731

3832
Ok(())
3933
}

front-end/tests/io_error.rs

+3-31
Original file line numberDiff line numberDiff line change
@@ -36,37 +36,9 @@ fn io_error_does_not_stop_formatting_other_files() -> TestResult {
3636
mod windows {
3737
use super::*;
3838

39-
use std::os::windows::io::AsRawHandle;
40-
use std::path::Path;
41-
use windows_sys::Win32::{
42-
Storage::FileSystem::{LockFileEx, LOCKFILE_EXCLUSIVE_LOCK, LOCK_FILE_FLAGS},
43-
System::IO::OVERLAPPED,
44-
};
45-
46-
fn fmt_with_lock(path: &Path, flags: LOCK_FILE_FLAGS) -> AssertResult {
47-
let handle = std::fs::OpenOptions::new().write(true).open(path)?;
48-
unsafe {
49-
let mut overlapped: OVERLAPPED = std::mem::zeroed();
50-
let ret = LockFileEx(
51-
handle.as_raw_handle() as isize,
52-
flags,
53-
0,
54-
!0,
55-
!0,
56-
&mut overlapped,
57-
);
58-
59-
if ret == 0 {
60-
return Err(Box::new(std::io::Error::last_os_error()));
61-
}
62-
}
63-
// I don't think a guard to unlock the file is necessary, because the Microsoft docs say
64-
// If a process terminates with a portion of a file locked or closes a file that has
65-
// outstanding locks, the locks are unlocked by the operating system.
66-
// And the file handle is closed at the end of this function.
67-
68-
Ok(fmt(path)?)
69-
}
39+
use windows_sys::Win32::Storage::FileSystem::LOCKFILE_EXCLUSIVE_LOCK;
40+
41+
use crate::utils::windows::fmt_with_lock;
7042

7143
const LOCKED_FILE_ERR_MSG: &str = "The process cannot access the file because another process has locked a portion of the file.";
7244

front-end/tests/tests.rs

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ mod config;
44
mod encoding;
55
mod file_discovery;
66
mod help;
7+
#[cfg(windows)]
78
mod idempotence;
89
mod io_error;
910
mod modes;

front-end/tests/utils.rs

+37
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,40 @@ pub fn fmt(
1717
pub type DynResult<T> = Result<T, Box<dyn std::error::Error>>;
1818
pub type TestResult = DynResult<()>;
1919
pub type AssertResult = DynResult<assert_cmd::assert::Assert>;
20+
21+
#[cfg(windows)]
22+
pub mod windows {
23+
use super::*;
24+
25+
use std::os::windows::io::AsRawHandle;
26+
use std::path::Path;
27+
use windows_sys::Win32::{
28+
Storage::FileSystem::{LockFileEx, LOCK_FILE_FLAGS},
29+
System::IO::OVERLAPPED,
30+
};
31+
32+
pub fn fmt_with_lock(path: &Path, flags: LOCK_FILE_FLAGS) -> AssertResult {
33+
let handle = std::fs::OpenOptions::new().write(true).open(path)?;
34+
unsafe {
35+
let mut overlapped: OVERLAPPED = std::mem::zeroed();
36+
let ret = LockFileEx(
37+
handle.as_raw_handle() as isize,
38+
flags,
39+
0,
40+
!0,
41+
!0,
42+
&mut overlapped,
43+
);
44+
45+
if ret == 0 {
46+
return Err(Box::new(std::io::Error::last_os_error()));
47+
}
48+
}
49+
// I don't think a guard to unlock the file is necessary, because the Microsoft docs say
50+
// If a process terminates with a portion of a file locked or closes a file that has
51+
// outstanding locks, the locks are unlocked by the operating system.
52+
// And the file handle is closed at the end of this function.
53+
54+
Ok(fmt(path)?)
55+
}
56+
}

0 commit comments

Comments
 (0)