Skip to content

Commit

Permalink
Fix notify-debouncer-full by removing mock_instant crate
Browse files Browse the repository at this point in the history
Trying to use mock_instant across multiple crates was a bad idea. All functions requiring a clock were removed from the notify-types crate and custom time mocking functions were added.
  • Loading branch information
dfaust committed Oct 25, 2024
1 parent e87b8dd commit c601b48
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 54 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ kqueue = "1.0.8"
libc = "0.2.4"
log = "0.4.17"
mio = { version = "1.0", features = ["os-ext"] }
mock_instant = "0.3.0"
instant = "0.1.12"
nix = "0.27.0"
notify = { path = "notify" }
Expand Down
3 changes: 0 additions & 3 deletions notify-debouncer-full/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ repository.workspace = true
[features]
default = ["macos_fsevent"]
serde = ["notify-types/serde"]
mock_instant = ["dep:mock_instant", "notify-types/mock_instant"]
crossbeam-channel = ["dep:crossbeam-channel", "notify/crossbeam-channel"]
macos_fsevent = ["notify/macos_fsevent"]
macos_kqueue = ["notify/macos_kqueue"]
Expand All @@ -28,10 +27,8 @@ crossbeam-channel = { workspace = true, optional = true }
file-id.workspace = true
walkdir.workspace = true
log.workspace = true
mock_instant = { workspace = true, optional = true }

[dev-dependencies]
notify-debouncer-full = { workspace = true, features = ["mock_instant"] }
pretty_assertions.workspace = true
rstest.workspace = true
serde.workspace = true
Expand Down
49 changes: 24 additions & 25 deletions notify-debouncer-full/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
//! As all file events are sourced from notify, the [known problems](https://docs.rs/notify/latest/notify/#known-problems) section applies here too.
mod cache;
mod time;

#[cfg(test)]
mod testing;
Expand All @@ -69,9 +70,11 @@ use std::{
atomic::{AtomicBool, Ordering},
Arc, Mutex,
},
time::Duration,
time::{Duration, Instant},
};

use time::now;

pub use cache::{FileIdCache, FileIdMap, NoCache, RecommendedCache};

pub use file_id;
Expand All @@ -84,12 +87,6 @@ use notify::{
Error, ErrorKind, Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher, WatcherKind,
};

#[cfg(feature = "mock_instant")]
use mock_instant::Instant;

#[cfg(not(feature = "mock_instant"))]
use std::time::Instant;

/// The set of requirements for watcher debounce event handling functions.
///
/// # Example implementation
Expand Down Expand Up @@ -198,7 +195,7 @@ impl<T: FileIdCache> DebounceDataInner<T> {

/// Retrieve a vec of debounced events, removing them if not continuous
pub fn debounced_events(&mut self) -> Vec<DebouncedEvent> {
let now = Instant::now();
let now = now();
let mut events_expired = Vec::with_capacity(self.queues.len());
let mut queues_remaining = HashMap::with_capacity(self.queues.len());

Expand Down Expand Up @@ -265,7 +262,7 @@ impl<T: FileIdCache> DebounceDataInner<T> {

if event.need_rescan() {
self.cache.rescan(&self.roots);
self.rescan_event = Some(event.into());
self.rescan_event = Some(DebouncedEvent { event, time: now() });
return;
}

Expand All @@ -277,7 +274,7 @@ impl<T: FileIdCache> DebounceDataInner<T> {

self.cache.add_path(path, recursive_mode);

self.push_event(event, Instant::now());
self.push_event(event, now());
}
EventKind::Modify(ModifyKind::Name(rename_mode)) => {
match rename_mode {
Expand All @@ -303,7 +300,7 @@ impl<T: FileIdCache> DebounceDataInner<T> {
}
}
EventKind::Remove(_) => {
self.push_remove_event(event, Instant::now());
self.push_remove_event(event, now());
}
EventKind::Other => {
// ignore meta events
Expand All @@ -315,7 +312,7 @@ impl<T: FileIdCache> DebounceDataInner<T> {
self.cache.add_path(path, recursive_mode);
}

self.push_event(event, Instant::now());
self.push_event(event, now());
}
}
}
Expand All @@ -334,7 +331,7 @@ impl<T: FileIdCache> DebounceDataInner<T> {
}

fn handle_rename_from(&mut self, event: Event) {
let time = Instant::now();
let time = now();
let path = &event.paths[0];

// store event
Expand Down Expand Up @@ -382,7 +379,7 @@ impl<T: FileIdCache> DebounceDataInner<T> {
self.push_rename_event(path, event, time);
} else {
// move in
self.push_event(event, Instant::now());
self.push_event(event, now());
}

self.rename_event = None;
Expand Down Expand Up @@ -753,10 +750,10 @@ mod tests {

use super::*;

use mock_instant::MockClock;
use pretty_assertions::assert_eq;
use rstest::rstest;
use testing::TestCase;
use time::MockTime;

#[rstest]
fn state(
Expand Down Expand Up @@ -805,16 +802,19 @@ mod tests {
fs::read_to_string(Path::new(&format!("./test_cases/{file_name}.hjson"))).unwrap();
let mut test_case = deser_hjson::from_str::<TestCase>(&file_content).unwrap();

MockClock::set_time(Duration::default());

let time = Instant::now();
let time = now();
MockTime::set_time(time);

let mut state = test_case.state.into_debounce_data_inner(time);
state.roots = vec![(PathBuf::from("/"), RecursiveMode::Recursive)];

let mut prev_event_time = Duration::default();

for event in test_case.events {
let event_time = Duration::from_millis(event.time);
let event = event.into_debounced_event(time, None);
MockClock::set_time(event.time - time);
MockTime::advance(event_time - prev_event_time);
prev_event_time = event_time;
state.add_event(event.event);
}

Expand Down Expand Up @@ -856,21 +856,20 @@ mod tests {
"errors not as expected"
);

let backup_time = Instant::now().duration_since(time);
let backup_time = now();
let backup_queues = state.queues.clone();

for (delay, events) in expected_events {
MockClock::set_time(backup_time);
MockTime::set_time(backup_time);
state.queues = backup_queues.clone();

match delay.as_str() {
"none" => {}
"short" => MockClock::advance(Duration::from_millis(10)),
"long" => MockClock::advance(Duration::from_millis(100)),
"short" => MockTime::advance(Duration::from_millis(10)),
"long" => MockTime::advance(Duration::from_millis(100)),
_ => {
if let Ok(ts) = delay.parse::<u64>() {
let ts = time + Duration::from_millis(ts);
MockClock::set_time(ts - time);
MockTime::set_time(time + Duration::from_millis(ts));
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions notify-debouncer-full/src/testing.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
use std::{
collections::{HashMap, VecDeque},
path::{Path, PathBuf},
time::Duration,
time::{Duration, Instant},
};

use file_id::FileId;
use mock_instant::Instant;
use notify::{
event::{
AccessKind, AccessMode, CreateKind, DataChange, Flag, MetadataKind, ModifyKind, RemoveKind,
Expand Down
47 changes: 47 additions & 0 deletions notify-debouncer-full/src/time.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#[cfg(not(test))]
mod build {
use std::time::Instant;

pub fn now() -> Instant {
Instant::now()
}
}

#[cfg(not(test))]
pub use build::*;

#[cfg(test)]
mod test {
use std::{
sync::Mutex,
time::{Duration, Instant},
};

thread_local! {
static NOW: Mutex<Option<Instant>> = Mutex::new(None);
}

pub fn now() -> Instant {
let time = NOW.with(|now| *now.lock().unwrap());
time.unwrap_or_else(|| Instant::now())
}

pub struct MockTime;

impl MockTime {
pub fn set_time(time: Instant) {
NOW.with(|now| *now.lock().unwrap() = Some(time));
}

pub fn advance(delta: Duration) {
NOW.with(|now| {
if let Some(n) = &mut *now.lock().unwrap() {
*n += delta;
}
});
}
}
}

#[cfg(test)]
pub use test::*;
1 change: 0 additions & 1 deletion notify-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ serialization-compat-6 = []

[dependencies]
serde = { workspace = true, optional = true }
mock_instant = { workspace = true, optional = true }
instant.workspace = true

[dev-dependencies]
Expand Down
22 changes: 0 additions & 22 deletions notify-types/src/debouncer_full.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
use std::ops::{Deref, DerefMut};

#[cfg(feature = "mock_instant")]
use mock_instant::Instant;

#[cfg(not(feature = "mock_instant"))]
use instant::Instant;

use crate::event::Event;
Expand Down Expand Up @@ -37,21 +33,3 @@ impl DerefMut for DebouncedEvent {
&mut self.event
}
}

impl Default for DebouncedEvent {
fn default() -> Self {
Self {
event: Default::default(),
time: Instant::now(),
}
}
}

impl From<Event> for DebouncedEvent {
fn from(event: Event) -> Self {
Self {
event,
time: Instant::now(),
}
}
}

0 comments on commit c601b48

Please sign in to comment.