Skip to content

Commit 4daac78

Browse files
committed
Test for changed files using contents instead of timestamps
For unknown reasons, using the last modified time to detect when a file is updated is flaky in CI. It appears that some file writes do not update the modified time. For example https://github.com/integrated-application-development/pasfmt/actions/runs/9787470331/job/27024005558 Since it's only used for tests, it's easy enough to just compare file contents instead.
1 parent 38a83b8 commit 4daac78

File tree

1 file changed

+34
-39
lines changed

1 file changed

+34
-39
lines changed

front-end/tests/file_discovery.rs

+34-39
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
use assert_fs::{prelude::*, TempDir};
22
use predicates::prelude::*;
33
use std::path::Path;
4-
use std::{path::PathBuf, time::SystemTime};
4+
use std::path::PathBuf;
55

66
use crate::utils::*;
77

8-
fn file_timestamps(glob: &str) -> DynResult<Vec<(PathBuf, SystemTime)>> {
8+
fn file_contents(glob: &str) -> DynResult<Vec<(PathBuf, String)>> {
99
let mut out = vec![];
1010
for f in glob::glob(glob)? {
1111
let f = f?;
1212
let metadata = f.metadata()?;
1313
if metadata.is_file() {
14-
out.push((f, metadata.modified()?))
14+
let contents = std::fs::read_to_string(&f)?;
15+
out.push((f, contents))
1516
}
1617
}
1718

@@ -20,34 +21,28 @@ fn file_timestamps(glob: &str) -> DynResult<Vec<(PathBuf, SystemTime)>> {
2021
Ok(out)
2122
}
2223

23-
fn file_timestamps_in_dir(dir: &Path) -> DynResult<Vec<(PathBuf, SystemTime)>> {
24-
file_timestamps(&(dir.to_string_lossy() + "/**/*"))
24+
fn file_contents_in_dir(dir: &Path) -> DynResult<Vec<(PathBuf, String)>> {
25+
file_contents(&(dir.to_string_lossy() + "/**/*"))
2526
}
2627

27-
fn assert_all_timestamps_changed(
28-
glob: &str,
29-
orig_timestamps: &[(PathBuf, SystemTime)],
30-
) -> TestResult {
31-
for (before, after) in orig_timestamps.iter().zip(file_timestamps(glob)?) {
28+
fn assert_all_contents_changed(glob: &str, orig_contents: &[(PathBuf, String)]) -> TestResult {
29+
for (before, after) in orig_contents.iter().zip(file_contents(glob)?) {
3230
assert_eq!(
3331
before.0, after.0,
3432
"File paths should be unchanged after formatting"
3533
);
3634
assert_ne!(
3735
before.1, after.1,
38-
"File {:?} is expected to be included in formatting, but the last modified time didn't change",
36+
"File {:?} is expected to be included in formatting, but its contents didn't change",
3937
before.0
4038
);
4139
}
4240

4341
Ok(())
4442
}
4543

46-
fn assert_no_timestamps_changed(
47-
glob: &str,
48-
orig_timestamps: &[(PathBuf, SystemTime)],
49-
) -> TestResult {
50-
assert_eq!(orig_timestamps, file_timestamps(glob)?);
44+
fn assert_no_contents_changed(glob: &str, orig_contents: &[(PathBuf, String)]) -> TestResult {
45+
assert_eq!(orig_contents, file_contents(glob)?);
5146

5247
Ok(())
5348
}
@@ -58,30 +53,30 @@ fn recursive_directory_search() -> TestResult {
5853

5954
tmp.copy_from(TESTS_DIR.to_string() + "/data/ext", &["**/*"])?;
6055

61-
let included_files_timestamps = file_timestamps_in_dir(&tmp.child("included"))?;
62-
let excluded_files_timestamps = file_timestamps_in_dir(&tmp.child("excluded"))?;
56+
let included_files_contents = file_contents_in_dir(&tmp.child("included"))?;
57+
let excluded_files_contents = file_contents_in_dir(&tmp.child("excluded"))?;
6358

6459
assert_eq!(
65-
included_files_timestamps.len(),
60+
included_files_contents.len(),
6661
6,
6762
"Unexpected number of files copied from tests/data/ext/included"
6863
);
6964
assert_eq!(
70-
excluded_files_timestamps.len(),
65+
excluded_files_contents.len(),
7166
6,
7267
"Unexpected number of files copied from tests/data/ext/excluded"
7368
);
7469

7570
fmt(&*tmp)?;
7671

77-
assert_no_timestamps_changed(
72+
assert_no_contents_changed(
7873
&(tmp.to_string_lossy() + "/excluded/**/*"),
79-
&excluded_files_timestamps,
74+
&excluded_files_contents,
8075
)?;
8176

82-
assert_all_timestamps_changed(
77+
assert_all_contents_changed(
8378
&(tmp.to_string_lossy() + "/included/**/*"),
84-
&included_files_timestamps,
79+
&included_files_contents,
8580
)?;
8681

8782
Ok(())
@@ -94,29 +89,29 @@ fn glob() -> TestResult {
9489
tmp.copy_from(TESTS_DIR.to_string() + "/data/ext/included", &["**/*"])?;
9590

9691
let pas_glob = tmp.to_string_lossy() + "/**/*.pas";
97-
let pas_timestamps = file_timestamps(&pas_glob)?;
98-
assert_eq!(pas_timestamps.len(), 1);
92+
let pas_contents = file_contents(&pas_glob)?;
93+
assert_eq!(pas_contents.len(), 1);
9994

10095
let dpr_glob = tmp.to_string_lossy() + "/**/*.dpr";
101-
let dpr_timestamps = file_timestamps(&dpr_glob)?;
102-
assert_eq!(dpr_timestamps.len(), 1);
96+
let dpr_contents = file_contents(&dpr_glob)?;
97+
assert_eq!(dpr_contents.len(), 1);
10398

10499
pasfmt()?
105100
.current_dir(&tmp)
106101
.arg("**/*.pas")
107102
.assert()
108103
.success();
109-
assert_all_timestamps_changed(&pas_glob, &pas_timestamps)?;
110-
assert_no_timestamps_changed(&dpr_glob, &dpr_timestamps)?;
104+
assert_all_contents_changed(&pas_glob, &pas_contents)?;
105+
assert_no_contents_changed(&dpr_glob, &dpr_contents)?;
111106

112-
let pas_timestamps = file_timestamps(&pas_glob)?;
107+
let pas_contents = file_contents(&pas_glob)?;
113108
pasfmt()?
114109
.current_dir(&tmp)
115110
.arg("**/*.dpr")
116111
.assert()
117112
.success();
118-
assert_all_timestamps_changed(&dpr_glob, &dpr_timestamps)?;
119-
assert_no_timestamps_changed(&pas_glob, &pas_timestamps)?;
113+
assert_all_contents_changed(&dpr_glob, &dpr_contents)?;
114+
assert_no_contents_changed(&pas_glob, &pas_contents)?;
120115

121116
Ok(())
122117
}
@@ -128,11 +123,11 @@ fn classic_glob_does_not_recurse() -> TestResult {
128123
tmp.copy_from(TESTS_DIR.to_string() + "/data/ext/included", &["**/*"])?;
129124

130125
let pas_glob = tmp.to_string_lossy() + "/**/*.pas";
131-
let pas_timestamps = file_timestamps(&pas_glob)?;
132-
assert_eq!(pas_timestamps.len(), 1);
126+
let pas_contents = file_contents(&pas_glob)?;
127+
assert_eq!(pas_contents.len(), 1);
133128

134129
pasfmt()?.current_dir(&tmp).arg("*.pas").assert().success();
135-
assert_no_timestamps_changed(&pas_glob, &pas_timestamps)?;
130+
assert_no_contents_changed(&pas_glob, &pas_contents)?;
136131

137132
Ok(())
138133
}
@@ -144,15 +139,15 @@ fn glob_can_include_any_ext() -> TestResult {
144139
tmp.copy_from(TESTS_DIR, &["data/ext/excluded/*"])?;
145140

146141
let excluded_glob = tmp.to_string_lossy() + "/data/ext/excluded/*";
147-
let excluded_files_timestamps = file_timestamps(&excluded_glob)?;
148-
assert_eq!(excluded_files_timestamps.len(), 6);
142+
let excluded_files_contents = file_contents(&excluded_glob)?;
143+
assert_eq!(excluded_files_contents.len(), 6);
149144

150145
pasfmt()?
151146
.current_dir(&tmp)
152147
.args(["**/*.*pas*", "**/*.*dpr*", "**/*.*dpk*"])
153148
.assert()
154149
.success();
155-
assert_all_timestamps_changed(&excluded_glob, &excluded_files_timestamps)?;
150+
assert_all_contents_changed(&excluded_glob, &excluded_files_contents)?;
156151

157152
Ok(())
158153
}

0 commit comments

Comments
 (0)