← Watch feed
Refactor monitor file listing in tests to filter .tmp files
Public commit record
83/100 · Strong
What the developer wrote
Authored by Joost Jager
Refactor monitor file listing in tests to filter .tmp files
Extract common logic for listing monitor files into a helper function
that filters out temporary .tmp files created during persistence
operations. This simplifies test code and improves reliability on
systems where directory iteration order is non-deterministic.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Mentions testing or verification
What changed, and why it matters
This commit only changes test code in the Lightning Dev Kit Rust library. It extracts repeated logic for listing monitor files into a small helper function that ignores temporary `.tmp` files. There is no change to production code, no security fix, and no vulnerability being addressed.
Recommended action
No security action needed. This is a test-only refactor.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff refactors two test functions in lightning-background-processor/src/lib.rs to use a new list_monitor_files helper. The helper filters out .tmp (including uppercase .TMP on Windows) entries from directory listings. The original inline code already skipped .tmp files; the refactor centralizes that logic and makes assertions more explicit. No runtime behavior of the library is changed.
Changed components
lightning-background-processor/src/lib.rs (test module only)Inspect captured patch +34 / −34
diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs
index c38d6df..e5ecb5b 100644
--- a/lightning-background-processor/src/lib.rs
+++ b/lightning-background-processor/src/lib.rs
@@ -1984,6 +1984,26 @@ mod tests {
const EVENT_DEADLINE: Duration =
Duration::from_millis(5 * (FRESHNESS_TIMER.as_millis() as u64));
+ /// Reads a directory and returns only non-`.tmp` files.
+ /// The file system may return files in any order, and during persistence
+ /// operations there may be temporary `.tmp` files present.
+ fn list_monitor_files(dir: &str) -> Vec<std::fs::DirEntry> {
+ std::fs::read_dir(dir)
+ .unwrap()
+ .filter_map(|entry| {
+ let entry = entry.unwrap();
+ let path_str = entry.path().to_str().unwrap().to_lowercase();
+ // Skip any .tmp files that may exist during persistence.
+ // On Windows, ReplaceFileW creates backup files with .TMP (uppercase).
+ if path_str.ends_with(".tmp") {
+ None
+ } else {
+ Some(entry)
+ }
+ })
+ .collect()
+ }
+
#[derive(Clone, Hash, PartialEq, Eq)]
struct TestDescriptor {}
impl SocketDescriptor for TestDescriptor {
@@ -3787,30 +3807,20 @@ mod tests {
);
let dir = format!("{}_persister_1/monitors", &persist_dir);
- let mut mons = std::fs::read_dir(&dir).unwrap();
- let mut mon = mons.next().unwrap().unwrap();
- if mon.path().to_str().unwrap().ends_with(".tmp") {
- mon = mons.next().unwrap().unwrap();
- assert_eq!(mon.path().extension(), None);
- }
- assert!(mons.next().is_none());
+ let mut mons = list_monitor_files(&dir);
+ assert_eq!(mons.len(), 1);
+ let mon = mons.pop().unwrap();
// Because the channel wasn't funded, we'll archive the ChannelMonitor immedaitely after
// its force-closed (at least on node B, which didn't put their money into it).
nodes[1].node.force_close_all_channels_broadcasting_latest_txn("".to_owned());
loop {
- let mut mons = std::fs::read_dir(&dir).unwrap();
- if let Some(new_mon) = mons.next() {
- let mut new_mon = new_mon.unwrap();
- if new_mon.path().to_str().unwrap().ends_with(".tmp") {
- new_mon = mons.next().unwrap().unwrap();
- assert_eq!(new_mon.path().extension(), None);
- }
- assert_eq!(new_mon.path(), mon.path());
- assert!(mons.next().is_none());
- } else {
+ let mons = list_monitor_files(&dir);
+ if mons.is_empty() {
break;
}
+ assert_eq!(mons.len(), 1);
+ assert_eq!(mons[0].path(), mon.path());
}
bp.stop().unwrap();
@@ -3855,30 +3865,20 @@ mod tests {
));
let dir = format!("{}_persister_1/monitors", &persist_dir);
- let mut mons = std::fs::read_dir(&dir).unwrap();
- let mut mon = mons.next().unwrap().unwrap();
- if mon.path().to_str().unwrap().ends_with(".tmp") {
- mon = mons.next().unwrap().unwrap();
- assert_eq!(mon.path().extension(), None);
- }
- assert!(mons.next().is_none());
+ let mut mons = list_monitor_files(&dir);
+ assert_eq!(mons.len(), 1);
+ let mon = mons.pop().unwrap();
// Because the channel wasn't funded, we'll archive the ChannelMonitor immedaitely after
// its force-closed (at least on node B, which didn't put their money into it).
nodes[1].node.force_close_all_channels_broadcasting_latest_txn("".to_owned());
loop {
- let mut mons = std::fs::read_dir(&dir).unwrap();
- if let Some(new_mon) = mons.next() {
- let mut new_mon = new_mon.unwrap();
- if new_mon.path().to_str().unwrap().ends_with(".tmp") {
- new_mon = mons.next().unwrap().unwrap();
- assert_eq!(new_mon.path().extension(), None);
- }
- assert_eq!(new_mon.path(), mon.path());
- assert!(mons.next().is_none());
- } else {
+ let mons = list_monitor_files(&dir);
+ if mons.is_empty() {
break;
}
+ assert_eq!(mons.len(), 1);
+ assert_eq!(mons[0].path(), mon.path());
tokio::task::yield_now().await;
}
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.