From 2ffd798bda86734d75f71f5c1b5e58cd4336b27d Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Sat, 25 Apr 2020 16:54:04 -0700 Subject: [PATCH] Extract a `watch` crate. [ci skip-jvm-tests] --- src/rust/engine/Cargo.lock | 25 +++- src/rust/engine/Cargo.toml | 10 +- src/rust/engine/graph/src/lib.rs | 37 ----- src/rust/engine/src/context.rs | 40 ++++- src/rust/engine/src/lib.rs | 4 - src/rust/engine/src/scheduler.rs | 5 +- src/rust/engine/testutil/src/lib.rs | 2 +- src/rust/engine/watch/Cargo.toml | 29 ++++ .../engine/{src/watch.rs => watch/src/lib.rs} | 75 +++++----- .../watch_tests.rs => watch/src/tests.rs} | 141 ++++++++---------- 10 files changed, 196 insertions(+), 172 deletions(-) create mode 100644 src/rust/engine/watch/Cargo.toml rename src/rust/engine/{src/watch.rs => watch/src/lib.rs} (83%) rename src/rust/engine/{src/watch_tests.rs => watch/src/tests.rs} (52%) diff --git a/src/rust/engine/Cargo.lock b/src/rust/engine/Cargo.lock index c86a6816fcd..bb42738a2e5 100644 --- a/src/rust/engine/Cargo.lock +++ b/src/rust/engine/Cargo.lock @@ -692,13 +692,11 @@ dependencies = [ "boxfuture 0.0.1", "bytes 0.4.12 (registry+https://github.com/rust-lang/crates.io-index)", "concrete_time 0.0.1", - "crossbeam-channel 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.5.13 (registry+https://github.com/rust-lang/crates.io-index)", "fnv 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)", "fs 0.0.1", "futures 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)", "futures 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)", - "futures-locks 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", "graph 0.0.1", "hashing 0.0.1", "indexmap 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -706,7 +704,6 @@ dependencies = [ "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "logging 0.0.1", - "notify 5.0.0-pre.1 (git+https://github.com/notify-rs/notify?rev=fba00891d9105e2f581c69fbe415a58cb7966fdd)", "num_cpus 1.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "num_enum 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -725,6 +722,7 @@ dependencies = [ "ui 0.0.1", "url 2.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "uuid 0.7.4 (registry+https://github.com/rust-lang/crates.io-index)", + "watch 0.0.1", "workunit_store 0.0.1", ] @@ -3565,6 +3563,27 @@ name = "wasm-bindgen-shared" version = "0.2.59" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "watch" +version = "0.0.1" +dependencies = [ + "crossbeam-channel 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", + "fs 0.0.1", + "futures 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)", + "futures 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)", + "futures-locks 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", + "graph 0.0.1", + "hashing 0.0.1", + "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", + "logging 0.0.1", + "notify 5.0.0-pre.1 (git+https://github.com/notify-rs/notify?rev=fba00891d9105e2f581c69fbe415a58cb7966fdd)", + "parking_lot 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", + "task_executor 0.0.1", + "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "testutil 0.0.1", + "tokio 0.2.13 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "web-sys" version = "0.3.36" diff --git a/src/rust/engine/Cargo.toml b/src/rust/engine/Cargo.toml index 4b32107f994..6df346f3dc4 100644 --- a/src/rust/engine/Cargo.toml +++ b/src/rust/engine/Cargo.toml @@ -42,6 +42,7 @@ members = [ "testutil/local_cas", "testutil/local_execution_server", "ui", + "watch", "workunit_store" ] @@ -77,6 +78,7 @@ default-members = [ "testutil/local_cas", "testutil/local_execution_server", "ui", + "watch", "workunit_store" ] @@ -86,12 +88,10 @@ async-trait = "0.1" boxfuture = { path = "boxfuture" } bytes = "0.4.5" concrete_time = { path = "concrete_time" } -crossbeam-channel = "0.3" fnv = "1.0.5" fs = { path = "fs" } futures01 = { package = "futures", version = "0.1" } futures = { version = "0.3", features = ["compat"] } -futures-locks = "0.3.0" graph = { path = "graph" } hashing = { path = "hashing" } indexmap = "1.0.2" @@ -101,11 +101,6 @@ log = "0.4" logging = { path = "logging" } num_cpus = "1" num_enum = "0.4" -# notify is currently an experimental API, we are pinning to https://docs.rs/notify/5.0.0-pre.1/notify/ -# because the latest prerelease at time of writing has removed the debounced watcher which we would like to use. -# The author suggests they will add the debounced watcher back into the stable 5.0.0 release. When that happens -# we can move to it. -notify = { git = "https://github.com/notify-rs/notify", rev = "fba00891d9105e2f581c69fbe415a58cb7966fdd" } parking_lot = "0.6" process_execution = { path = "process_execution" } rand = "0.6" @@ -121,6 +116,7 @@ tokio = { version = "0.2", features = ["rt-threaded"] } ui = { path = "ui" } url = "2.1" uuid = { version = "0.7", features = ["v4"] } +watch = { path = "watch" } workunit_store = { path = "workunit_store" } [dev-dependencies] diff --git a/src/rust/engine/graph/src/lib.rs b/src/rust/engine/graph/src/lib.rs index f2c4e6eea55..9103c1f4c90 100644 --- a/src/rust/engine/graph/src/lib.rs +++ b/src/rust/engine/graph/src/lib.rs @@ -1055,43 +1055,6 @@ impl Graph { } } -// This module provides a trait which contains functions that -// should only be used in tests. A user must explicitly import the trait -// to use the extra test functions, and they should only be imported into -// test modules. -pub mod test_support { - use super::{EntryId, EntryState, Graph, Node}; - pub trait TestGraph { - fn set_fixture_entry_state_for_id(&self, id: EntryId, state: EntryState); - fn add_fixture_entry(&self, node: N) -> EntryId; - fn entry_state(&self, id: EntryId) -> &str; - } - impl TestGraph for Graph { - fn set_fixture_entry_state_for_id(&self, id: EntryId, state: EntryState) { - let mut inner = self.inner.lock(); - let entry = inner.entry_for_id_mut(id).unwrap(); - let mut entry_state = entry.state.lock(); - *entry_state = state; - } - - fn add_fixture_entry(&self, node: N) -> EntryId { - let mut inner = self.inner.lock(); - inner.ensure_entry(node) - } - - fn entry_state(&self, id: EntryId) -> &str { - let mut inner = self.inner.lock(); - let entry = inner.entry_for_id_mut(id).unwrap(); - let entry_state = entry.state.lock(); - match *entry_state { - EntryState::Completed { .. } => "completed", - EntryState::Running { .. } => "running", - EntryState::NotStarted { .. } => "not started", - } - } - } -} - /// /// Represents the state of a particular walk through a Graph. Implements Iterator and has the same /// lifetime as the Graph itself. diff --git a/src/rust/engine/src/context.rs b/src/rust/engine/src/context.rs index c198f7c120a..173191c1e9e 100644 --- a/src/rust/engine/src/context.rs +++ b/src/rust/engine/src/context.rs @@ -2,8 +2,9 @@ // Licensed under the Apache License, Version 2.0 (see LICENSE). use std; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use std::convert::{Into, TryInto}; +use std::ops::Deref; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; @@ -18,11 +19,12 @@ use crate::nodes::{NodeKey, WrappedNode}; use crate::scheduler::Session; use crate::tasks::{Rule, Tasks}; use crate::types::Types; -use crate::watch::InvalidationWatcher; + use boxfuture::{BoxFuture, Boxable}; use core::clone::Clone; use fs::{safe_create_dir_all_ioerror, GitignoreStyleExcludes, PosixFS}; -use graph::{EntryId, Graph, NodeContext}; +use graph::{EntryId, Graph, InvalidationResult, NodeContext}; +use log::info; use process_execution::{ self, speculate::SpeculatingCommandRunner, BoundedCommandRunner, Platform, ProcessMetadata, }; @@ -32,6 +34,7 @@ use rule_graph::RuleGraph; use sharded_lmdb::ShardedLmdb; use store::Store; use tokio::runtime::{Builder, Runtime}; +use watch::{Invalidatable, InvalidationWatcher}; const GIGABYTES: usize = 1024 * 1024 * 1024; @@ -44,7 +47,7 @@ const GIGABYTES: usize = 1024 * 1024 * 1024; /// https://github.com/tokio-rs/tokio/issues/369 is resolved. /// pub struct Core { - pub graph: Arc>, + pub graph: Arc, pub tasks: Tasks, pub rule_graph: RuleGraph, pub types: Types, @@ -238,7 +241,7 @@ impl Core { process_execution_metadata, )); } - let graph = Arc::new(Graph::new()); + let graph = Arc::new(InvalidatableGraph(Graph::new())); let http_client = reqwest::Client::new(); let rule_graph = RuleGraph::new(tasks.as_map(), root_subject_types); @@ -295,6 +298,33 @@ impl Core { } } +pub struct InvalidatableGraph(Graph); + +impl Invalidatable for InvalidatableGraph { + fn invalidate(&self, paths: &HashSet, caller: &str) -> usize { + let InvalidationResult { cleared, dirtied } = self.invalidate_from_roots(move |node| { + if let Some(fs_subject) = node.fs_subject() { + paths.contains(fs_subject) + } else { + false + } + }); + info!( + "{} invalidation: cleared {} and dirtied {} nodes for: {:?}", + caller, cleared, dirtied, paths + ); + cleared + dirtied + } +} + +impl Deref for InvalidatableGraph { + type Target = Graph; + + fn deref(&self) -> &Graph { + &self.0 + } +} + #[derive(Clone)] pub struct Context { entry_id: Option, diff --git a/src/rust/engine/src/lib.rs b/src/rust/engine/src/lib.rs index 55e5ce62e09..5aafd98b63c 100644 --- a/src/rust/engine/src/lib.rs +++ b/src/rust/engine/src/lib.rs @@ -40,7 +40,6 @@ mod scheduler; mod selectors; mod tasks; mod types; -mod watch; pub use crate::context::Core; pub use crate::core::{Function, Key, Params, TypeId, Value}; @@ -51,6 +50,3 @@ pub use crate::scheduler::{ }; pub use crate::tasks::{Rule, Tasks}; pub use crate::types::Types; - -#[cfg(test)] -mod watch_tests; diff --git a/src/rust/engine/src/scheduler.rs b/src/rust/engine/src/scheduler.rs index 0135843bc49..2566dbd3e4e 100644 --- a/src/rust/engine/src/scheduler.rs +++ b/src/rust/engine/src/scheduler.rs @@ -14,7 +14,7 @@ use futures01::future::{self, Future}; use crate::context::{Context, Core}; use crate::core::{Failure, Params, TypeId, Value}; use crate::nodes::{NodeKey, Select, Tracer, Visualizer}; -use crate::watch::InvalidationWatcher; + use graph::{Graph, InvalidationResult}; use hashing; use indexmap::IndexMap; @@ -22,6 +22,7 @@ use log::{debug, info, warn}; use logging::logger::LOGGER; use parking_lot::Mutex; use ui::{EngineDisplay, KeyboardCommand}; +use watch::Invalidatable; use workunit_store::WorkUnitStore; pub enum ExecutionTermination { @@ -228,7 +229,7 @@ impl Scheduler { /// Invalidate the invalidation roots represented by the given Paths. /// pub fn invalidate(&self, paths: &HashSet) -> usize { - InvalidationWatcher::invalidate(&self.core.graph, paths, "watchman") + self.core.graph.invalidate(paths, "watchman") } /// diff --git a/src/rust/engine/testutil/src/lib.rs b/src/rust/engine/testutil/src/lib.rs index 342a5b0c24a..5a0d712d9f2 100644 --- a/src/rust/engine/testutil/src/lib.rs +++ b/src/rust/engine/testutil/src/lib.rs @@ -54,7 +54,7 @@ pub fn make_file(path: &Path, contents: &[u8], mode: u32) { file.set_permissions(permissions).unwrap(); } -pub fn append_to_exisiting_file(path: &Path, contents: &[u8]) { +pub fn append_to_existing_file(path: &Path, contents: &[u8]) { let mut file = std::fs::OpenOptions::new().write(true).open(&path).unwrap(); file.write_all(contents).unwrap(); } diff --git a/src/rust/engine/watch/Cargo.toml b/src/rust/engine/watch/Cargo.toml new file mode 100644 index 00000000000..0b4b43b5799 --- /dev/null +++ b/src/rust/engine/watch/Cargo.toml @@ -0,0 +1,29 @@ +[package] +version = "0.0.1" +edition = "2018" +name = "watch" +authors = [ "Pants Build " ] +publish = false + +[dependencies] +crossbeam-channel = "0.3" +fs = { path = "../fs" } +futures = { version = "0.3", features = ["compat"] } +futures-locks = "0.3.0" +futures01 = { package = "futures", version = "0.1" } +graph = { path = "../graph" } +log = "0.4" +logging = { path = "../logging" } +# notify is currently an experimental API, we are pinning to https://docs.rs/notify/5.0.0-pre.1/notify/ +# because the latest prerelease at time of writing has removed the debounced watcher which we would like to use. +# The author suggests they will add the debounced watcher back into the stable 5.0.0 release. When that happens +# we can move to it. +notify = { git = "https://github.com/notify-rs/notify", rev = "fba00891d9105e2f581c69fbe415a58cb7966fdd" } +task_executor = { path = "../task_executor" } + +[dev-dependencies] +hashing = { path = "../hashing" } +parking_lot = "0.6" +tempfile = "3" +testutil = { path = "../testutil" } +tokio = { version = "0.2", features = ["rt-core", "macros"] } diff --git a/src/rust/engine/src/watch.rs b/src/rust/engine/watch/src/lib.rs similarity index 83% rename from src/rust/engine/src/watch.rs rename to src/rust/engine/watch/src/lib.rs index 7752ae07ec0..48e65110adf 100644 --- a/src/rust/engine/src/watch.rs +++ b/src/rust/engine/watch/src/lib.rs @@ -1,6 +1,33 @@ -// Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). +// Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). // Licensed under the Apache License, Version 2.0 (see LICENSE). +#![deny(warnings)] +// Enable all clippy lints except for many of the pedantic ones. It's a shame this needs to be copied and pasted across crates, but there doesn't appear to be a way to include inner attributes from a common source. +#![deny( + clippy::all, + clippy::default_trait_access, + clippy::expl_impl_clone_on_copy, + clippy::if_not_else, + clippy::needless_continue, + clippy::unseparated_literal_suffix, + clippy::used_underscore_binding +)] +// It is often more clear to show that nothing is being moved. +#![allow(clippy::match_ref_pats)] +// Subjective style. +#![allow( + clippy::len_without_is_empty, + clippy::redundant_field_names, + clippy::too_many_arguments +)] +// Default isn't as big a deal as people seem to think it is. +#![allow(clippy::new_without_default, clippy::new_ret_no_self)] +// Arc can be more clear than needing to grok Orderings: +#![allow(clippy::mutex_atomic)] + +#[cfg(test)] +mod tests; + use std::collections::HashSet; use std::path::PathBuf; use std::sync::{Arc, Weak}; @@ -10,16 +37,13 @@ use std::time::Duration; use crossbeam_channel::{self, Receiver, RecvTimeoutError, TryRecvError}; use futures::compat::Future01CompatExt; use futures_locks::Mutex; -use log::{debug, error, info, warn}; +use log::{debug, error, warn}; use notify::{RecommendedWatcher, RecursiveMode, Watcher}; use task_executor::Executor; use fs::GitignoreStyleExcludes; -use graph::{Graph, InvalidationResult}; use logging; -use crate::nodes::NodeKey; - /// /// An InvalidationWatcher maintains a Thread that receives events from a notify Watcher. /// @@ -29,14 +53,6 @@ use crate::nodes::NodeKey; /// /// TODO: Need the above polling /// -/// TODO: To simplify testing the InvalidationWatcher we could create a trait which -/// has an `invalidate_from_roots` method and impl it on the Graph. Then we could make the InvalidationWatcher -/// take an argument that implements the trait. -/// Then we wouldn't have to mock out a Graph object in watch_tests.rs. This will probably -/// only be possible when we remove watchman invalidation, when the one code path for invaldation will be -/// the notify background thread. -/// Potential impl here: https://github.com/pantsbuild/pants/pull/9318#discussion_r396005978 -/// pub struct InvalidationWatcher { watcher: Arc>, executor: Executor, @@ -45,8 +61,8 @@ pub struct InvalidationWatcher { } impl InvalidationWatcher { - pub fn new( - graph: Weak>, + pub fn new( + invalidatable: Weak, executor: Executor, build_root: PathBuf, ignorer: Arc, @@ -81,7 +97,7 @@ impl InvalidationWatcher { } InvalidationWatcher::start_background_thread( - graph, + invalidatable, ignorer, canonical_build_root, thread_liveness_sender, @@ -97,8 +113,8 @@ impl InvalidationWatcher { } // Public for testing purposes. - pub(crate) fn start_background_thread( - graph: Weak>, + pub(crate) fn start_background_thread( + invalidatable: Weak, ignorer: Arc, canonical_build_root: PathBuf, liveness_sender: crossbeam_channel::Sender<()>, @@ -108,10 +124,10 @@ impl InvalidationWatcher { logging::set_thread_destination(logging::Destination::Pantsd); loop { let event_res = watch_receiver.recv_timeout(Duration::from_millis(10)); - let graph = if let Some(g) = graph.upgrade() { + let invalidatable = if let Some(g) = invalidatable.upgrade() { g } else { - // The Graph has been dropped: we're done. + // The Invalidatable has been dropped: we're done. break; }; match event_res { @@ -156,7 +172,7 @@ impl InvalidationWatcher { // Only invalidate stuff if we have paths that weren't filtered out by gitignore. if !paths.is_empty() { debug!("notify invalidating {:?} because of {:?}", paths, ev.kind); - InvalidationWatcher::invalidate(&graph, &paths, "notify"); + invalidatable.invalidate(&paths, "notify"); }; } Ok(Err(err)) => { @@ -228,19 +244,8 @@ impl InvalidationWatcher { Err(TryRecvError::Empty) => true, } } +} - pub fn invalidate(graph: &Graph, paths: &HashSet, caller: &str) -> usize { - let InvalidationResult { cleared, dirtied } = graph.invalidate_from_roots(move |node| { - if let Some(fs_subject) = node.fs_subject() { - paths.contains(fs_subject) - } else { - false - } - }); - info!( - "{} invalidation: cleared {} and dirtied {} nodes for: {:?}", - caller, cleared, dirtied, paths - ); - cleared + dirtied - } +pub trait Invalidatable: Send + Sync + 'static { + fn invalidate(&self, paths: &HashSet, caller: &str) -> usize; } diff --git a/src/rust/engine/src/watch_tests.rs b/src/rust/engine/watch/src/tests.rs similarity index 52% rename from src/rust/engine/src/watch_tests.rs rename to src/rust/engine/watch/src/tests.rs index 14b4e938bb8..7655253a974 100644 --- a/src/rust/engine/src/watch_tests.rs +++ b/src/rust/engine/watch/src/tests.rs @@ -1,25 +1,18 @@ -use crate::nodes::{DigestFile, NodeKey, NodeResult}; -use crate::watch::InvalidationWatcher; -use crossbeam_channel; -use fs::{File, GitignoreStyleExcludes}; -use graph::entry::{EntryResult, EntryState, Generation, RunToken}; -use graph::{test_support::TestGraph, EntryId, Graph}; -use hashing::EMPTY_DIGEST; -use notify; +use crate::{Invalidatable, InvalidationWatcher}; + +use std::collections::HashSet; use std::fs::create_dir; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::Arc; use std::thread::sleep; use std::time::Duration; -use task_executor::Executor; -use testutil::{append_to_exisiting_file, make_file}; -fn init_logger() -> () { - match env_logger::try_init() { - Ok(()) => (), - Err(_) => (), - } -} +use crossbeam_channel; +use fs::GitignoreStyleExcludes; +use notify; +use parking_lot::Mutex; +use task_executor::Executor; +use testutil::{append_to_existing_file, make_file}; fn setup_fs() -> (tempfile::TempDir, PathBuf) { // setup a build_root with a file in it to watch. @@ -32,35 +25,16 @@ fn setup_fs() -> (tempfile::TempDir, PathBuf) { (tempdir, file_path) } -fn setup_graph(fs_subject: PathBuf) -> (Arc>, EntryId) { - let node = NodeKey::DigestFile(DigestFile(File { - path: fs_subject, - is_executable: false, - })); - let graph = Arc::new(Graph::new()); - let entry_id = graph.add_fixture_entry(node); - let completed_state = EntryState::Completed { - run_token: RunToken::initial(), - generation: Generation::initial(), - result: EntryResult::Clean(Ok(NodeResult::Digest(EMPTY_DIGEST))), - dep_generations: vec![], - }; - graph.set_fixture_entry_state_for_id(entry_id, completed_state); - // Assert the nodes initial state is completed - assert!(graph.entry_state(entry_id) == "completed"); - (graph, entry_id) -} - fn setup_watch( ignorer: Arc, - graph: Arc>, + invalidatable: Arc, build_root: PathBuf, file_path: PathBuf, ) -> InvalidationWatcher { let mut rt = tokio::runtime::Runtime::new().unwrap(); let executor = Executor::new(rt.handle().clone()); let watcher = InvalidationWatcher::new( - Arc::downgrade(&graph), + Arc::downgrade(&invalidatable), executor, build_root, ignorer, @@ -73,35 +47,34 @@ fn setup_watch( #[test] fn receive_watch_event_on_file_change() { - // set up a node in the graph to check that it gets cleared by the invalidation watcher. // Instantiate a watcher and watch the file in question. - init_logger(); let (tempdir, file_path) = setup_fs(); let build_root = tempdir.path().to_path_buf(); - let (graph, entry_id) = setup_graph( - file_path - .clone() - .strip_prefix(build_root.clone()) - .unwrap() - .to_path_buf(), - ); + let file_path_rel = file_path + .clone() + .strip_prefix(build_root.clone()) + .unwrap() + .to_path_buf(); + let invalidatable = Arc::new(TestInvalidatable::default()); let ignorer = GitignoreStyleExcludes::create(&[]).unwrap(); let _watcher = setup_watch( ignorer, - graph.clone(), + invalidatable.clone(), build_root.clone(), file_path.clone(), ); + // Update the content of the file being watched. let new_content = "stnetnoc".as_bytes().to_vec(); - append_to_exisiting_file(&file_path, &new_content); - // Wait for watcher background thread to trigger a node invalidation, - // by checking the entry state for the node. It will be reset to EntryState::NotStarted - // when Graph::invalidate_from_roots calls clear on the node. + append_to_existing_file(&file_path, &new_content); + + // Wait for the watcher background thread to trigger a node invalidation, which will cause the + // new salt to be used. for _ in 0..10 { sleep(Duration::from_millis(100)); - if graph.entry_state(entry_id) == "not started" { + if invalidatable.was_invalidated(&file_path_rel) { + // Observed invalidation. return; } } @@ -114,34 +87,32 @@ fn receive_watch_event_on_file_change() { #[test] fn ignore_file_events_matching_patterns_in_pants_ignore() { - init_logger(); let (tempdir, file_path) = setup_fs(); let build_root = tempdir.path().to_path_buf(); - let (graph, entry_id) = setup_graph( - file_path - .clone() - .strip_prefix(build_root.clone()) - .unwrap() - .to_path_buf(), - ); + let file_path_rel = file_path + .clone() + .strip_prefix(build_root.clone()) + .unwrap() + .to_path_buf(); + let invalidatable = Arc::new(TestInvalidatable::default()); let ignorer = GitignoreStyleExcludes::create(&["/foo".to_string()]).unwrap(); let _watcher = setup_watch( ignorer, - graph.clone(), + invalidatable.clone(), build_root.clone(), file_path.clone(), ); + // Update the content of the file being watched. let new_content = "stnetnoc".as_bytes().to_vec(); - append_to_exisiting_file(&file_path, &new_content); - // Wait for watcher background thread to trigger a node invalidation, - // by checking the entry state for the node. It will be reset to EntryState::NotStarted - // when Graph::invalidate_from_roots calls clear on the node. + append_to_existing_file(&file_path, &new_content); + + // Wait for the watcher background thread to trigger a node invalidation, which would cause the + // new salt to be used. for _ in 0..10 { sleep(Duration::from_millis(100)); - // If the state changed the node was invalidated so fail. - if graph.entry_state(entry_id) != "completed" { + if invalidatable.was_invalidated(&file_path_rel) { assert!(false, "Node was invalidated even though it was ignored") } } @@ -149,22 +120,15 @@ fn ignore_file_events_matching_patterns_in_pants_ignore() { #[test] fn test_liveness() { - init_logger(); - let (tempdir, file_path) = setup_fs(); + let (tempdir, _) = setup_fs(); let build_root = tempdir.path().to_path_buf(); - let (graph, _entry_id) = setup_graph( - file_path - .clone() - .strip_prefix(build_root.clone()) - .unwrap() - .to_path_buf(), - ); + let invalidatable = Arc::new(TestInvalidatable::default()); let ignorer = GitignoreStyleExcludes::create(&[]).unwrap(); let (liveness_sender, liveness_receiver) = crossbeam_channel::unbounded(); let (event_sender, event_receiver) = crossbeam_channel::unbounded(); InvalidationWatcher::start_background_thread( - Arc::downgrade(&graph), + Arc::downgrade(&invalidatable), ignorer, build_root, liveness_sender, @@ -179,3 +143,24 @@ fn test_liveness() { .recv_timeout(Duration::from_millis(100)) .is_ok()); } + +#[derive(Default)] +struct TestInvalidatable { + pub calls: Mutex>>, +} + +impl TestInvalidatable { + fn was_invalidated(&self, path: &Path) -> bool { + let calls = self.calls.lock(); + calls.iter().any(|call| call.contains(path)) + } +} + +impl Invalidatable for TestInvalidatable { + fn invalidate(&self, paths: &HashSet, _caller: &str) -> usize { + let invalidated = paths.len(); + let mut calls = self.calls.lock(); + calls.push(paths.clone()); + invalidated + } +}