diff --git a/.vscode/settings.json b/.vscode/settings.json index 82d167a..4648a35 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,5 +1,6 @@ { "markdownlint.config": { "MD024": false // Allow multiple headers with the same content; exists in NEWS historically - } + }, + "rust-analyzer.linkedProjects": ["./Cargo.toml"] } diff --git a/Cargo.toml b/Cargo.toml index 13982e6..c0d6132 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,7 @@ cachedir = "0.3" clicolors-control = "1.0" crc32c = { version = "0.6", optional = true } derive_more = "0.99" -fail = "0.5.1" +fail = { version = "0.5.1" } filetime = "0.2" futures = { version = "0.3", optional = true } globset = "0.4.5" diff --git a/NEWS.md b/NEWS.md index 817fa20..59ad250 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # Conserve release history +## Unreleased + +- `restore` no longer prints stats, due to internal changes; this will be restored later. + ## 23.11.0 - Fixed: Restore now sets Unix user/group ownership on symlinks and directories. Previously, only file ownership was restored. (Setting file ownership typically requires restoring as root.) diff --git a/src/archive.rs b/src/archive.rs index d3ed478..c3d7ebe 100644 --- a/src/archive.rs +++ b/src/archive.rs @@ -22,7 +22,7 @@ use std::time::Instant; use itertools::Itertools; use rayon::prelude::*; use serde::{Deserialize, Serialize}; -use tracing::{debug, error, warn}; +use tracing::{debug, warn}; use crate::blockhash::BlockHash; use crate::jsonio::{read_json, write_json}; @@ -127,9 +127,10 @@ impl Archive { band_selection: BandSelectionPolicy, subtree: Apath, exclude: Exclude, + monitor: Arc, ) -> Result> { self.open_stored_tree(band_selection)? - .iter_entries(subtree, exclude) + .iter_entries(subtree, exclude, monitor) } /// Returns a vector of band ids, in sorted order from first to last. @@ -311,7 +312,7 @@ impl Archive { /// tracing messages. This function only returns an error if validation /// stops due to a fatal error. pub fn validate(&self, options: &ValidateOptions, monitor: Arc) -> Result<()> { - self.validate_archive_dir()?; + self.validate_archive_dir(monitor.clone())?; debug!("List bands..."); let band_ids = self.list_band_ids()?; @@ -328,9 +329,9 @@ impl Archive { // TODO: Check for unexpected files or directories in the blockdir. let present_blocks: HashSet = self.block_dir.blocks(monitor.clone())?.collect(); - for block_hash in referenced_lens.keys() { - if !present_blocks.contains(block_hash) { - error!(%block_hash, "Referenced block missing"); + for hash in referenced_lens.keys() { + if !present_blocks.contains(hash) { + monitor.error(Error::BlockMissing { hash: hash.clone() }) } } } else { @@ -339,25 +340,24 @@ impl Archive { let block_lengths: HashMap = self.block_dir.validate(monitor.clone())?; // 3b. Check that all referenced ranges are inside the present data. - for (block_hash, referenced_len) in referenced_lens { - if let Some(&actual_len) = block_lengths.get(&block_hash) { + for (hash, referenced_len) in referenced_lens { + if let Some(&actual_len) = block_lengths.get(&hash) { if referenced_len > actual_len as u64 { - error!( - %block_hash, - referenced_len, + monitor.error(Error::BlockTooShort { + hash: hash.clone(), actual_len, - "Block is shorter than referenced length" - ); + referenced_len: referenced_len as usize, + }); } } else { - error!(%block_hash, "Referenced block missing"); + monitor.error(Error::BlockMissing { hash: hash.clone() }) } } } Ok(()) } - fn validate_archive_dir(&self) -> Result<()> { + fn validate_archive_dir(&self, monitor: Arc) -> Result<()> { // TODO: More tests for the problems detected here. debug!("Check archive directory..."); let mut seen_bands = HashSet::::new(); @@ -366,7 +366,9 @@ impl Archive { if let Ok(band_id) = dir_name.parse::() { if !seen_bands.insert(band_id) { // TODO: Test this - error!(%band_id, "Duplicated band directory"); + monitor.error(Error::InvalidMetadata { + details: format!("Duplicated band directory for {band_id:?}"), + }); } } else if !dir_name.eq_ignore_ascii_case(BLOCK_DIR) { // TODO: The whole path not just the filename diff --git a/src/backup.rs b/src/backup.rs index a16adc7..09c97b1 100644 --- a/src/backup.rs +++ b/src/backup.rs @@ -26,7 +26,7 @@ use std::time::{Duration, Instant}; use bytes::BytesMut; use derive_more::{Add, AddAssign}; use itertools::Itertools; -use tracing::{error, trace, warn}; +use tracing::{trace, warn}; use crate::blockdir::Address; use crate::change::Change; @@ -89,18 +89,19 @@ pub fn backup( monitor: Arc, ) -> Result { let start = Instant::now(); - let mut writer = BackupWriter::begin(archive, options)?; + let mut writer = BackupWriter::begin(archive, options, monitor.clone())?; let mut stats = BackupStats::default(); let source_tree = LiveTree::open(source_path)?; let task = monitor.start_task("Backup".to_string()); - let entry_iter = source_tree.iter_entries(Apath::root(), options.exclude.clone())?; + let entry_iter = + source_tree.iter_entries(Apath::root(), options.exclude.clone(), monitor.clone())?; for entry_group in entry_iter.chunks(options.max_entries_per_hunk).into_iter() { for entry in entry_group { match writer.copy_entry(&entry, &source_tree, options, monitor.clone()) { Err(err) => { - error!(?entry, ?err, "Error copying entry to backup"); + monitor.error(err); stats.errors += 1; continue; } @@ -150,14 +151,18 @@ impl BackupWriter { /// Create a new BackupWriter. /// /// This currently makes a new top-level band. - pub fn begin(archive: &Archive, options: &BackupOptions) -> Result { + pub fn begin( + archive: &Archive, + options: &BackupOptions, + monitor: Arc, + ) -> Result { if gc_lock::GarbageCollectionLock::is_locked(archive)? { return Err(Error::GarbageCollectionLockHeld); } let basis_index = if let Some(basis_band_id) = archive.last_band_id()? { - IterStitchedIndexHunks::new(archive, basis_band_id) + IterStitchedIndexHunks::new(archive, basis_band_id, monitor) } else { - IterStitchedIndexHunks::empty(archive) + IterStitchedIndexHunks::empty(archive, monitor) } .iter_entries(Apath::root(), Exclude::nothing()); diff --git a/src/band.rs b/src/band.rs index 16259f2..64d72e4 100644 --- a/src/band.rs +++ b/src/band.rs @@ -27,10 +27,11 @@ use std::sync::Arc; use itertools::Itertools; use serde::{Deserialize, Serialize}; use time::OffsetDateTime; -use tracing::{debug, error, warn}; +use tracing::{debug, warn}; use crate::jsonio::{read_json, write_json}; use crate::misc::remove_item; +use crate::monitor::Monitor; use crate::transport::{ListDir, Transport}; use crate::*; @@ -285,10 +286,12 @@ impl Band { }) } - pub fn validate(&self) -> Result<()> { + pub fn validate(&self, monitor: Arc) -> Result<()> { let ListDir { mut files, dirs } = self.transport.list_dir("")?; if !files.contains(&BAND_HEAD_FILENAME.to_string()) { - error!(band_id = ?self.band_id, "Band head file missing"); + monitor.error(Error::BandHeadMissing { + band_id: self.band_id, + }); } remove_item(&mut files, &BAND_HEAD_FILENAME); remove_item(&mut files, &BAND_TAIL_FILENAME); diff --git a/src/bin/conserve.rs b/src/bin/conserve.rs index 3cc9682..164e1f9 100644 --- a/src/bin/conserve.rs +++ b/src/bin/conserve.rs @@ -22,7 +22,6 @@ use std::time::Instant; use clap::{Parser, Subcommand}; use conserve::change::Change; -use conserve::trace_counter::{global_error_count, global_warn_count}; use rayon::prelude::ParallelIterator; use time::UtcOffset; #[allow(unused_imports)] @@ -395,7 +394,7 @@ impl Command { include_unchanged: *include_unchanged, }; let mut bw = BufWriter::new(stdout); - for change in diff(&st, <, &options)? { + for change in diff(&st, <, &options, monitor.clone())? { if *json { serde_json::to_writer(&mut bw, &change)?; } else { @@ -439,14 +438,15 @@ impl Command { // TODO: Option for subtree. Box::new( stored_tree_from_opt(archive, &stos.backup)? - .iter_entries(Apath::root(), exclude)? + .iter_entries(Apath::root(), exclude, monitor.clone())? .map(|it| it.into()), ) } else { - Box::new( - LiveTree::open(stos.source.clone().unwrap())? - .iter_entries(Apath::root(), exclude)?, - ) + Box::new(LiveTree::open(stos.source.clone().unwrap())?.iter_entries( + Apath::root(), + exclude, + monitor.clone(), + )?) }; monitor.clear_progress_bars(); if *json { @@ -467,11 +467,12 @@ impl Command { exclude, exclude_from, only_subtree, - no_stats, long_listing, + no_stats, } => { let band_selection = band_selection_policy_from_opt(backup); let archive = Archive::open(open_transport(archive)?)?; + let _ = no_stats; // accepted but ignored; we never currently print stats let options = RestoreOptions { exclude: Exclude::from_patterns_and_files(exclude, exclude_from)?, only_subtree: only_subtree.clone(), @@ -483,11 +484,8 @@ impl Command { &changes_json.as_deref(), )?, }; - let stats = restore(&archive, destination, &options, monitor)?; + restore(&archive, destination, &options, monitor)?; debug!("Restore complete"); - if !no_stats { - debug!(%stats); - } } Command::Size { stos, @@ -516,8 +514,8 @@ impl Command { let options = ValidateOptions { skip_block_hashes: *quick, }; - Archive::open(open_transport(archive)?)?.validate(&options, monitor)?; - if global_error_count() != 0 || global_warn_count() != 0 { + Archive::open(open_transport(archive)?)?.validate(&options, monitor.clone())?; + if monitor.error_count() != 0 { warn!("Archive has some problems."); } else { info!("Archive is OK."); @@ -638,18 +636,12 @@ fn main() -> Result { monitor.counters(), )?; } - let error_count = global_error_count(); - let warn_count = global_warn_count(); match result { Err(err) => { error!("{err:#}"); - debug!(error_count, warn_count); Ok(ExitCode::Failure) } - Ok(ExitCode::Success) if error_count != 0 || warn_count != 0 => { - debug!(error_count, warn_count); - Ok(ExitCode::NonFatalErrors) - } + Ok(ExitCode::Success) if monitor.error_count() != 0 => Ok(ExitCode::NonFatalErrors), Ok(exit_code) => Ok(exit_code), } } diff --git a/src/blockdir.rs b/src/blockdir.rs index fe5ba4d..a1e0f2a 100644 --- a/src/blockdir.rs +++ b/src/blockdir.rs @@ -193,9 +193,10 @@ impl BlockDir { let end = start + len; let actual_len = bytes.len(); if end > actual_len { - return Err(Error::AddressTooLong { - address: address.to_owned(), + return Err(Error::BlockTooShort { + hash: address.hash.clone(), actual_len, + referenced_len: len, }); } Ok(bytes.slice(start..end)) @@ -343,7 +344,7 @@ mod test { use tempfile::TempDir; - use crate::monitor::collect::CollectMonitor; + use crate::monitor::test::TestMonitor; use crate::transport::open_local_transport; use super::*; @@ -356,7 +357,7 @@ mod test { let tempdir = TempDir::new().unwrap(); let blockdir = BlockDir::open(open_local_transport(tempdir.path()).unwrap()); let mut stats = BackupStats::default(); - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let hash = blockdir .store_or_deduplicate(Bytes::from("stuff"), &mut stats, monitor.clone()) .unwrap(); @@ -369,7 +370,7 @@ mod test { // Open again to get a fresh cache let blockdir = BlockDir::open(open_local_transport(tempdir.path()).unwrap()); - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); OpenOptions::new() .write(true) .truncate(true) @@ -385,7 +386,7 @@ mod test { fn temp_files_are_not_returned_as_blocks() { let tempdir = TempDir::new().unwrap(); let blockdir = BlockDir::open(open_local_transport(tempdir.path()).unwrap()); - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let subdir = tempdir.path().join(subdir_relpath("123")); create_dir(&subdir).unwrap(); write( @@ -407,16 +408,16 @@ mod test { let mut stats = BackupStats::default(); let content = Bytes::from("stuff"); let hash = blockdir - .store_or_deduplicate(content.clone(), &mut stats, CollectMonitor::arc()) + .store_or_deduplicate(content.clone(), &mut stats, TestMonitor::arc()) .unwrap(); assert_eq!(blockdir.stats.cache_hit.load(Relaxed), 0); - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); assert!(blockdir.contains(&hash, monitor.clone()).unwrap()); assert_eq!(blockdir.stats.cache_hit.load(Relaxed), 1); assert_eq!(monitor.get_counter(Counter::BlockExistenceCacheHit), 1); - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let retrieved = blockdir.get_block_content(&hash, monitor.clone()).unwrap(); assert_eq!(content, retrieved); assert_eq!(monitor.get_counter(Counter::BlockContentCacheHit), 1); @@ -436,13 +437,13 @@ mod test { let blockdir = BlockDir::open(open_local_transport(tempdir.path()).unwrap()); let mut stats = BackupStats::default(); let content = Bytes::from("stuff"); - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let hash = blockdir .store_or_deduplicate(content.clone(), &mut stats, monitor.clone()) .unwrap(); // reopen - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let blockdir = BlockDir::open(open_local_transport(tempdir.path()).unwrap()); assert!(blockdir.contains(&hash, monitor.clone()).unwrap()); assert_eq!(blockdir.stats.cache_hit.load(Relaxed), 0); diff --git a/src/diff.rs b/src/diff.rs index e113b07..0666c3e 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -15,8 +15,11 @@ //! //! See also [conserve::show_diff] to format the diff as text. +use std::sync::Arc; + use readahead_iterator::IntoReadahead; +use crate::monitor::Monitor; use crate::*; #[derive(Debug)] @@ -41,14 +44,15 @@ pub fn diff( st: &StoredTree, lt: &LiveTree, options: &DiffOptions, + monitor: Arc, ) -> Result> { let readahead = 1000; let include_unchanged: bool = options.include_unchanged; // Copy out to avoid lifetime problems in the callback let ait = st - .iter_entries(Apath::root(), options.exclude.clone())? + .iter_entries(Apath::root(), options.exclude.clone(), monitor.clone())? .readahead(readahead); let bit = lt - .iter_entries(Apath::root(), options.exclude.clone())? + .iter_entries(Apath::root(), options.exclude.clone(), monitor.clone())? .filter(|le| le.kind() != Kind::Unknown) .readahead(readahead); Ok(MergeTrees::new(ait, bit) diff --git a/src/errors.rs b/src/errors.rs index 6bb98ac..4a47545 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,5 +1,5 @@ // Conserve backup system. -// Copyright 2015-2023 Martin Pool. +// Copyright 2015-2024 Martin Pool. // This program is free software; you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -19,7 +19,6 @@ use std::path::PathBuf; use thiserror::Error; -use crate::blockdir::Address; use crate::*; /// Conserve specific error. @@ -29,8 +28,15 @@ pub enum Error { #[error("Block file {hash:?} corrupt: does not have the expected hash")] BlockCorrupt { hash: BlockHash }, - #[error("{address:?} extends beyond decompressed block length {actual_len:?}")] - AddressTooLong { address: Address, actual_len: usize }, + #[error("Referenced block {hash} is missing")] + BlockMissing { hash: BlockHash }, + + #[error("Block {hash} is too short: actual len {actual_len}, referenced len {referenced_len}")] + BlockTooShort { + hash: BlockHash, + actual_len: usize, + referenced_len: usize, + }, #[error("Not a Conserve archive (no CONSERVE header found)")] NotAnArchive, @@ -124,8 +130,27 @@ pub enum Error { #[error("Failed to read source tree {:?}", path)] ListSourceTree { path: PathBuf, source: io::Error }, - #[error("Failed to restore {:?}", path)] - Restore { path: PathBuf, source: io::Error }, + #[error("Failed to restore file {:?}", path)] + RestoreFile { path: PathBuf, source: io::Error }, + + #[error("Failed to restore symlink {path:?}")] + RestoreSymlink { path: PathBuf, source: io::Error }, + + #[error("Failed to read block content {hash} for {apath}")] + RestoreFileBlock { + apath: Apath, + hash: BlockHash, + source: Box, + }, + + #[error("Failed to restore directory {:?}", path)] + RestoreDirectory { path: PathBuf, source: io::Error }, + + #[error("Failed to restore ownership of {:?}", path)] + RestoreOwnership { path: PathBuf, source: io::Error }, + + #[error("Failed to restore permissions on {:?}", path)] + RestorePermissions { path: PathBuf, source: io::Error }, #[error("Failed to restore modification time on {:?}", path)] RestoreModificationTime { path: PathBuf, source: io::Error }, diff --git a/src/gc_lock.rs b/src/gc_lock.rs index 96c21a3..25fc286 100644 --- a/src/gc_lock.rs +++ b/src/gc_lock.rs @@ -108,7 +108,7 @@ impl Drop for GarbageCollectionLock { #[cfg(test)] mod test { use super::*; - use crate::monitor::collect::CollectMonitor; + use crate::monitor::test::TestMonitor; use crate::test_fixtures::{ScratchArchive, TreeFixture}; #[test] @@ -131,7 +131,7 @@ mod test { &archive, source.path(), &BackupOptions::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ) .unwrap(); let delete_guard = GarbageCollectionLock::new(&archive).unwrap(); @@ -147,7 +147,7 @@ mod test { &archive, source.path(), &BackupOptions::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ); assert_eq!( backup_result.expect_err("backup fails").to_string(), diff --git a/src/index.rs b/src/index.rs index 3f7b4f1..0a7703b 100644 --- a/src/index.rs +++ b/src/index.rs @@ -520,7 +520,7 @@ impl>> IndexEntryIter { mod tests { use tempfile::TempDir; - use crate::monitor::collect::CollectMonitor; + use crate::monitor::test::TestMonitor; use super::transport::local::LocalTransport; use super::*; @@ -572,7 +572,7 @@ mod tests { let (_testdir, mut ib) = setup(); ib.push_entry(sample_entry("/zzz")); ib.push_entry(sample_entry("/aaa")); - ib.finish_hunk(CollectMonitor::arc()).unwrap(); + ib.finish_hunk(TestMonitor::arc()).unwrap(); } #[test] @@ -580,7 +580,7 @@ mod tests { fn index_builder_checks_names() { let (_testdir, mut ib) = setup(); ib.push_entry(sample_entry("../escapecat")); - ib.finish_hunk(CollectMonitor::arc()).unwrap(); + ib.finish_hunk(TestMonitor::arc()).unwrap(); } #[test] @@ -590,7 +590,7 @@ mod tests { let (_testdir, mut ib) = setup(); ib.push_entry(sample_entry("/again")); ib.push_entry(sample_entry("/again")); - ib.finish_hunk(CollectMonitor::arc()).unwrap(); + ib.finish_hunk(TestMonitor::arc()).unwrap(); } #[test] @@ -599,9 +599,9 @@ mod tests { fn no_duplicate_paths_across_hunks() { let (_testdir, mut ib) = setup(); ib.push_entry(sample_entry("/again")); - ib.finish_hunk(CollectMonitor::arc()).unwrap(); + ib.finish_hunk(TestMonitor::arc()).unwrap(); ib.push_entry(sample_entry("/again")); - ib.finish_hunk(CollectMonitor::arc()).unwrap(); + ib.finish_hunk(TestMonitor::arc()).unwrap(); } #[test] @@ -612,7 +612,7 @@ mod tests { #[test] fn basic() { let (testdir, mut ib) = setup(); - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); ib.append_entries(&mut vec![sample_entry("/apple"), sample_entry("/banana")]); let hunks = ib.finish(monitor.clone()).unwrap(); assert_eq!(monitor.get_counter(Counter::IndexWrites), 1); @@ -644,9 +644,9 @@ mod tests { fn multiple_hunks() { let (testdir, mut ib) = setup(); ib.append_entries(&mut vec![sample_entry("/1.1"), sample_entry("/1.2")]); - ib.finish_hunk(CollectMonitor::arc()).unwrap(); + ib.finish_hunk(TestMonitor::arc()).unwrap(); ib.append_entries(&mut vec![sample_entry("/2.1"), sample_entry("/2.2")]); - ib.finish_hunk(CollectMonitor::arc()).unwrap(); + ib.finish_hunk(TestMonitor::arc()).unwrap(); let index_read = IndexRead::open_path(testdir.path()); let it = index_read.iter_entries(); @@ -677,9 +677,9 @@ mod tests { fn iter_hunks_advance_to_after() { let (testdir, mut ib) = setup(); ib.append_entries(&mut vec![sample_entry("/1.1"), sample_entry("/1.2")]); - ib.finish_hunk(CollectMonitor::arc()).unwrap(); + ib.finish_hunk(TestMonitor::arc()).unwrap(); ib.append_entries(&mut vec![sample_entry("/2.1"), sample_entry("/2.2")]); - ib.finish_hunk(CollectMonitor::arc()).unwrap(); + ib.finish_hunk(TestMonitor::arc()).unwrap(); let index_read = IndexRead::open_path(testdir.path()); let names: Vec = index_read @@ -761,13 +761,13 @@ mod tests { ib.push_entry(sample_entry("/bar")); ib.push_entry(sample_entry("/foo")); ib.push_entry(sample_entry("/foobar")); - ib.finish_hunk(CollectMonitor::arc()).unwrap(); + ib.finish_hunk(TestMonitor::arc()).unwrap(); // Make multiple hunks to test traversal across hunks. ib.push_entry(sample_entry("/g01")); ib.push_entry(sample_entry("/g02")); ib.push_entry(sample_entry("/g03")); - ib.finish_hunk(CollectMonitor::arc()).unwrap(); + ib.finish_hunk(TestMonitor::arc()).unwrap(); // Advance to /foo and read on from there. let mut it = IndexRead::open_path(testdir.path()).iter_entries(); @@ -802,9 +802,9 @@ mod tests { for i in 0..100_000 { ib.push_entry(sample_entry(&format!("/{i:0>10}"))); } - ib.finish_hunk(CollectMonitor::arc())?; + ib.finish_hunk(TestMonitor::arc())?; // Think about, but don't actually add some files - ib.finish_hunk(CollectMonitor::arc())?; + ib.finish_hunk(TestMonitor::arc())?; let read_index = IndexRead::open_path(testdir.path()); assert_eq!(read_index.iter_hunks().count(), 1); Ok(()) diff --git a/src/lib.rs b/src/lib.rs index 629f387..1d2d589 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,7 +43,6 @@ mod stitch; mod stored_tree; pub mod termui; pub mod test_fixtures; -pub mod trace_counter; pub mod transport; mod tree; pub mod unix_mode; @@ -72,7 +71,7 @@ pub use crate::misc::bytes_to_human_mb; pub use crate::owner::Owner; pub use crate::restore::{restore, RestoreOptions}; pub use crate::show::{show_versions, ShowVersionsOptions}; -pub use crate::stats::{DeleteStats, RestoreStats}; +pub use crate::stats::DeleteStats; pub use crate::stored_tree::StoredTree; pub use crate::transport::{open_transport, Transport}; pub use crate::tree::{ReadTree, TreeSize}; diff --git a/src/live_tree.rs b/src/live_tree.rs index 37a879b..019fcfd 100644 --- a/src/live_tree.rs +++ b/src/live_tree.rs @@ -18,10 +18,12 @@ use std::fs; use std::fs::File; use std::io::ErrorKind; use std::path::{Path, PathBuf}; +use std::sync::Arc; use tracing::{error, warn}; use crate::entry::{EntryValue, KindMeta}; +use crate::monitor::Monitor; use crate::owner::Owner; use crate::stats::LiveTreeIterStats; use crate::unix_mode::UnixMode; @@ -63,7 +65,12 @@ impl tree::ReadTree for LiveTree { type Entry = EntryValue; type IT = Iter; - fn iter_entries(&self, subtree: Apath, exclude: Exclude) -> Result { + fn iter_entries( + &self, + subtree: Apath, + exclude: Exclude, + _monitor: Arc, + ) -> Result { Iter::new(&self.path, subtree, exclude) } } diff --git a/src/merge.rs b/src/merge.rs index 2cbbfa9..86e6308 100644 --- a/src/merge.rs +++ b/src/merge.rs @@ -121,6 +121,7 @@ where #[cfg(test)] mod tests { + use crate::monitor::test::TestMonitor; use crate::test_fixtures::*; use crate::*; @@ -130,12 +131,13 @@ mod tests { fn merge_entry_trees() { let ta = TreeFixture::new(); let tb = TreeFixture::new(); + let monitor = TestMonitor::arc(); let di = MergeTrees::new( ta.live_tree() - .iter_entries(Apath::root(), Exclude::nothing()) + .iter_entries(Apath::root(), Exclude::nothing(), monitor.clone()) .unwrap(), tb.live_tree() - .iter_entries(Apath::root(), Exclude::nothing()) + .iter_entries(Apath::root(), Exclude::nothing(), monitor.clone()) .unwrap(), ) .collect::>(); diff --git a/src/monitor/collect.rs b/src/monitor/collect.rs deleted file mode 100644 index d842cf9..0000000 --- a/src/monitor/collect.rs +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright 2023 Martin Pool - -//! Collect monitored information so that it can be inspected by tests. - -#![allow(unused_imports)] - -use std::collections::HashSet; -use std::mem::take; -use std::sync::atomic::AtomicUsize; -use std::sync::atomic::Ordering::Relaxed; -use std::sync::{Arc, Mutex, Weak}; - -use super::task::{Task, TaskList, TaskState}; -use super::{Monitor, Problem}; -use crate::counters::{Counter, Counters}; -use crate::{Apath, GarbageCollectionLock}; - -/// A monitor that collects information for later inspection. -/// -/// Problems are collected in a vector. -/// -/// Tasks are ignored. -/// -/// Totals of counters are kept. -#[derive(Default)] -pub struct CollectMonitor { - pub problems: Mutex>, - counters: Counters, - started_files: Mutex>, - task_list: Mutex, -} - -impl CollectMonitor { - pub fn new() -> Self { - CollectMonitor::default() - } - - pub fn get_counter(&self, counter: Counter) -> usize { - self.counters.get(counter) - } - - pub fn take_problems(&self) -> Vec { - take(self.problems.lock().unwrap().as_mut()) - } - - pub fn take_started_files(&self) -> Vec { - take(self.started_files.lock().unwrap().as_mut()) - } - - pub fn arc() -> Arc { - Arc::new(CollectMonitor::new()) - } - - pub fn counters(&self) -> &Counters { - &self.counters - } -} - -impl Monitor for CollectMonitor { - fn count(&self, counter: Counter, increment: usize) { - self.counters.count(counter, increment) - } - - fn set_counter(&self, counter: Counter, value: usize) { - self.counters.set(counter, value) - } - - fn problem(&self, problem: Problem) { - self.problems.lock().unwrap().push(problem); - } - - fn start_task(&self, name: String) -> Task { - self.task_list.lock().unwrap().start_task(name) - } -} diff --git a/src/monitor/mod.rs b/src/monitor/mod.rs index 7e418ff..b14e064 100644 --- a/src/monitor/mod.rs +++ b/src/monitor/mod.rs @@ -1,15 +1,15 @@ -// Copyright 2023 Martin Pool +// Copyright 2023-2024 Martin Pool //! Communication from the library to a monitor: a test, a UI, etc. -pub mod collect; pub mod task; - -use std::fmt::Debug; +pub mod test; use self::task::Task; use crate::counters::Counter; +/// A monitor receives events from the library and may collect them, report them +/// to the terminal, log them, etc. pub trait Monitor: Send + Sync + 'static { /// Notify that a counter increased by a given amount. fn count(&self, counter: Counter, increment: usize); @@ -17,14 +17,8 @@ pub trait Monitor: Send + Sync + 'static { /// Set the absolute value of a counter. fn set_counter(&self, counter: Counter, value: usize); - /// Notify that a problem occurred. - fn problem(&self, problem: Problem); + /// A non-fatal error occurred. + fn error(&self, error: crate::Error); fn start_task(&self, name: String) -> Task; } - -#[derive(Debug)] -pub enum Problem { - /// Some generic error. - Error(crate::Error), -} diff --git a/src/monitor/test.rs b/src/monitor/test.rs new file mode 100644 index 0000000..5844f73 --- /dev/null +++ b/src/monitor/test.rs @@ -0,0 +1,90 @@ +// Copyright 2023-2024 Martin Pool + +//! Collect monitored information so that it can be inspected by tests. + +use std::mem::take; +use std::sync::{Arc, Mutex}; + +use super::task::{Task, TaskList}; +use super::Monitor; +use crate::counters::{Counter, Counters}; +use crate::{Apath, Error}; + +/// A monitor that collects information for later inspection, +/// particularly from tests. +/// +/// Errors are collected in a vector. +/// +/// Tasks are ignored. +/// +/// Totals of counters are kept. +#[derive(Default)] +pub struct TestMonitor { + errors: Mutex>, + counters: Counters, + started_files: Mutex>, + task_list: Mutex, +} + +impl TestMonitor { + pub fn new() -> Self { + TestMonitor::default() + } + + /// Construct a new TestMonitor and wrap it in an Arc. + pub fn arc() -> Arc { + Arc::new(TestMonitor::new()) + } + + pub fn get_counter(&self, counter: Counter) -> usize { + self.counters.get(counter) + } + + /// Return the list of errors, and clear it. + pub fn take_errors(&self) -> Vec { + take(self.errors.lock().unwrap().as_mut()) + } + + /// Assert that no errors have yet occurred (since the list was cleared.) + /// + /// Panic if any errors have been reported. + pub fn assert_no_errors(&self) { + let errors = self.errors.lock().unwrap(); + assert!(errors.is_empty(), "Unexpected errors: {errors:#?}"); + } + + /// Assert the expected value of a counter. + pub fn assert_counter(&self, counter: Counter, expected: usize) { + let actual = self.counters.get(counter); + assert_eq!( + actual, expected, + "Expected counter {counter:?} to be {expected}, but was {actual}", + ); + } + + pub fn take_started_files(&self) -> Vec { + take(self.started_files.lock().unwrap().as_mut()) + } + + pub fn counters(&self) -> &Counters { + &self.counters + } +} + +impl Monitor for TestMonitor { + fn count(&self, counter: Counter, increment: usize) { + self.counters.count(counter, increment) + } + + fn set_counter(&self, counter: Counter, value: usize) { + self.counters.set(counter, value) + } + + fn error(&self, error: Error) { + self.errors.lock().unwrap().push(error); + } + + fn start_task(&self, name: String) -> Task { + self.task_list.lock().unwrap().start_task(name) + } +} diff --git a/src/owner.rs b/src/owner.rs index ad6f9bc..d5835e9 100644 --- a/src/owner.rs +++ b/src/owner.rs @@ -18,6 +18,7 @@ //! be restored on a different system. use std::fmt::Display; +use std::io; use std::path::Path; use serde::{Deserialize, Serialize}; @@ -44,7 +45,7 @@ impl Owner { self.user.is_none() && self.group.is_none() } - pub fn set_owner>(&self, path: P) -> crate::Result<()> { + pub fn set_owner>(&self, path: P) -> io::Result<()> { set_owner(self, path.as_ref()) } } diff --git a/src/owner/unix.rs b/src/owner/unix.rs index aa41426..6c94111 100644 --- a/src/owner/unix.rs +++ b/src/owner/unix.rs @@ -1,6 +1,6 @@ // Conserve backup system. // Copyright 2022 Stephanie Aelmore. -// Copyright 2015-2023 Martin Pool. +// Copyright 2015-2024 Martin Pool. // This program is free software; you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -23,7 +23,6 @@ use lazy_static::lazy_static; use uzers::{Groups, Users, UsersCache}; use super::Owner; -use crate::Result; lazy_static! { static ref USERS_CACHE: Mutex = Mutex::new(UsersCache::new()); @@ -43,7 +42,7 @@ impl From<&fs::Metadata> for Owner { } #[mutants::skip] // TODO: Difficult to test as non-root but we could at least test that at least groups are restored! -pub(crate) fn set_owner(owner: &Owner, path: &Path) -> Result<()> { +pub(crate) fn set_owner(owner: &Owner, path: &Path) -> io::Result<()> { let users_cache = USERS_CACHE.lock().unwrap(); let uid_opt = owner .user @@ -65,6 +64,6 @@ pub(crate) fn set_owner(owner: &Owner, path: &Path) -> Result<()> { // complaining Ok(()) } - Err(err) => Err(err.into()), + Err(err) => Err(err), } } diff --git a/src/owner/windows.rs b/src/owner/windows.rs index 59827ac..14e6fc1 100644 --- a/src/owner/windows.rs +++ b/src/owner/windows.rs @@ -15,6 +15,7 @@ //! Windows null implementation of file ownership. use std::fs::Metadata; +use std::io; use std::path::Path; use super::Owner; @@ -30,6 +31,6 @@ impl From<&Metadata> for Owner { } } -pub fn set_owner(_owner: &Owner, _path: &Path) -> Result<()> { +pub fn set_owner(_owner: &Owner, _path: &Path) -> io::Result<()> { Ok(()) } diff --git a/src/restore.rs b/src/restore.rs index 4fb13b1..52ec62b 100644 --- a/src/restore.rs +++ b/src/restore.rs @@ -1,4 +1,4 @@ -// Copyright 2015-2023 Martin Pool. +// Copyright 2015-2024 Martin Pool. // This program is free software; you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -12,26 +12,22 @@ //! Restore from the archive to the filesystem. -use std::fs::File; -use std::io; -use std::io::Write; +use std::fs::{self, File}; +use std::io::{self, Write}; use std::path::{Path, PathBuf}; -use std::sync::atomic::Ordering::Relaxed; use std::sync::Arc; -use std::{fs, time::Instant}; use fail::fail_point; use filetime::set_file_handle_times; #[cfg(unix)] use filetime::set_symlink_file_times; use time::OffsetDateTime; -use tracing::{error, instrument, trace, warn}; +use tracing::{instrument, trace, warn}; use crate::band::BandSelectionPolicy; use crate::counters::Counter; use crate::io::{directory_is_empty, ensure_dir_exists}; use crate::monitor::Monitor; -use crate::stats::RestoreStats; use crate::unix_mode::UnixMode; use crate::unix_time::ToFileTime; use crate::*; @@ -68,15 +64,13 @@ pub fn restore( destination: &Path, options: &RestoreOptions, monitor: Arc, -) -> Result { +) -> Result<()> { let st = archive.open_stored_tree(options.band_selection.clone())?; ensure_dir_exists(destination)?; if !options.overwrite && !directory_is_empty(destination)? { return Err(Error::DestinationNotEmpty); } - let mut stats = RestoreStats::default(); let task = monitor.start_task("Restore".to_string()); - let start = Instant::now(); let block_dir = archive.block_dir(); // // This causes us to walk the source tree twice, which is probably an acceptable option // // since it's nice to see realistic overall progress. We could keep all the entries @@ -92,6 +86,7 @@ pub fn restore( let entry_iter = st.iter_entries( options.only_subtree.clone().unwrap_or_else(Apath::root), options.exclude.clone(), + monitor.clone(), )?; let mut deferrals = Vec::new(); for entry in entry_iter { @@ -100,12 +95,15 @@ pub fn restore( match entry.kind() { Kind::Dir => { monitor.count(Counter::Dirs, 1); - stats.directories += 1; - if let Err(err) = create_dir(&path) { - if err.kind() != io::ErrorKind::AlreadyExists { - error!(?path, ?err, "Failed to create directory"); - stats.errors += 1; - continue; + if *entry.apath() != Apath::root() { + if let Err(err) = create_dir(&path) { + if err.kind() != io::ErrorKind::AlreadyExists { + monitor.error(Error::RestoreDirectory { + path: path.clone(), + source: err, + }); + continue; + } } } deferrals.push(DirDeferral { @@ -116,33 +114,23 @@ pub fn restore( }) } Kind::File => { - stats.files += 1; monitor.count(Counter::Files, 1); - match restore_file(path.clone(), &entry, block_dir, monitor.clone()) { - Err(err) => { - // TODO: Report it to the monitor too? - error!(?err, ?path, "Failed to restore file"); - stats.errors += 1; - continue; - } - Ok(s) => { - monitor.count(Counter::FileBytes, s.uncompressed_file_bytes as usize); - stats += s; - } + if let Err(err) = restore_file(path.clone(), &entry, block_dir, monitor.clone()) { + monitor.error(err); + continue; } } Kind::Symlink => { monitor.count(Counter::Symlinks, 1); - stats.symlinks += 1; if let Err(err) = restore_symlink(&path, &entry) { - error!(?path, ?err, "Failed to restore symlink"); - stats.errors += 1; + monitor.error(err); continue; } } Kind::Unknown => { - stats.unknown_kind += 1; - warn!(apath = ?entry.apath(), "Unknown file kind"); + monitor.error(Error::InvalidMetadata { + details: format!("Unknown file kind {:?}", entry.apath()), + }); } }; if let Some(cb) = options.change_callback.as_ref() { @@ -150,15 +138,12 @@ pub fn restore( cb(&EntryChange::added(&entry))?; } } - stats += apply_deferrals(&deferrals)?; - stats.elapsed = start.elapsed(); - stats.block_cache_hits = block_dir.stats.cache_hit.load(Relaxed); - // TODO: Merge in stats from the tree iter and maybe the source tree? - Ok(stats) + apply_deferrals(&deferrals, monitor.clone())?; + Ok(()) } fn create_dir(path: &Path) -> io::Result<()> { - fail_point!("conserve::restore::create-dir", |_| { + fail_point!("restore::create-dir", |_| { Err(io::Error::new( io::ErrorKind::PermissionDenied, "Simulated failure", @@ -179,8 +164,7 @@ struct DirDeferral { owner: Owner, } -fn apply_deferrals(deferrals: &[DirDeferral]) -> Result { - let mut stats = RestoreStats::default(); +fn apply_deferrals(deferrals: &[DirDeferral], monitor: Arc) -> Result<()> { for DirDeferral { path, unix_mode, @@ -188,20 +172,26 @@ fn apply_deferrals(deferrals: &[DirDeferral]) -> Result { owner, } in deferrals { - if let Err(err) = owner.set_owner(path) { - error!(?path, ?err, "Error restoring ownership"); - stats.errors += 1; + if let Err(source) = owner.set_owner(path) { + monitor.error(Error::RestoreOwnership { + path: path.clone(), + source, + }); } - if let Err(err) = unix_mode.set_permissions(path) { - error!(?path, ?err, "Failed to set directory permissions"); - stats.errors += 1; + if let Err(source) = unix_mode.set_permissions(path) { + monitor.error(Error::RestorePermissions { + path: path.clone(), + source, + }); } - if let Err(err) = filetime::set_file_mtime(path, (*mtime).to_file_time()) { - error!(?path, ?err, "Failed to set directory mtime"); - stats.errors += 1; + if let Err(source) = filetime::set_file_mtime(path, (*mtime).to_file_time()) { + monitor.error(Error::RestoreModificationTime { + path: path.clone(), + source, + }); } } - Ok(stats) + Ok(()) } /// Copy in the contents of a file from another tree. @@ -211,16 +201,11 @@ fn restore_file( source_entry: &IndexEntry, block_dir: &BlockDir, monitor: Arc, -) -> Result { - let mut stats = RestoreStats::default(); - let mut out = File::create(&path).map_err(|err| { - error!(?path, ?err, "Error creating destination file"); - Error::Restore { - path: path.clone(), - source: err, - } +) -> Result<()> { + let mut out = File::create(&path).map_err(|err| Error::RestoreFile { + path: path.clone(), + source: err, })?; - let mut len = 0u64; for addr in &source_entry.addrs { // TODO: We could combine small parts // in memory, and then write them in a single system call. However @@ -229,21 +214,18 @@ fn restore_file( // probably a waste. let bytes = block_dir .read_address(addr, monitor.clone()) - .map_err(|err| { - error!(?path, ?err, "Failed to read block content for file"); - err + .map_err(|source| Error::RestoreFileBlock { + apath: source_entry.apath.clone(), + hash: addr.hash.clone(), + source: Box::new(source), })?; - out.write_all(&bytes).map_err(|err| { - error!(?path, ?err, "Failed to write content to restore file"); - Error::Restore { - path: path.clone(), - source: err, - } + out.write_all(&bytes).map_err(|err| Error::RestoreFile { + path: path.clone(), + source: err, })?; - len += bytes.len() as u64; + monitor.count(Counter::FileBytes, bytes.len()); } - stats.uncompressed_file_bytes = len; - out.flush().map_err(|source| Error::Restore { + out.flush().map_err(|source| Error::RestoreFile { path: path.clone(), source, })?; @@ -255,37 +237,42 @@ fn restore_file( })?; // Restore permissions only if there are mode bits stored in the archive - if let Err(err) = source_entry.unix_mode().set_permissions(&path) { - error!(?path, ?err, "Error restoring unix permissions"); - stats.errors += 1; + if let Err(source) = source_entry.unix_mode().set_permissions(&path) { + monitor.error(Error::RestorePermissions { + path: path.clone(), + source, + }); } // Restore ownership if possible. // TODO: Stats and warnings if a user or group is specified in the index but // does not exist on the local system. - if let Err(err) = &source_entry.owner().set_owner(&path) { - error!(?path, ?err, "Error restoring ownership"); - stats.errors += 1; + if let Err(source) = source_entry.owner().set_owner(&path) { + monitor.error(Error::RestoreOwnership { + path: path.clone(), + source, + }); } // TODO: Accumulate more stats. trace!("Restored file"); - Ok(stats) + Ok(()) } #[cfg(unix)] -fn restore_symlink(path: &Path, entry: &IndexEntry) -> Result { - let mut stats = RestoreStats::default(); +fn restore_symlink(path: &Path, entry: &IndexEntry) -> Result<()> { use std::os::unix::fs as unix_fs; if let Some(ref target) = entry.symlink_target() { if let Err(source) = unix_fs::symlink(target, path) { - return Err(Error::Restore { + return Err(Error::RestoreSymlink { path: path.to_owned(), source, }); } - if let Err(err) = &entry.owner().set_owner(path) { - error!(?path, ?err, "Error restoring ownership"); - stats.errors += 1; + if let Err(source) = entry.owner().set_owner(path) { + return Err(Error::RestoreOwnership { + path: path.to_owned(), + source, + }); } let mtime = entry.mtime().to_file_time(); if let Err(source) = set_symlink_file_times(path, mtime, mtime) { @@ -295,17 +282,18 @@ fn restore_symlink(path: &Path, entry: &IndexEntry) -> Result { }); } } else { - error!(apath = ?entry.apath(), "No target in symlink entry"); - stats.errors += 1; + return Err(Error::InvalidMetadata { + details: format!("No target in symlink entry {:?}", entry.apath()), + }); } - Ok(stats) + Ok(()) } #[cfg(not(unix))] #[mutants::skip] -fn restore_symlink(_restore_path: &Path, entry: &IndexEntry) -> Result { +fn restore_symlink(_restore_path: &Path, entry: &IndexEntry) -> Result<()> { // TODO: Add a test with a canned index containing a symlink, and expect // it cannot be restored on Windows and can be on Unix. warn!("Can't restore symlinks on non-Unix: {}", entry.apath()); - Ok(RestoreStats::default()) + Ok(()) } diff --git a/src/stats.rs b/src/stats.rs index 6e5693f..f753371 100644 --- a/src/stats.rs +++ b/src/stats.rs @@ -91,42 +91,6 @@ pub struct LiveTreeIterStats { pub entries_returned: usize, } -#[derive(Add, AddAssign, Debug, Default, Eq, PartialEq, Clone)] -pub struct RestoreStats { - pub files: usize, - pub symlinks: usize, - pub directories: usize, - pub unknown_kind: usize, - - pub errors: usize, - - pub uncompressed_file_bytes: u64, - - pub elapsed: Duration, - - pub block_cache_hits: usize, -} - -impl fmt::Display for RestoreStats { - fn fmt(&self, w: &mut fmt::Formatter<'_>) -> fmt::Result { - write_count(w, "files:", self.files); - write_size(w, " ", self.uncompressed_file_bytes); - - write_count(w, "symlinks", self.symlinks); - write_count(w, "directories", self.directories); - write_count(w, "unsupported file kind", self.unknown_kind); - writeln!(w).unwrap(); - - write_count(w, "block cache hits", self.block_cache_hits); - writeln!(w).unwrap(); - - write_count(w, "errors", self.errors); - write_duration(w, "elapsed", self.elapsed)?; - - Ok(()) - } -} - #[derive(Add, AddAssign, Clone, Copy, Debug, Default, Eq, PartialEq)] pub struct DeleteStats { pub deleted_band_count: usize, diff --git a/src/stitch.rs b/src/stitch.rs index 0e945e0..68f1f9f 100644 --- a/src/stitch.rs +++ b/src/stitch.rs @@ -28,9 +28,12 @@ //! seen. //! * Bands might be deleted, so their numbers are not contiguous. -use tracing::{error, trace}; +use std::sync::Arc; + +use tracing::trace; use crate::index::{IndexEntryIter, IndexHunkIter}; +use crate::monitor::Monitor; use crate::*; pub struct IterStitchedIndexHunks { @@ -40,6 +43,8 @@ pub struct IterStitchedIndexHunks { archive: Archive, state: State, + + monitor: Arc, } /// What state is a stitch iter in, and what should happen next? @@ -70,19 +75,25 @@ impl IterStitchedIndexHunks { /// the same point in the previous band, continuing backwards recursively /// until either there are no more previous indexes, or a complete index /// is found. - pub(crate) fn new(archive: &Archive, band_id: BandId) -> IterStitchedIndexHunks { + pub(crate) fn new( + archive: &Archive, + band_id: BandId, + monitor: Arc, + ) -> IterStitchedIndexHunks { IterStitchedIndexHunks { archive: archive.clone(), last_apath: None, state: State::BeforeBand(band_id), + monitor, } } - pub(crate) fn empty(archive: &Archive) -> IterStitchedIndexHunks { + pub(crate) fn empty(archive: &Archive, monitor: Arc) -> IterStitchedIndexHunks { IterStitchedIndexHunks { archive: archive.clone(), last_apath: None, state: State::Done, + monitor, } } @@ -132,7 +143,7 @@ impl Iterator for IterStitchedIndexHunks { } } Err(err) => { - error!(?band_id, ?err, "Failed to open next band"); + self.monitor.error(err); State::AfterBand(*band_id) } } @@ -179,7 +190,7 @@ fn previous_existing_band(archive: &Archive, mut band_id: BandId) -> Option IndexEntry { @@ -196,7 +207,7 @@ mod test { } fn simple_ls(archive: &Archive, band_id: BandId) -> String { - let strs: Vec = IterStitchedIndexHunks::new(archive, band_id) + let strs: Vec = IterStitchedIndexHunks::new(archive, band_id, TestMonitor::arc()) .flatten() .map(|entry| format!("{}:{}", &entry.apath, entry.target.unwrap())) .collect(); @@ -222,7 +233,7 @@ mod test { // 1 was deleted in b2, 2 is carried over from b2, // and 3 is carried over from b1. - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let band = Band::create(&af)?; assert_eq!(band.id(), BandId::zero()); let mut ib = band.index_builder(); @@ -239,7 +250,7 @@ mod test { "2 hunks were finished" ); - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let band = Band::create(&af)?; assert_eq!(band.id().to_string(), "b0001"); let mut ib = band.index_builder(); @@ -254,7 +265,7 @@ mod test { band.close(2)?; // b2 - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let band = Band::create(&af)?; assert_eq!(band.id().to_string(), "b0002"); let mut ib = band.index_builder(); @@ -275,7 +286,7 @@ mod test { assert_eq!(band.id().to_string(), "b0004"); // b5 - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let band = Band::create(&af)?; assert_eq!(band.id().to_string(), "b0005"); let mut ib = band.index_builder(); @@ -321,7 +332,7 @@ mod test { &af, tf.path(), &BackupOptions::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ) .expect("backup should work"); @@ -330,10 +341,12 @@ mod test { let band_id = band_ids.first().expect("expected at least one band"); - let mut iter = IterStitchedIndexHunks::new(&af, *band_id); + let monitor = TestMonitor::arc(); + let mut iter = IterStitchedIndexHunks::new(&af, *band_id, monitor.clone()); // Get the first and only index entry. // `index_hunks` and `band_id` should be `Some`. assert!(iter.next().is_some()); + monitor.assert_no_errors(); // Remove the band head. This band can not be opened anymore. // If accessed this should fail the test. @@ -344,5 +357,11 @@ mod test { for _ in 0..10 { assert!(iter.next().is_none()); } + + // It's not an error (at the moment) because a band with no head effectively doesn't exist. + // (Maybe later the presence of a band directory with no head file should raise a warning.) + let errors = monitor.take_errors(); + dbg!(&errors); + assert_eq!(errors.len(), 0); } } diff --git a/src/stored_tree.rs b/src/stored_tree.rs index e25d353..f5834af 100644 --- a/src/stored_tree.rs +++ b/src/stored_tree.rs @@ -21,6 +21,7 @@ use std::sync::Arc; use crate::blockdir::BlockDir; +use crate::monitor::Monitor; use crate::stitch::IterStitchedIndexHunks; use crate::*; @@ -60,9 +61,16 @@ impl ReadTree for StoredTree { /// Return an iter of index entries in this stored tree. // TODO: Should return an iter of Result so that we can inspect them... - fn iter_entries(&self, subtree: Apath, exclude: Exclude) -> Result { - Ok(IterStitchedIndexHunks::new(&self.archive, self.band.id()) - .iter_entries(subtree, exclude)) + fn iter_entries( + &self, + subtree: Apath, + exclude: Exclude, + monitor: Arc, + ) -> Result { + Ok( + IterStitchedIndexHunks::new(&self.archive, self.band.id(), monitor) + .iter_entries(subtree, exclude), + ) } } @@ -70,6 +78,8 @@ impl ReadTree for StoredTree { mod test { use std::path::Path; + use crate::monitor::test::TestMonitor; + use super::super::test_fixtures::*; use super::super::*; @@ -83,8 +93,9 @@ mod test { assert_eq!(st.band().id(), last_band_id); + let monitor = TestMonitor::arc(); let names: Vec = st - .iter_entries(Apath::root(), Exclude::nothing()) + .iter_entries(Apath::root(), Exclude::nothing(), monitor.clone()) .unwrap() .map(|e| e.apath.into()) .collect(); @@ -121,8 +132,9 @@ mod test { .open_stored_tree(BandSelectionPolicy::Latest) .unwrap(); + let monitor = TestMonitor::arc(); let names: Vec = st - .iter_entries("/subdir".into(), Exclude::nothing()) + .iter_entries("/subdir".into(), Exclude::nothing(), monitor.clone()) .unwrap() .map(|entry| entry.apath.into()) .collect(); diff --git a/src/termui/monitor.rs b/src/termui/monitor.rs index 3f656fc..fd7e8c1 100644 --- a/src/termui/monitor.rs +++ b/src/termui/monitor.rs @@ -1,19 +1,21 @@ -// Copyright 2023 Martin Pool +// Copyright 2023-2024 Martin Pool //! Monitor on a terminal UI. -use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering::Relaxed; +use std::sync::atomic::{AtomicBool, AtomicUsize}; use std::sync::{Arc, Mutex}; use std::thread::{sleep, spawn, JoinHandle}; use std::time::Duration; use nutmeg::{Destination, View}; use thousands::Separable; +use tracing::error; use crate::counters::{Counter, Counters}; use crate::monitor::task::{Task, TaskList}; -use crate::monitor::{Monitor, Problem}; +use crate::monitor::Monitor; +use crate::Error; pub struct TermUiMonitor { // operation: Operation, @@ -28,6 +30,8 @@ pub struct TermUiMonitor { poller: Option>, /// True to ask the poller thread to stop, during drop. stop_poller: Arc, + /// Number of errors reported. + error_count: AtomicUsize, } /// The nutmeg model. @@ -72,6 +76,7 @@ impl TermUiMonitor { view, poller, stop_poller, + error_count: AtomicUsize::new(0), } } @@ -87,6 +92,11 @@ impl TermUiMonitor { pub fn counters(&self) -> &Counters { &self.counters } + + /// Return the number of errors reported. + pub fn error_count(&self) -> usize { + self.error_count.load(Relaxed) + } } impl Drop for TermUiMonitor { @@ -109,9 +119,9 @@ impl Monitor for TermUiMonitor { self.counters.set(counter, value) } - fn problem(&self, problem: Problem) { - // TODO: Colorful styling; maybe also send it to trace?? - self.view.message(format!("Problem: {:?}", problem)); + fn error(&self, error: Error) { + error!(target: "conserve", "{error}"); + self.error_count.fetch_add(1, Relaxed); } fn start_task(&self, name: String) -> Task { diff --git a/src/termui/trace.rs b/src/termui/trace.rs index 9f4bc74..5451d2c 100644 --- a/src/termui/trace.rs +++ b/src/termui/trace.rs @@ -73,7 +73,6 @@ pub fn enable_tracing( } Registry::default() .with(console_layer) - .with(crate::trace_counter::CounterLayer()) .with(json_layer) .init(); flush_guard diff --git a/src/test_fixtures.rs b/src/test_fixtures.rs index a78f4f7..ac8a3e2 100644 --- a/src/test_fixtures.rs +++ b/src/test_fixtures.rs @@ -23,7 +23,7 @@ use std::path::{Path, PathBuf}; use tempfile::TempDir; use crate::backup::BackupOptions; -use crate::monitor::collect::CollectMonitor; +use crate::monitor::test::TestMonitor; use crate::*; /// A temporary archive, deleted when it goes out of scope. @@ -66,10 +66,10 @@ impl ScratchArchive { } let options = &BackupOptions::default(); - backup(&self.archive, srcdir.path(), options, CollectMonitor::arc()).unwrap(); + backup(&self.archive, srcdir.path(), options, TestMonitor::arc()).unwrap(); srcdir.create_file("hello2"); - backup(&self.archive, srcdir.path(), options, CollectMonitor::arc()).unwrap(); + backup(&self.archive, srcdir.path(), options, TestMonitor::arc()).unwrap(); } pub fn transport(&self) -> &dyn Transport { diff --git a/src/trace_counter.rs b/src/trace_counter.rs deleted file mode 100644 index e881089..0000000 --- a/src/trace_counter.rs +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2023 Martin Pool. - -//! Count the number of `tracing` errors and warnings. - -use std::sync::atomic::{AtomicUsize, Ordering}; - -use tracing::{Event, Level, Subscriber}; -use tracing_subscriber::layer::Context; -use tracing_subscriber::Layer; - -/// Count of errors emitted to trace. -static ERROR_COUNT: AtomicUsize = AtomicUsize::new(0); - -/// Count of warnings emitted to trace. -static WARN_COUNT: AtomicUsize = AtomicUsize::new(0); - -/// Return the number of errors logged in the program so far. -pub fn global_error_count() -> usize { - ERROR_COUNT.load(Ordering::Relaxed) -} - -/// Return the number of warnings logged in the program so far. -pub fn global_warn_count() -> usize { - WARN_COUNT.load(Ordering::Relaxed) -} - -/// A tracing Layer that counts errors and warnings into static counters. -pub(crate) struct CounterLayer(); - -impl Layer for CounterLayer -where - S: Subscriber, -{ - fn on_event(&self, event: &Event<'_>, _ctx: Context<'_, S>) { - match *event.metadata().level() { - Level::ERROR => ERROR_COUNT.fetch_add(1, Ordering::Relaxed), - Level::WARN => WARN_COUNT.fetch_add(1, Ordering::Relaxed), - _ => 0, - }; - } -} diff --git a/src/tree.rs b/src/tree.rs index 71c85b8..e23b703 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -29,7 +29,12 @@ pub trait ReadTree { /// Errors reading individual paths or directories are sent to the UI and /// counted, but are not treated as fatal, and don't appear as Results in the /// iterator. - fn iter_entries(&self, subtree: Apath, exclude: Exclude) -> Result; + fn iter_entries( + &self, + subtree: Apath, + exclude: Exclude, + monitor: Arc, + ) -> Result; /// Measure the tree size. /// @@ -37,7 +42,7 @@ pub trait ReadTree { fn size(&self, exclude: Exclude, monitor: Arc) -> Result { let mut file_bytes = 0u64; let task = monitor.start_task("Measure tree".to_string()); - for e in self.iter_entries(Apath::root(), exclude)? { + for e in self.iter_entries(Apath::root(), exclude, monitor.clone())? { // While just measuring size, ignore directories/files we can't stat. if let Some(bytes) = e.size() { monitor.count(Counter::Files, 1); diff --git a/src/validate.rs b/src/validate.rs index fdb2d8a..8ec13d5 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -15,9 +15,8 @@ use std::collections::HashMap; use std::fmt::Debug; use std::sync::Arc; -use tracing::{debug, error}; +use tracing::debug; -use crate::misc::ResultExt; use crate::monitor::Monitor; use crate::*; @@ -45,24 +44,24 @@ pub(crate) fn validate_bands( let band = match Band::open(archive, *band_id) { Ok(band) => band, Err(err) => { - error!(%err, %band_id, "Error opening band"); + monitor.error(err); continue 'band; } }; - if let Err(err) = band.validate() { - error!(%err, %band_id, "Error validating band"); + if let Err(err) = band.validate(monitor.clone()) { + monitor.error(err); continue 'band; }; let st = match archive.open_stored_tree(BandSelectionPolicy::Specified(*band_id)) { Err(err) => { - error!(%err, %band_id, "Error validating stored tree"); + monitor.error(err); continue 'band; } Ok(st) => st, }; - let band_block_lens = match validate_stored_tree(&st, monitor.as_ref()) { + let band_block_lens = match validate_stored_tree(&st, monitor.clone()) { Err(err) => { - error!(%err, %band_id, "Error validating stored tree"); + monitor.error(err); continue 'band; } Ok(block_lens) => block_lens, @@ -80,15 +79,17 @@ fn merge_block_lens(into: &mut HashMap, from: &HashMap Result> { +fn validate_stored_tree( + st: &StoredTree, + monitor: Arc, +) -> Result> { // TODO: Check other entry properties are correct. // TODO: Check they're in apath order. // TODO: Count progress for index blocks within one tree? let _task = monitor.start_task(format!("Validate stored tree {}", st.band().id())); let mut block_lens = HashMap::new(); for entry in st - .iter_entries(Apath::root(), Exclude::nothing()) - .our_inspect_err(|err| error!(%err, "Error iterating index entries"))? + .iter_entries(Apath::root(), Exclude::nothing(), monitor.clone())? .filter(|entry| entry.kind() == Kind::File) { // TODO: Read index hunks, count into the task per hunk. Then, we can diff --git a/tests/api/archive.rs b/tests/api/archive.rs index cd3758b..d78a3c1 100644 --- a/tests/api/archive.rs +++ b/tests/api/archive.rs @@ -20,7 +20,7 @@ use assert_fs::prelude::*; use assert_fs::TempDir; use conserve::archive::Archive; -use conserve::monitor::collect::CollectMonitor; +use conserve::monitor::test::TestMonitor; use conserve::test_fixtures::ScratchArchive; use conserve::Band; use conserve::BandId; @@ -80,16 +80,13 @@ fn empty_archive() { "Archive should have no bands yet" ); assert_eq!( - af.referenced_blocks(&af.list_band_ids().unwrap(), CollectMonitor::arc()) + af.referenced_blocks(&af.list_band_ids().unwrap(), TestMonitor::arc()) .unwrap() .len(), 0 ); assert_eq!( - af.block_dir() - .blocks(CollectMonitor::arc()) - .unwrap() - .count(), + af.block_dir().blocks(TestMonitor::arc()).unwrap().count(), 0 ); } @@ -118,16 +115,13 @@ fn create_bands() { assert_eq!(af.last_band_id().unwrap(), Some(BandId::new(&[1]))); assert_eq!( - af.referenced_blocks(&af.list_band_ids().unwrap(), CollectMonitor::arc()) + af.referenced_blocks(&af.list_band_ids().unwrap(), TestMonitor::arc()) .unwrap() .len(), 0 ); assert_eq!( - af.block_dir() - .blocks(CollectMonitor::arc()) - .unwrap() - .count(), + af.block_dir().blocks(TestMonitor::arc()).unwrap().count(), 0 ); } diff --git a/tests/api/backup.rs b/tests/api/backup.rs index b03e860..2b326ae 100644 --- a/tests/api/backup.rs +++ b/tests/api/backup.rs @@ -17,7 +17,7 @@ use std::sync::Arc; use assert_fs::prelude::*; use assert_fs::TempDir; use conserve::counters::Counter; -use conserve::monitor::collect::CollectMonitor; +use conserve::monitor::test::TestMonitor; use filetime::{set_file_mtime, FileTime}; use rayon::prelude::ParallelIterator; @@ -37,7 +37,7 @@ pub fn simple_backup() { let srcdir = TreeFixture::new(); srcdir.create_file("hello"); - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let backup_stats = backup( &af, srcdir.path(), @@ -59,7 +59,7 @@ pub fn simple_backup() { assert!(archive.band_exists(BandId::zero()).unwrap()); assert!(archive.band_is_closed(BandId::zero()).unwrap()); assert!(!archive.band_exists(BandId::new(&[1])).unwrap()); - let copy_stats = restore( + restore( &archive, restore_dir.path(), &RestoreOptions::default(), @@ -67,7 +67,7 @@ pub fn simple_backup() { ) .expect("restore"); - assert_eq!(copy_stats.uncompressed_file_bytes, 8); + monitor.assert_counter(Counter::FileBytes, 8); } #[test] @@ -85,7 +85,7 @@ pub fn simple_backup_with_excludes() -> Result<()> { exclude, ..BackupOptions::default() }; - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let stats = backup(&af, srcdir.path(), &options, monitor.clone()).expect("backup"); check_backup(&af); @@ -109,20 +109,20 @@ pub fn simple_backup_with_excludes() -> Result<()> { assert!(band_info.is_closed); assert!(band_info.end_time.is_some()); - let copy_stats = restore( + let monitor = TestMonitor::arc(); + restore( &archive, restore_dir.path(), &RestoreOptions::default(), - CollectMonitor::arc(), + monitor.clone(), ) .expect("restore"); - - assert_eq!(copy_stats.uncompressed_file_bytes, 8); + monitor.assert_counter(Counter::FileBytes, 8); // TODO: Read back contents of that file. // TODO: Check index stats. // TODO: Check what was restored. - af.validate(&ValidateOptions::default(), Arc::new(CollectMonitor::new())) + af.validate(&ValidateOptions::default(), Arc::new(TestMonitor::new())) .unwrap(); assert!(!logs_contain("ERROR") && !logs_contain("WARN")); Ok(()) @@ -147,7 +147,7 @@ pub fn backup_more_excludes() { exclude, ..Default::default() }; - let stats = backup(&af, srcdir.path(), &options, CollectMonitor::arc()).expect("backup"); + let stats = backup(&af, srcdir.path(), &options, TestMonitor::arc()).expect("backup"); assert_eq!(1, stats.written_blocks); assert_eq!(1, stats.files); @@ -183,7 +183,7 @@ fn check_backup(af: &ScratchArchive) { assert!(file_entry.mtime > 0); assert_eq!( - af.referenced_blocks(&af.list_band_ids().unwrap(), CollectMonitor::arc()) + af.referenced_blocks(&af.list_band_ids().unwrap(), TestMonitor::arc()) .unwrap() .into_iter() .map(|h| h.to_string()) @@ -192,16 +192,14 @@ fn check_backup(af: &ScratchArchive) { ); assert_eq!( af.block_dir() - .blocks(CollectMonitor::arc()) + .blocks(TestMonitor::arc()) .unwrap() .map(|h| h.to_string()) .collect::>(), vec![HELLO_HASH] ); assert_eq!( - af.unreferenced_blocks(CollectMonitor::arc()) - .unwrap() - .count(), + af.unreferenced_blocks(TestMonitor::arc()).unwrap().count(), 0 ); } @@ -211,10 +209,11 @@ fn large_file() { let af = ScratchArchive::new(); let tf = TreeFixture::new(); - let large_content = vec![b'a'; 4 << 20]; + let file_size = 4 << 20; + let large_content = vec![b'a'; file_size]; tf.create_file_with_contents("large", &large_content); - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let backup_stats = backup( &af, tf.path(), @@ -237,14 +236,17 @@ fn large_file() { // Try to restore it let rd = TempDir::new().unwrap(); let restore_archive = Archive::open_path(af.path()).unwrap(); - let restore_stats = restore( + let monitor = TestMonitor::arc(); + restore( &restore_archive, rd.path(), &RestoreOptions::default(), - CollectMonitor::arc(), + monitor.clone(), ) .expect("restore"); - assert_eq!(restore_stats.files, 1); + monitor.assert_no_errors(); + monitor.assert_counter(Counter::Files, 1); + monitor.assert_counter(Counter::FileBytes, file_size); let content = std::fs::read(rd.path().join("large")).unwrap(); assert_eq!(large_content, content); @@ -267,7 +269,7 @@ fn source_unreadable() { &af, tf.path(), &BackupOptions::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ) .expect("backup"); assert_eq!(stats.errors, 1); @@ -290,8 +292,9 @@ fn mtime_before_epoch() { set_file_mtime(file_path, t1969).expect("Failed to set file times"); let lt = LiveTree::open(tf.path()).unwrap(); + let monitor = TestMonitor::arc(); let entries = lt - .iter_entries(Apath::root(), Exclude::nothing()) + .iter_entries(Apath::root(), Exclude::nothing(), monitor.clone()) .unwrap() .collect::>(); @@ -303,7 +306,7 @@ fn mtime_before_epoch() { &af, tf.path(), &BackupOptions::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ) .expect("backup shouldn't crash on before-epoch mtimes"); } @@ -319,7 +322,7 @@ pub fn symlink() { &af, srcdir.path(), &BackupOptions::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ) .expect("backup"); @@ -352,7 +355,7 @@ pub fn empty_file_uses_zero_blocks() { &af, srcdir.path(), &BackupOptions::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ) .unwrap(); @@ -362,7 +365,7 @@ pub fn empty_file_uses_zero_blocks() { // Read back the empty file let st = af.open_stored_tree(BandSelectionPolicy::Latest).unwrap(); let empty_entry = st - .iter_entries(Apath::root(), Exclude::nothing()) + .iter_entries(Apath::root(), Exclude::nothing(), TestMonitor::arc()) .unwrap() .find(|i| &i.apath == "/empty") .expect("found one entry"); @@ -374,7 +377,7 @@ pub fn empty_file_uses_zero_blocks() { &af, dest.path(), &RestoreOptions::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ) .expect("restore"); // TODO: Check restore stats. @@ -389,7 +392,7 @@ pub fn detect_unmodified() { srcdir.create_file("bbb"); let options = BackupOptions::default(); - let stats = backup(&af, srcdir.path(), &options, CollectMonitor::arc()).unwrap(); + let stats = backup(&af, srcdir.path(), &options, TestMonitor::arc()).unwrap(); assert_eq!(stats.files, 2); assert_eq!(stats.new_files, 2); @@ -397,7 +400,7 @@ pub fn detect_unmodified() { // Make a second backup from the same tree, and we should see that // both files are unmodified. - let stats = backup(&af, srcdir.path(), &options, CollectMonitor::arc()).unwrap(); + let stats = backup(&af, srcdir.path(), &options, TestMonitor::arc()).unwrap(); assert_eq!(stats.files, 2); assert_eq!(stats.new_files, 0); @@ -407,7 +410,7 @@ pub fn detect_unmodified() { // as unmodified. srcdir.create_file_with_contents("bbb", b"longer content for bbb"); - let stats = backup(&af, srcdir.path(), &options, CollectMonitor::arc()).unwrap(); + let stats = backup(&af, srcdir.path(), &options, TestMonitor::arc()).unwrap(); assert_eq!(stats.files, 2); assert_eq!(stats.new_files, 0); @@ -423,7 +426,7 @@ pub fn detect_minimal_mtime_change() { srcdir.create_file_with_contents("bbb", b"longer content for bbb"); let options = BackupOptions::default(); - let stats = backup(&af, srcdir.path(), &options, CollectMonitor::arc()).unwrap(); + let stats = backup(&af, srcdir.path(), &options, TestMonitor::arc()).unwrap(); assert_eq!(stats.files, 2); assert_eq!(stats.new_files, 2); @@ -445,7 +448,7 @@ pub fn detect_minimal_mtime_change() { } } - let stats = backup(&af, srcdir.path(), &options, CollectMonitor::arc()).unwrap(); + let stats = backup(&af, srcdir.path(), &options, TestMonitor::arc()).unwrap(); assert_eq!(stats.files, 2); assert_eq!(stats.unmodified_files, 1); } @@ -461,7 +464,7 @@ fn small_files_combined_two_backups() { &af, srcdir.path(), &BackupOptions::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ) .unwrap(); // Although the two files have the same content, we do not yet dedupe them @@ -479,7 +482,7 @@ fn small_files_combined_two_backups() { &af, srcdir.path(), &BackupOptions::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ) .unwrap(); assert_eq!(stats2.new_files, 1); @@ -488,10 +491,7 @@ fn small_files_combined_two_backups() { assert_eq!(stats2.combined_blocks, 1); assert_eq!( - af.block_dir() - .blocks(CollectMonitor::arc()) - .unwrap() - .count(), + af.block_dir().blocks(TestMonitor::arc()).unwrap().count(), 2 ); } @@ -513,7 +513,7 @@ fn many_small_files_combined_to_one_block() { max_entries_per_hunk: 1000, ..Default::default() }; - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let stats = backup(&af, srcdir.path(), &backup_options, monitor.clone()).expect("backup"); assert_eq!( monitor.get_counter(Counter::IndexWrites), @@ -533,14 +533,14 @@ fn many_small_files_combined_to_one_block() { let tree = af.open_stored_tree(BandSelectionPolicy::Latest).unwrap(); let mut entry_iter = tree - .iter_entries(Apath::root(), Exclude::nothing()) + .iter_entries(Apath::root(), Exclude::nothing(), TestMonitor::arc()) .unwrap(); assert_eq!(entry_iter.next().unwrap().apath(), "/"); for (i, entry) in entry_iter.enumerate() { assert_eq!(entry.apath().to_string(), format!("/file{i:04}")); } assert_eq!( - tree.iter_entries(Apath::root(), Exclude::nothing()) + tree.iter_entries(Apath::root(), Exclude::nothing(), TestMonitor::arc()) .unwrap() .count(), 2000 @@ -566,7 +566,7 @@ pub fn mixed_medium_small_files_two_hunks() { small_file_cap: 100_000, ..Default::default() }; - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let stats = backup(&af, srcdir.path(), &backup_options, monitor.clone()).expect("backup"); assert_eq!( monitor.get_counter(Counter::IndexWrites), @@ -586,14 +586,14 @@ pub fn mixed_medium_small_files_two_hunks() { let tree = af.open_stored_tree(BandSelectionPolicy::Latest).unwrap(); let mut entry_iter = tree - .iter_entries(Apath::root(), Exclude::nothing()) + .iter_entries(Apath::root(), Exclude::nothing(), TestMonitor::arc()) .unwrap(); assert_eq!(entry_iter.next().unwrap().apath(), "/"); for (i, entry) in entry_iter.enumerate() { assert_eq!(entry.apath().to_string(), format!("/file{i:04}")); } assert_eq!( - tree.iter_entries(Apath::root(), Exclude::nothing()) + tree.iter_entries(Apath::root(), Exclude::nothing(), TestMonitor::arc()) .unwrap() .count(), 2000 @@ -607,7 +607,7 @@ fn detect_unchanged_from_stitched_index() { srcdir.create_file("a"); srcdir.create_file("b"); // Use small hunks for easier manipulation. - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let stats = backup( &af, srcdir.path(), @@ -623,7 +623,7 @@ fn detect_unchanged_from_stitched_index() { assert_eq!(monitor.get_counter(Counter::IndexWrites), 3,); // Make a second backup, with the first file changed. - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); srcdir.create_file_with_contents("a", b"new a contents"); let stats = backup( &af, @@ -647,7 +647,7 @@ fn detect_unchanged_from_stitched_index() { // The third backup should see nothing changed, by looking at the stitched // index from both b0 and b1. - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let stats = backup( &af, srcdir.path(), diff --git a/tests/api/damaged.rs b/tests/api/damaged.rs index bc09842..f2bcbc0 100644 --- a/tests/api/damaged.rs +++ b/tests/api/damaged.rs @@ -1,5 +1,5 @@ // Conserve backup system. -// Copyright 2020-2023 Martin Pool. +// Copyright 2020-2024 Martin Pool. // This program is free software; you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -14,9 +14,8 @@ //! Test validation of archives with some problems. use std::path::Path; -use std::sync::Arc; -use conserve::monitor::collect::CollectMonitor; +use conserve::monitor::test::TestMonitor; use tracing_test::traced_test; use conserve::*; @@ -25,9 +24,14 @@ use conserve::*; #[test] fn missing_block_when_checking_hashes() -> Result<()> { let archive = Archive::open_path(Path::new("testdata/damaged/missing-block"))?; - archive.validate(&ValidateOptions::default(), Arc::new(CollectMonitor::new()))?; - assert!(logs_contain( - "Referenced block missing block_hash=fec91c70284c72d0d4e3684788a90de9338a5b2f47f01fedbe203cafd68708718ae5672d10eca804a8121904047d40d1d6cf11e7a76419357a9469af41f22d01")); + let monitor = TestMonitor::arc(); + archive + .validate(&ValidateOptions::default(), monitor.clone()) + .unwrap(); + let errors = monitor.take_errors(); + dbg!(&errors); + assert_eq!(errors.len(), 1); + assert!(matches!(errors[0], Error::BlockMissing { .. })); Ok(()) } @@ -35,13 +39,16 @@ fn missing_block_when_checking_hashes() -> Result<()> { #[test] fn missing_block_skip_block_hashes() -> Result<()> { let archive = Archive::open_path(Path::new("testdata/damaged/missing-block"))?; + let monitor = TestMonitor::arc(); archive.validate( &ValidateOptions { skip_block_hashes: true, }, - Arc::new(CollectMonitor::new()), + monitor.clone(), )?; - assert!(logs_contain( - "Referenced block missing block_hash=fec91c70284c72d0d4e3684788a90de9338a5b2f47f01fedbe203cafd68708718ae5672d10eca804a8121904047d40d1d6cf11e7a76419357a9469af41f22d01")); + let errors = monitor.take_errors(); + dbg!(&errors); + assert_eq!(errors.len(), 1); + assert!(matches!(errors[0], Error::BlockMissing { .. })); Ok(()) } diff --git a/tests/api/delete.rs b/tests/api/delete.rs index b616cf9..bd2a2ae 100644 --- a/tests/api/delete.rs +++ b/tests/api/delete.rs @@ -12,7 +12,7 @@ //! Test deletion. -use conserve::monitor::collect::CollectMonitor; +use conserve::monitor::test::TestMonitor; use conserve::test_fixtures::ScratchArchive; use conserve::*; @@ -25,7 +25,7 @@ fn delete_all_bands() { .delete_bands( &[BandId::new(&[0]), BandId::new(&[1])], &Default::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ) .expect("delete_bands"); diff --git a/tests/api/diff.rs b/tests/api/diff.rs index 1cd3dbf..07beeb5 100644 --- a/tests/api/diff.rs +++ b/tests/api/diff.rs @@ -15,7 +15,7 @@ use filetime::{set_file_mtime, FileTime}; use itertools::Itertools; -use conserve::monitor::collect::CollectMonitor; +use conserve::monitor::test::TestMonitor; use conserve::test_fixtures::{ScratchArchive, TreeFixture}; use conserve::*; @@ -24,13 +24,7 @@ fn create_tree() -> (ScratchArchive, TreeFixture) { let a = ScratchArchive::new(); let tf = TreeFixture::new(); tf.create_file_with_contents("thing", b"contents of thing"); - let stats = backup( - &a, - tf.path(), - &BackupOptions::default(), - CollectMonitor::arc(), - ) - .unwrap(); + let stats = backup(&a, tf.path(), &BackupOptions::default(), TestMonitor::arc()).unwrap(); assert_eq!(stats.new_files, 1); (a, tf) } @@ -45,7 +39,10 @@ fn diff_unchanged() { include_unchanged: true, ..DiffOptions::default() }; - let changes: Vec = diff(&st, &tf.live_tree(), &options).unwrap().collect(); + let monitor = TestMonitor::arc(); + let changes: Vec = diff(&st, &tf.live_tree(), &options, monitor.clone()) + .unwrap() + .collect(); dbg!(&changes); assert_eq!(changes.len(), 2); // Root directory and the file "/thing". assert_eq!(changes[0].apath, "/"); @@ -60,7 +57,9 @@ fn diff_unchanged() { include_unchanged: false, ..DiffOptions::default() }; - let changes = diff(&st, &tf.live_tree(), &options).unwrap().collect_vec(); + let changes = diff(&st, &tf.live_tree(), &options, TestMonitor::arc()) + .unwrap() + .collect_vec(); println!("changes with include_unchanged=false:\n{changes:#?}"); assert_eq!(changes.len(), 0); } @@ -80,7 +79,9 @@ fn mtime_only_change_reported_as_changed() { include_unchanged: false, ..DiffOptions::default() }; - let changes: Vec = diff(&st, &tf.live_tree(), &options).unwrap().collect(); + let changes: Vec = diff(&st, &tf.live_tree(), &options, TestMonitor::arc()) + .unwrap() + .collect(); dbg!(&changes); assert_eq!(changes.len(), 1); assert_eq!(changes[0].apath, "/thing"); @@ -111,7 +112,9 @@ fn chgrp_reported_as_changed() { include_unchanged: false, ..DiffOptions::default() }; - let changes: Vec = diff(&st, &tf.live_tree(), &options).unwrap().collect(); + let changes: Vec = diff(&st, &tf.live_tree(), &options, TestMonitor::arc()) + .unwrap() + .collect(); dbg!(&changes); assert_eq!(changes.len(), 1); assert_eq!(changes[0].apath, "/thing"); @@ -128,13 +131,7 @@ fn symlink_target_change_reported_as_changed() { let a = ScratchArchive::new(); let tf = TreeFixture::new(); tf.create_symlink("link", "target"); - backup( - &a, - tf.path(), - &BackupOptions::default(), - CollectMonitor::arc(), - ) - .unwrap(); + backup(&a, tf.path(), &BackupOptions::default(), TestMonitor::arc()).unwrap(); let link_path = tf.path().join("link"); remove_file(&link_path).unwrap(); @@ -149,7 +146,9 @@ fn symlink_target_change_reported_as_changed() { include_unchanged: false, ..DiffOptions::default() }; - let changes: Vec = diff(&st, &tf.live_tree(), &options).unwrap().collect(); + let changes: Vec = diff(&st, &tf.live_tree(), &options, TestMonitor::arc()) + .unwrap() + .collect(); dbg!(&changes); assert_eq!(changes.len(), 1); assert_eq!(changes[0].apath, "/link"); diff --git a/tests/api/gc.rs b/tests/api/gc.rs index e3ec5f4..ada988e 100644 --- a/tests/api/gc.rs +++ b/tests/api/gc.rs @@ -12,7 +12,7 @@ //! Test garbage collection. -use conserve::monitor::collect::CollectMonitor; +use conserve::monitor::test::TestMonitor; use conserve::test_fixtures::{ScratchArchive, TreeFixture}; use conserve::*; use rayon::prelude::ParallelIterator; @@ -32,13 +32,13 @@ fn unreferenced_blocks() { &archive, tf.path(), &BackupOptions::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ) .expect("backup"); // Delete the band and index std::fs::remove_dir_all(archive.path().join("b0000")).unwrap(); - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let unreferenced: Vec = archive .unreferenced_blocks(monitor.clone()) @@ -115,7 +115,7 @@ fn backup_prevented_by_gc_lock() -> Result<()> { let lock1 = GarbageCollectionLock::new(&archive)?; // Backup should fail while gc lock is held. - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let backup_result = backup( &archive, tf.path(), diff --git a/tests/api/live_tree.rs b/tests/api/live_tree.rs index afb6284..570d8c2 100644 --- a/tests/api/live_tree.rs +++ b/tests/api/live_tree.rs @@ -10,6 +10,7 @@ // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. +use conserve::monitor::test::TestMonitor; use pretty_assertions::assert_eq; use conserve::entry::EntryValue; @@ -34,7 +35,7 @@ fn list_simple_directory() { tf.create_dir("jam/.etc"); let lt = LiveTree::open(tf.path()).unwrap(); let result: Vec = lt - .iter_entries(Apath::root(), Exclude::nothing()) + .iter_entries(Apath::root(), Exclude::nothing(), TestMonitor::arc()) .unwrap() .collect(); let names = entry_iter_to_apath_strings(&result); @@ -76,7 +77,10 @@ fn exclude_entries_directory() { let exclude = Exclude::from_strings(["/**/fooo*", "/**/ba[pqr]", "/**/*bas"]).unwrap(); let lt = LiveTree::open(tf.path()).unwrap(); - let names = entry_iter_to_apath_strings(lt.iter_entries(Apath::root(), exclude).unwrap()); + let names = entry_iter_to_apath_strings( + lt.iter_entries(Apath::root(), exclude, TestMonitor::arc()) + .unwrap(), + ); // First one is the root assert_eq!(names, ["/", "/baz", "/baz/test"]); @@ -94,8 +98,10 @@ fn symlinks() { tf.create_symlink("from", "to"); let lt = LiveTree::open(tf.path()).unwrap(); - let names = - entry_iter_to_apath_strings(lt.iter_entries(Apath::root(), Exclude::nothing()).unwrap()); + let names = entry_iter_to_apath_strings( + lt.iter_entries(Apath::root(), Exclude::nothing(), TestMonitor::arc()) + .unwrap(), + ); assert_eq!(names, ["/", "/from"]); } @@ -112,7 +118,7 @@ fn iter_subtree_entries() { let lt = LiveTree::open(tf.path()).unwrap(); let names = entry_iter_to_apath_strings( - lt.iter_entries("/subdir".into(), Exclude::nothing()) + lt.iter_entries("/subdir".into(), Exclude::nothing(), TestMonitor::arc()) .unwrap(), ); assert_eq!(names, ["/subdir", "/subdir/a", "/subdir/b"]); @@ -127,7 +133,9 @@ fn exclude_cachedir() { cachedir::add_tag(cache_dir).unwrap(); let lt = LiveTree::open(tf.path()).unwrap(); - let names = - entry_iter_to_apath_strings(lt.iter_entries(Apath::root(), Exclude::nothing()).unwrap()); + let names = entry_iter_to_apath_strings( + lt.iter_entries(Apath::root(), Exclude::nothing(), TestMonitor::arc()) + .unwrap(), + ); assert_eq!(names, ["/", "/a"]); } diff --git a/tests/api/old_archives.rs b/tests/api/old_archives.rs index 00188b4..f7f075b 100644 --- a/tests/api/old_archives.rs +++ b/tests/api/old_archives.rs @@ -21,7 +21,8 @@ use std::sync::Arc; use assert_fs::prelude::*; use assert_fs::TempDir; -use conserve::monitor::collect::CollectMonitor; +use conserve::counters::Counter; +use conserve::monitor::test::TestMonitor; use predicates::prelude::*; use pretty_assertions::assert_eq; @@ -81,7 +82,7 @@ fn validate_archive() { let archive = open_old_archive(ver, "minimal"); archive - .validate(&ValidateOptions::default(), Arc::new(CollectMonitor::new())) + .validate(&ValidateOptions::default(), Arc::new(TestMonitor::new())) .expect("validate archive"); assert!(!logs_contain("ERROR") && !logs_contain("WARN")); } @@ -99,16 +100,18 @@ fn long_listing_old_archive() { let mut stdout = Vec::::new(); // show archive contents + let monitor = TestMonitor::arc(); show::show_entry_names( archive .open_stored_tree(BandSelectionPolicy::Latest) .unwrap() - .iter_entries(Apath::root(), Exclude::nothing()) + .iter_entries(Apath::root(), Exclude::nothing(), monitor.clone()) .unwrap(), &mut stdout, true, ) .unwrap(); + monitor.assert_no_errors(); if first_with_perms.matches(&semver::Version::parse(ver).unwrap()) { assert_eq!( @@ -139,18 +142,19 @@ fn restore_old_archive() { println!("restore {} to {:?}", ver, dest.path()); let archive = open_old_archive(ver, "minimal"); - let restore_stats = restore( + let monitor = TestMonitor::arc(); + restore( &archive, dest.path(), &RestoreOptions::default(), - CollectMonitor::arc(), + monitor.clone(), ) .expect("restore"); - assert_eq!(restore_stats.files, 2); - assert_eq!(restore_stats.symlinks, 0); - assert_eq!(restore_stats.directories, 2); - assert_eq!(restore_stats.errors, 0); + monitor.assert_counter(Counter::Symlinks, 0); + monitor.assert_counter(Counter::Files, 2); + monitor.assert_counter(Counter::Dirs, 2); + monitor.assert_no_errors(); dest.child("hello").assert("hello world\n"); dest.child("subdir").assert(predicate::path::is_dir()); @@ -196,7 +200,7 @@ fn restore_modify_backup() { &archive, working_tree.path(), &RestoreOptions::default(), - CollectMonitor::arc(), + TestMonitor::arc(), ) .expect("restore"); @@ -228,7 +232,7 @@ fn restore_modify_backup() { })), ..Default::default() }, - CollectMonitor::arc(), + TestMonitor::arc(), ) .expect("Backup modified tree"); diff --git a/tests/api/owner.rs b/tests/api/owner.rs index 9947021..b63bc73 100644 --- a/tests/api/owner.rs +++ b/tests/api/owner.rs @@ -13,7 +13,7 @@ //! Tests for storing file ownership/user/group. use conserve::backup::{backup, BackupOptions}; -use conserve::monitor::collect::CollectMonitor; +use conserve::monitor::test::TestMonitor; use conserve::{restore, Archive, RestoreOptions}; use crate::copy_testdata_archive; @@ -31,7 +31,7 @@ fn adding_owners_to_old_archive_does_not_rewrite_blocks() { let archive_temp = copy_testdata_archive("minimal", "0.6.0"); let restore_temp = tempfile::tempdir().expect("create temp dir"); let archive = Archive::open_path(archive_temp.path()).expect("open archive"); - let restore_monitor = CollectMonitor::arc(); + let restore_monitor = TestMonitor::arc(); restore( &archive, restore_temp.path(), @@ -41,7 +41,7 @@ fn adding_owners_to_old_archive_does_not_rewrite_blocks() { .expect("restore"); // Now backup again without making any changes. - let backup_monitor = CollectMonitor::arc(); + let backup_monitor = TestMonitor::arc(); let stats = backup( &archive, restore_temp.path(), diff --git a/tests/api/restore.rs b/tests/api/restore.rs index c31baf4..d5505c6 100644 --- a/tests/api/restore.rs +++ b/tests/api/restore.rs @@ -17,7 +17,8 @@ use std::cell::RefCell; use std::fs::{read_link, symlink_metadata}; use std::path::PathBuf; -use conserve::monitor::collect::CollectMonitor; +use conserve::counters::Counter; +use conserve::monitor::test::TestMonitor; use filetime::{set_symlink_file_times, FileTime}; use tempfile::TempDir; @@ -39,15 +40,11 @@ fn simple_restore() { })), ..Default::default() }; - let stats = restore( - &restore_archive, - destdir.path(), - &options, - CollectMonitor::arc(), - ) - .expect("restore"); + let monitor = TestMonitor::arc(); + restore(&restore_archive, destdir.path(), &options, monitor.clone()).expect("restore"); - assert_eq!(stats.files, 3); + monitor.assert_no_errors(); + monitor.assert_counter(Counter::Files, 3); let mut expected_names = vec![ "/", "/hello", @@ -86,10 +83,11 @@ fn restore_specified_band() { band_selection: BandSelectionPolicy::Specified(band_id), ..RestoreOptions::default() }; - let stats = - restore(&archive, destdir.path(), &options, CollectMonitor::arc()).expect("restore"); + let monitor = TestMonitor::arc(); + restore(&archive, destdir.path(), &options, monitor.clone()).expect("restore"); + monitor.assert_no_errors(); // Does not have the 'hello2' file added in the second version. - assert_eq!(stats.files, 2); + monitor.assert_counter(Counter::Files, 2); } #[test] @@ -102,7 +100,7 @@ pub fn decline_to_overwrite() { ..RestoreOptions::default() }; assert!(!options.overwrite, "overwrite is false by default"); - let restore_err_str = restore(&af, destdir.path(), &options, CollectMonitor::arc()) + let restore_err_str = restore(&af, destdir.path(), &options, TestMonitor::arc()) .expect_err("restore should fail if the destination exists") .to_string(); assert!( @@ -123,15 +121,11 @@ pub fn forced_overwrite() { overwrite: true, ..RestoreOptions::default() }; - let stats = restore( - &restore_archive, - destdir.path(), - &options, - CollectMonitor::arc(), - ) - .expect("restore"); - assert_eq!(stats.files, 3); - let dest = &destdir.path(); + let monitor = TestMonitor::arc(); + restore(&restore_archive, destdir.path(), &options, monitor.clone()).expect("restore"); + monitor.assert_no_errors(); + monitor.assert_counter(Counter::Files, 3); + let dest = destdir.path(); assert!(dest.join("hello").is_file()); assert!(dest.join("existing").is_file()); } @@ -147,25 +141,21 @@ fn exclude_files() { exclude: Exclude::from_strings(["/**/subfile"]).unwrap(), ..RestoreOptions::default() }; - let stats = restore( - &restore_archive, - destdir.path(), - &options, - CollectMonitor::arc(), - ) - .expect("restore"); + let monitor = TestMonitor::arc(); + restore(&restore_archive, destdir.path(), &options, monitor.clone()).expect("restore"); - let dest = &destdir.path(); + let dest = destdir.path(); assert!(dest.join("hello").is_file()); assert!(dest.join("hello2").is_file()); assert!(dest.join("subdir").is_dir()); - assert_eq!(stats.files, 2); + monitor.assert_no_errors(); + monitor.assert_counter(Counter::Files, 2); } #[test] #[cfg(unix)] fn restore_symlink() { - use conserve::monitor::collect::CollectMonitor; + use conserve::monitor::test::TestMonitor; let af = ScratchArchive::new(); let srcdir = TreeFixture::new(); @@ -174,20 +164,16 @@ fn restore_symlink() { let years_ago = FileTime::from_unix_time(189216000, 0); set_symlink_file_times(srcdir.path().join("symlink"), years_ago, years_ago).unwrap(); - backup( - &af, - srcdir.path(), - &Default::default(), - CollectMonitor::arc(), - ) - .unwrap(); + let monitor = TestMonitor::arc(); + backup(&af, srcdir.path(), &Default::default(), monitor.clone()).unwrap(); let restore_dir = TempDir::new().unwrap(); + let monitor = TestMonitor::arc(); restore( &af, restore_dir.path(), &Default::default(), - CollectMonitor::arc(), + monitor.clone(), ) .unwrap(); diff --git a/tests/cli/delete.rs b/tests/cli/delete.rs index b7ac903..3130779 100644 --- a/tests/cli/delete.rs +++ b/tests/cli/delete.rs @@ -16,7 +16,7 @@ use assert_cmd::prelude::*; use assert_fs::prelude::*; use assert_fs::TempDir; -use conserve::monitor::collect::CollectMonitor; +use conserve::monitor::test::TestMonitor; use predicates::prelude::*; use conserve::test_fixtures::ScratchArchive; @@ -40,10 +40,7 @@ fn delete_both_bands() { assert_eq!(af.list_band_ids().unwrap().len(), 0); assert_eq!( - af.block_dir() - .blocks(CollectMonitor::arc()) - .unwrap() - .count(), + af.block_dir().blocks(TestMonitor::arc()).unwrap().count(), 0 ); } @@ -64,10 +61,7 @@ fn delete_first_version() { // b0 contains two small files packed into the same block, which is not deleted. // b1 (not deleted) adds one additional block, which is still referenced. assert_eq!( - af.block_dir() - .blocks(CollectMonitor::arc()) - .unwrap() - .count(), + af.block_dir().blocks(TestMonitor::arc()).unwrap().count(), 2 ); @@ -106,10 +100,7 @@ fn delete_second_version() { assert_eq!(af.list_band_ids().unwrap(), &[BandId::new(&[0])]); // b0 contains two small files packed into the same block. assert_eq!( - af.block_dir() - .blocks(CollectMonitor::arc()) - .unwrap() - .count(), + af.block_dir().blocks(TestMonitor::arc()).unwrap().count(), 1 ); diff --git a/tests/cli/validate.rs b/tests/cli/validate.rs index a88887a..4fc27cd 100644 --- a/tests/cli/validate.rs +++ b/tests/cli/validate.rs @@ -77,12 +77,12 @@ fn validate_non_fatal_problems_nonzero_result_and_json_log() { let events = read_log_json(log_temp.path()); dbg!(&events); let errors = filter_by_level(&events, Level::ERROR); + // TODO: Write errors to json, read that json too. assert_eq!(errors.len(), 1); assert_eq!( errors[0]["fields"], json!({ - "block_hash": "fec91c70284c72d0d4e3684788a90de9338a5b2f47f01fedbe203cafd68708718ae5672d10eca804a8121904047d40d1d6cf11e7a76419357a9469af41f22d01", - "message": "Referenced block missing", + "message": "Referenced block fec91c70284c72d0d4e3684788a90de9338a5b2f47f01fedbe203cafd68708718ae5672d10eca804a8121904047d40d1d6cf11e7a76419357a9469af41f22d01 is missing", }) ); } diff --git a/tests/damage/damage.rs b/tests/damage/damage.rs index 7b02e98..f3e3452 100644 --- a/tests/damage/damage.rs +++ b/tests/damage/damage.rs @@ -16,7 +16,7 @@ use std::fs::{remove_file, OpenOptions}; use std::path::{Path, PathBuf}; -use conserve::monitor::collect::CollectMonitor; +use conserve::monitor::test::TestMonitor; use conserve::transport::open_local_transport; use conserve::{Archive, BandId, BlockHash}; use itertools::Itertools; @@ -85,7 +85,7 @@ impl DamageLocation { .expect("open archive"); let block_dir = archive.block_dir(); let block_hash = block_dir - .blocks(CollectMonitor::arc()) + .blocks(TestMonitor::arc()) .expect("list blocks") .collect::>() .into_iter() diff --git a/tests/damage/main.rs b/tests/damage/main.rs index b7ab008..e7ad30e 100644 --- a/tests/damage/main.rs +++ b/tests/damage/main.rs @@ -23,7 +23,8 @@ use rstest::rstest; use tracing_test::traced_test; // use predicates::prelude::*; -use conserve::monitor::collect::CollectMonitor; +use conserve::counters::Counter; +use conserve::monitor::test::TestMonitor; use conserve::{ backup, restore, Apath, Archive, BackupOptions, BandId, BandSelectionPolicy, EntryTrait, Exclude, RestoreOptions, ValidateOptions, @@ -85,7 +86,7 @@ fn backup_after_damage( &archive, source_dir.path(), &backup_options, - CollectMonitor::arc(), + TestMonitor::arc(), ) .expect("initial backup"); @@ -103,7 +104,7 @@ fn backup_after_damage( &archive, source_dir.path(), &backup_options, - CollectMonitor::arc(), + TestMonitor::arc(), ) .expect("write second backup after damage"); dbg!(&backup_stats); @@ -139,21 +140,23 @@ fn backup_after_damage( } // Can restore the second backup - let restore_dir = TempDir::new().unwrap(); - let restore_stats = restore( - &archive, - restore_dir.path(), - &RestoreOptions::default(), - CollectMonitor::arc(), - ) - .expect("restore second backup"); - dbg!(&restore_stats); - assert_eq!(restore_stats.files, 1); - assert_eq!(restore_stats.errors, 0); + { + let restore_dir = TempDir::new().unwrap(); + let monitor = TestMonitor::arc(); + restore( + &archive, + restore_dir.path(), + &RestoreOptions::default(), + monitor.clone(), + ) + .expect("restore second backup"); + monitor.assert_counter(Counter::Files, 1); + monitor.assert_no_errors(); - // Since the second backup rewrote the single file in the backup (and the root dir), - // we should get all the content back out. - assert_paths!(source_dir.path(), restore_dir.path()); + // Since the second backup rewrote the single file in the backup (and the root dir), + // we should get all the content back out. + assert_paths!(source_dir.path(), restore_dir.path()); + } // You can see both versions. let versions = archive.list_band_ids().expect("list versions"); @@ -165,6 +168,7 @@ fn backup_after_damage( BandSelectionPolicy::Latest, Apath::root(), Exclude::nothing(), + TestMonitor::arc(), ) .expect("iter entries") .map(|e| e.apath().to_string()) @@ -179,6 +183,6 @@ fn backup_after_damage( // Validation completes although with warnings. // TODO: This should return problems that we can inspect. archive - .validate(&ValidateOptions::default(), Arc::new(CollectMonitor::new())) + .validate(&ValidateOptions::default(), Arc::new(TestMonitor::new())) .expect("validate"); } diff --git a/tests/expensive/changes.rs b/tests/expensive/changes.rs index 9c35dd3..ae54a9f 100644 --- a/tests/expensive/changes.rs +++ b/tests/expensive/changes.rs @@ -17,7 +17,7 @@ use std::collections::BTreeMap; use std::fs; use std::path::Path; -use conserve::monitor::collect::CollectMonitor; +use conserve::monitor::test::TestMonitor; use proptest::prelude::*; use proptest_derive::Arbitrary; use tempfile::TempDir; @@ -94,7 +94,7 @@ fn backup_sequential_changes(changes: &[TreeChange]) { max_entries_per_hunk: 3, ..BackupOptions::default() }; - backup(&archive, tf.path(), &options, CollectMonitor::arc()).unwrap(); + backup(&archive, tf.path(), &options, TestMonitor::arc()).unwrap(); let snapshot = TempDir::new().unwrap(); cp_r::CopyOptions::default() .copy_tree(tf.path(), snapshot.path()) @@ -126,7 +126,7 @@ fn check_restore_against_snapshot(archive: &Archive, band_id: BandId, snapshot: band_selection: BandSelectionPolicy::Specified(band_id), ..RestoreOptions::default() }; - restore(archive, restore_dir.path(), &options, CollectMonitor::arc()).unwrap(); + restore(archive, restore_dir.path(), &options, TestMonitor::arc()).unwrap(); dir_assert::assert_paths(restore_dir.path(), snapshot).unwrap(); } diff --git a/tests/failpoints/README.md b/tests/failpoints/README.md new file mode 100644 index 0000000..4b98071 --- /dev/null +++ b/tests/failpoints/README.md @@ -0,0 +1,9 @@ +# Conserve failpoint tests + +The tests in this directory simulate IO errors or other failures in Conserve, and assert that Conserve handles and reports them correctly. + +Failpoints aren't built by default. + +To run them use + + cargo test --features fail/failpoints --test failpoints diff --git a/tests/failpoints/restore.rs b/tests/failpoints/restore.rs index 74c2300..83241ab 100644 --- a/tests/failpoints/restore.rs +++ b/tests/failpoints/restore.rs @@ -2,10 +2,11 @@ //! Simulate IO errors during restore. +use std::io; use std::path::Path; use assert_fs::TempDir; -use conserve::monitor::collect::CollectMonitor; +use conserve::monitor::test::TestMonitor; use conserve::transport::open_local_transport; use fail::FailScenario; @@ -14,7 +15,7 @@ use conserve::*; #[test] fn create_dir_permission_denied() { let scenario = FailScenario::setup(); - fail::cfg("conserve::restore::create-dir", "return").unwrap(); + fail::cfg("restore::create-dir", "return").unwrap(); let archive = Archive::open(open_local_transport(Path::new("testdata/archive/simple/v0.6.10")).unwrap()) .unwrap(); @@ -22,11 +23,23 @@ fn create_dir_permission_denied() { ..RestoreOptions::default() }; let restore_tmp = TempDir::new().unwrap(); - let monitor = CollectMonitor::arc(); + let monitor = TestMonitor::arc(); let stats = restore(&archive, restore_tmp.path(), &options, monitor.clone()).expect("Restore"); dbg!(&stats); - dbg!(&monitor.problems.lock().unwrap()); - assert_eq!(stats.errors, 3); - // TODO: Check that the monitor saw the errors too, once that's hooked up. + let errors = monitor.take_errors(); + dbg!(&errors); + assert_eq!(errors.len(), 2); + if let Error::RestoreDirectory { path, .. } = &errors[0] { + assert!(path.ends_with("subdir")); + } else { + panic!("Unexpected error {:?}", errors[0]); + } + // Also, since we didn't create the directory, we fail to create the file within it. + if let Error::RestoreFile { path, source } = &errors[1] { + assert!(path.ends_with("subdir/subfile")); + assert_eq!(source.kind(), io::ErrorKind::NotFound); + } else { + panic!("Unexpected error {:?}", errors[1]); + } scenario.teardown(); }