Skip to content

Commit 030ce44

Browse files
authored
fix: osx file reload (#194)
fixes #60 As explained in the [notify documentation](https://github.com/notify-rs/notify/blob/ded07f442a96f33c6b7fefe3195396a33a28ddc3/src/lib.rs#L413) the file Create event has a higher priority than the Write event. However our tests create the file and almost immediately start writing to it. The watcher did not propagate Create events, which lead to flakiness in these tests. I'm lacking context but given the `tokio::time::sleep()`s in the tests, this issue might have been seen on Linux as well, and worked around. This commit removes the sleep()s in the test write functions, allows us to listen to Create events as well as Write events, and re enables the tests on osx.
1 parent 5a3e34c commit 030ce44

File tree

2 files changed

+22
-14
lines changed

2 files changed

+22
-14
lines changed

Diff for: crates/apollo-router/src/files.rs

+22-12
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,29 @@ pub(crate) fn watch(path: PathBuf, delay: Option<Duration>) -> impl Stream<Item
2121
.expect("Failed to initialise file watching.");
2222
watcher
2323
.watch(path, move |event: hotwatch::Event| {
24-
if let hotwatch::Event::Write(_path) = event {
25-
if let Err(_err) = watch_sender.try_send(()) {
26-
tracing::error!(
27-
"Failed to process file watch notification. {}",
28-
_err.to_string()
29-
)
24+
tracing::debug!("file watcher: received event: {:?}", &event);
25+
match event {
26+
// https://github.com/notify-rs/notify/blob/ded07f442a96f33c6b7fefe3195396a33a28ddc3/src/lib.rs#L413
27+
//
28+
// `Create` events have a higher priority than `Write` and `Chmod`. These events will not be
29+
// emitted if they are detected before the `Create` event has been emitted.
30+
//
31+
// Hotwatch will sometimes send a Create event instead of a Write event,
32+
// if the write occured immediately after the file creation.
33+
hotwatch::Event::Write(_) | hotwatch::Event::Create(_) => {
34+
if let Err(_err) = watch_sender.try_send(()) {
35+
tracing::error!(
36+
"Failed to process file watch notification. {}",
37+
_err.to_string()
38+
)
39+
}
3040
}
41+
_ => {}
3142
}
3243
})
3344
.expect("Failed to watch file.");
45+
// Tell watchers once they should read the file once,
46+
// then listen to fs events.
3447
stream::once(future::ready(()))
3548
.chain(watch_receiver)
3649
.chain(stream::once(async move {
@@ -50,21 +63,19 @@ pub(crate) mod tests {
5063
use std::env::temp_dir;
5164
use std::fs::File;
5265
use std::io::{Seek, SeekFrom, Write};
53-
#[cfg(not(target_os = "macos"))]
5466
use test_log::test;
5567

56-
#[cfg(not(target_os = "macos"))]
5768
#[test(tokio::test)]
5869
async fn basic_watch() {
5970
let (path, mut file) = create_temp_file();
6071
let mut watch = watch(path, Some(Duration::from_millis(10)));
61-
watch.next().await;
72+
// Signal telling us to read the file once, and then poll.
73+
assert!(futures::poll!(watch.next()).is_ready());
74+
6275
assert!(futures::poll!(watch.next()).is_pending());
6376
write_and_flush(&mut file, "Some data").await;
64-
watch.next().await;
6577
assert!(futures::poll!(watch.next()).is_pending());
6678
write_and_flush(&mut file, "Some data").await;
67-
watch.next().await;
6879
assert!(futures::poll!(watch.next()).is_pending())
6980
}
7081

@@ -81,6 +92,5 @@ pub(crate) mod tests {
8192
file.seek(SeekFrom::Start(0)).unwrap();
8293
file.write_all(contents.as_bytes()).unwrap();
8394
file.flush().unwrap();
84-
tokio::time::sleep(Duration::from_millis(20)).await;
8595
}
8696
}

Diff for: crates/apollo-router/src/lib.rs

-2
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,6 @@ mod tests {
644644
.await
645645
}
646646

647-
#[cfg(not(target_os = "macos"))]
648647
#[test(tokio::test)]
649648
async fn config_by_file_watching() {
650649
let (path, mut file) = create_temp_file();
@@ -723,7 +722,6 @@ mod tests {
723722
assert!(matches!(stream.next().await.unwrap(), NoMoreConfiguration));
724723
}
725724

726-
#[cfg(not(target_os = "macos"))]
727725
#[test(tokio::test)]
728726
async fn schema_by_file_watching() {
729727
let (path, mut file) = create_temp_file();

0 commit comments

Comments
 (0)