mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-26 03:10:21 +00:00
M14: apply the dead-path rule to families, not just the last resort
Round-6 review caught that this branch rejected is_file() for one path and kept using it forty lines above. The family loop filtered candidates with is_file(), which follows symlinks, so an entry that exists but no longer resolves was classified "not installed" and skipped — exactly the defect this branch fixes elsewhere. Now both blocks share one rule: presence via symlink_metadata, then a hard requirement that the entry resolves. Absence is still tolerated and reported; a broken entry fails loudly with the family named, because that is the failure we are trying to catch rather than a machine difference. Checked against this Mac first: no family or last-resort path is present-but-dangling here, so the stricter rule cannot produce a spurious red on current macOS. Also corrected two stale comments — the tripwire test is separate from the layout test *below* it, and the oracle's doc comment now states the exception the PR description already does: a Mac where nothing in the list covers Devanagari fails, since such a machine cannot validate anything. Verified solo on macOS 26.6 (Apple M5 Max), no concurrent writers this time: fmt --all clean; clippy -D warnings all-targets quiet; system_fonts 13 passed with and without PDFCRAFT_SYSTEM_FONTS=0; whole crate 1341 passed / 0 failed / 5 ignored; xtask layers (37 crates) and assets (272/6/22). Mutation evidence for the change, via font folders redirected to /tmp so system paths were untouched: with a dangling DevanagariMT.ttc in the system folder the current code fails at "exists but does not resolve", while reverting to is_file() filtering makes the identical defect pass silently. Re-ran the pinned regressions too: cap raised to 512 MiB, MAX_FACES cut to 1, probe swapped to Arabic alef, last_resort derived from first(), misspelling restored — all red.
This commit is contained in:
@@ -492,7 +492,7 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// Retune tripwires, deliberately separate from the layout test above: these assert that three
|
||||
/// Retune tripwires, deliberately separate from `the_macos_devanagari_list_is_apples_layout` below: these assert that three
|
||||
/// constants still hold the values this module's contract depends on. They cannot fail on their
|
||||
/// own — they exist so that editing one is a decision someone has to make twice. Each was
|
||||
/// checked by deleting the pin and watching the matching regression go green again: raising
|
||||
@@ -549,8 +549,9 @@ mod tests {
|
||||
|
||||
/// On a Mac, where Apple's fonts are installed: every family folder that really exists is
|
||||
/// reachable from the candidate list, and something in the list covers Devanagari. A family
|
||||
/// Apple no longer ships is reported rather than failed — this list exists precisely for the
|
||||
/// case where one of them is missing. CI has no macOS runner (`.github/workflows/ci.yml`
|
||||
/// Apple no longer ships is reported rather than failed — this list exists precisely for the case
|
||||
/// where one of them is missing, with one exception: if *nothing* here covers Devanagari the test
|
||||
/// fails loudly, because a machine with no face to check cannot validate the list at all. CI has no macOS runner (`.github/workflows/ci.yml`
|
||||
/// keeps the org's Mac slots for release builds), so this runs on contributors' and
|
||||
/// maintainers' machines.
|
||||
#[cfg(target_os = "macos")]
|
||||
@@ -562,12 +563,21 @@ mod tests {
|
||||
}
|
||||
let list = devanagari_candidates();
|
||||
for family in MACOS_DEVANAGARI_FAMILIES {
|
||||
let installed: Vec<PathBuf> = MACOS_FONT_DIRS.iter().map(|dir| Path::new(dir).join(family)).filter(|p| p.is_file()).collect();
|
||||
if installed.is_empty() {
|
||||
// Same rule as the last resort below: an entry that exists counts, even a link whose
|
||||
// target has gone — "present but unresolved" is precisely the dead-path defect this change
|
||||
// is about, and filtering on `is_file()` would quietly reclassify it as "not installed".
|
||||
// Absence is tolerated; a broken entry is not.
|
||||
let present: Vec<PathBuf> =
|
||||
MACOS_FONT_DIRS.iter().map(|dir| Path::new(dir).join(family)).filter(|p| std::fs::symlink_metadata(p).is_ok()).collect();
|
||||
if present.is_empty() {
|
||||
eprintln!("{family}: not installed on this machine, so the list offers nothing that exists — fine, and reported only");
|
||||
continue;
|
||||
}
|
||||
for path in &installed {
|
||||
for path in &present {
|
||||
assert!(
|
||||
path.is_file(),
|
||||
"{family}: {path:?} exists but does not resolve — its target was moved or removed, so this candidate path has gone dead"
|
||||
);
|
||||
assert!(list.contains(path), "{path:?} is installed here but the candidate list skips it: {list:?}");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user