diff options
author | Florian Diebold <[email protected]> | 2021-02-12 14:58:29 +0000 |
---|---|---|
committer | Florian Diebold <[email protected]> | 2021-02-12 15:31:16 +0000 |
commit | a7387cae2ca9d5e114246e6fada98bfe7808e1d0 (patch) | |
tree | db0fe8d939afaa10929debf317be670f79909c9f | |
parent | dee5aba43a1b45131bf31268431fa71923f2ef2a (diff) |
Fix slow tests sometimes failing
In some situations we reloaded the workspace in the tests after having reported
to be ready. There's two fixes here:
1. Add a version to the VFS config and include that version in progress reports,
so that we don't think we're done prematurely;
2. Delay status transitions until after changes are applied. Otherwise the last
change during loading can potentially trigger a workspace reload, if it contains
interesting changes.
-rw-r--r-- | crates/rust-analyzer/src/cli/load_cargo.rs | 8 | ||||
-rw-r--r-- | crates/rust-analyzer/src/global_state.rs | 2 | ||||
-rw-r--r-- | crates/rust-analyzer/src/main_loop.rs | 20 | ||||
-rw-r--r-- | crates/rust-analyzer/src/reload.rs | 17 | ||||
-rw-r--r-- | crates/vfs-notify/src/lib.rs | 10 | ||||
-rw-r--r-- | crates/vfs/src/loader.rs | 8 |
6 files changed, 51 insertions, 14 deletions
diff --git a/crates/rust-analyzer/src/cli/load_cargo.rs b/crates/rust-analyzer/src/cli/load_cargo.rs index cc63c6cc2..cf0cd81de 100644 --- a/crates/rust-analyzer/src/cli/load_cargo.rs +++ b/crates/rust-analyzer/src/cli/load_cargo.rs | |||
@@ -59,7 +59,11 @@ pub fn load_cargo(root: &Path, config: &LoadCargoConfig) -> Result<(AnalysisHost | |||
59 | ); | 59 | ); |
60 | 60 | ||
61 | let project_folders = ProjectFolders::new(&[ws], &[], build_data.as_ref()); | 61 | let project_folders = ProjectFolders::new(&[ws], &[], build_data.as_ref()); |
62 | loader.set_config(vfs::loader::Config { load: project_folders.load, watch: vec![] }); | 62 | loader.set_config(vfs::loader::Config { |
63 | load: project_folders.load, | ||
64 | watch: vec![], | ||
65 | version: 0, | ||
66 | }); | ||
63 | 67 | ||
64 | log::debug!("crate graph: {:?}", crate_graph); | 68 | log::debug!("crate graph: {:?}", crate_graph); |
65 | let host = load(crate_graph, project_folders.source_root_config, &mut vfs, &receiver); | 69 | let host = load(crate_graph, project_folders.source_root_config, &mut vfs, &receiver); |
@@ -79,7 +83,7 @@ fn load( | |||
79 | // wait until Vfs has loaded all roots | 83 | // wait until Vfs has loaded all roots |
80 | for task in receiver { | 84 | for task in receiver { |
81 | match task { | 85 | match task { |
82 | vfs::loader::Message::Progress { n_done, n_total } => { | 86 | vfs::loader::Message::Progress { n_done, n_total, config_version: _ } => { |
83 | if n_done == n_total { | 87 | if n_done == n_total { |
84 | break; | 88 | break; |
85 | } | 89 | } |
diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 6374a9f3c..c3bc8791d 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs | |||
@@ -67,6 +67,7 @@ pub(crate) struct GlobalState { | |||
67 | req_queue: ReqQueue, | 67 | req_queue: ReqQueue, |
68 | pub(crate) task_pool: Handle<TaskPool<Task>, Receiver<Task>>, | 68 | pub(crate) task_pool: Handle<TaskPool<Task>, Receiver<Task>>, |
69 | pub(crate) loader: Handle<Box<dyn vfs::loader::Handle>, Receiver<vfs::loader::Message>>, | 69 | pub(crate) loader: Handle<Box<dyn vfs::loader::Handle>, Receiver<vfs::loader::Message>>, |
70 | pub(crate) vfs_config_version: u32, | ||
70 | pub(crate) flycheck: Vec<FlycheckHandle>, | 71 | pub(crate) flycheck: Vec<FlycheckHandle>, |
71 | pub(crate) flycheck_sender: Sender<flycheck::Message>, | 72 | pub(crate) flycheck_sender: Sender<flycheck::Message>, |
72 | pub(crate) flycheck_receiver: Receiver<flycheck::Message>, | 73 | pub(crate) flycheck_receiver: Receiver<flycheck::Message>, |
@@ -120,6 +121,7 @@ impl GlobalState { | |||
120 | GlobalState { | 121 | GlobalState { |
121 | sender, | 122 | sender, |
122 | req_queue: ReqQueue::default(), | 123 | req_queue: ReqQueue::default(), |
124 | vfs_config_version: 0, | ||
123 | task_pool, | 125 | task_pool, |
124 | loader, | 126 | loader, |
125 | flycheck: Vec::new(), | 127 | flycheck: Vec::new(), |
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index bdffff5ae..2829d5970 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs | |||
@@ -5,6 +5,7 @@ use std::{ | |||
5 | time::{Duration, Instant}, | 5 | time::{Duration, Instant}, |
6 | }; | 6 | }; |
7 | 7 | ||
8 | use always_assert::always; | ||
8 | use crossbeam_channel::{select, Receiver}; | 9 | use crossbeam_channel::{select, Receiver}; |
9 | use ide::PrimeCachesProgress; | 10 | use ide::PrimeCachesProgress; |
10 | use ide::{Canceled, FileId}; | 11 | use ide::{Canceled, FileId}; |
@@ -186,7 +187,7 @@ impl GlobalState { | |||
186 | log::info!("task queue len: {}", task_queue_len); | 187 | log::info!("task queue len: {}", task_queue_len); |
187 | } | 188 | } |
188 | 189 | ||
189 | let prev_status = self.status; | 190 | let mut new_status = self.status; |
190 | match event { | 191 | match event { |
191 | Event::Lsp(msg) => match msg { | 192 | Event::Lsp(msg) => match msg { |
192 | lsp_server::Message::Request(req) => self.on_request(loop_start, req)?, | 193 | lsp_server::Message::Request(req) => self.on_request(loop_start, req)?, |
@@ -298,22 +299,23 @@ impl GlobalState { | |||
298 | } | 299 | } |
299 | } | 300 | } |
300 | } | 301 | } |
301 | vfs::loader::Message::Progress { n_total, n_done } => { | 302 | vfs::loader::Message::Progress { n_total, n_done, config_version } => { |
303 | always!(config_version <= self.vfs_config_version); | ||
302 | if n_total == 0 { | 304 | if n_total == 0 { |
303 | self.transition(Status::Invalid); | 305 | new_status = Status::Invalid; |
304 | } else { | 306 | } else { |
305 | let state = if n_done == 0 { | 307 | let state = if n_done == 0 { |
306 | self.transition(Status::Loading); | 308 | new_status = Status::Loading; |
307 | Progress::Begin | 309 | Progress::Begin |
308 | } else if n_done < n_total { | 310 | } else if n_done < n_total { |
309 | Progress::Report | 311 | Progress::Report |
310 | } else { | 312 | } else { |
311 | assert_eq!(n_done, n_total); | 313 | assert_eq!(n_done, n_total); |
312 | let status = Status::Ready { | 314 | new_status = Status::Ready { |
313 | partial: self.config.load_out_dirs_from_check() | 315 | partial: self.config.load_out_dirs_from_check() |
314 | && self.workspace_build_data.is_none(), | 316 | && self.workspace_build_data.is_none() |
317 | || config_version < self.vfs_config_version, | ||
315 | }; | 318 | }; |
316 | self.transition(status); | ||
317 | Progress::End | 319 | Progress::End |
318 | }; | 320 | }; |
319 | self.report_progress( | 321 | self.report_progress( |
@@ -398,6 +400,10 @@ impl GlobalState { | |||
398 | } | 400 | } |
399 | 401 | ||
400 | let state_changed = self.process_changes(); | 402 | let state_changed = self.process_changes(); |
403 | let prev_status = self.status; | ||
404 | if prev_status != new_status { | ||
405 | self.transition(new_status); | ||
406 | } | ||
401 | let is_ready = matches!(self.status, Status::Ready { .. }); | 407 | let is_ready = matches!(self.status, Status::Ready { .. }); |
402 | if prev_status == Status::Loading && is_ready { | 408 | if prev_status == Status::Loading && is_ready { |
403 | for flycheck in &self.flycheck { | 409 | for flycheck in &self.flycheck { |
diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index c4f1d098b..c07efa330 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs | |||
@@ -50,6 +50,16 @@ impl GlobalState { | |||
50 | Status::Loading | Status::NeedsReload => return, | 50 | Status::Loading | Status::NeedsReload => return, |
51 | Status::Ready { .. } | Status::Invalid => (), | 51 | Status::Ready { .. } | Status::Invalid => (), |
52 | } | 52 | } |
53 | log::info!( | ||
54 | "Reloading workspace because of the following changes: {}", | ||
55 | itertools::join( | ||
56 | changes | ||
57 | .iter() | ||
58 | .filter(|(path, kind)| is_interesting(path, *kind)) | ||
59 | .map(|(path, kind)| format!("{}/{:?}", path.display(), kind)), | ||
60 | ", " | ||
61 | ) | ||
62 | ); | ||
53 | if self.config.cargo_autoreload() { | 63 | if self.config.cargo_autoreload() { |
54 | self.fetch_workspaces_request(); | 64 | self.fetch_workspaces_request(); |
55 | } else { | 65 | } else { |
@@ -290,7 +300,12 @@ impl GlobalState { | |||
290 | FilesWatcher::Client => vec![], | 300 | FilesWatcher::Client => vec![], |
291 | FilesWatcher::Notify => project_folders.watch, | 301 | FilesWatcher::Notify => project_folders.watch, |
292 | }; | 302 | }; |
293 | self.loader.handle.set_config(vfs::loader::Config { load: project_folders.load, watch }); | 303 | self.vfs_config_version += 1; |
304 | self.loader.handle.set_config(vfs::loader::Config { | ||
305 | load: project_folders.load, | ||
306 | watch, | ||
307 | version: self.vfs_config_version, | ||
308 | }); | ||
294 | 309 | ||
295 | // Create crate graph from all the workspaces | 310 | // Create crate graph from all the workspaces |
296 | let crate_graph = { | 311 | let crate_graph = { |
diff --git a/crates/vfs-notify/src/lib.rs b/crates/vfs-notify/src/lib.rs index c605bcf3c..f7ae0577d 100644 --- a/crates/vfs-notify/src/lib.rs +++ b/crates/vfs-notify/src/lib.rs | |||
@@ -86,8 +86,10 @@ impl NotifyActor { | |||
86 | self.watcher = watcher.map(|it| (it, watcher_receiver)); | 86 | self.watcher = watcher.map(|it| (it, watcher_receiver)); |
87 | } | 87 | } |
88 | 88 | ||
89 | let config_version = config.version; | ||
90 | |||
89 | let n_total = config.load.len(); | 91 | let n_total = config.load.len(); |
90 | self.send(loader::Message::Progress { n_total, n_done: 0 }); | 92 | self.send(loader::Message::Progress { n_total, n_done: 0, config_version }); |
91 | 93 | ||
92 | self.watched_entries.clear(); | 94 | self.watched_entries.clear(); |
93 | 95 | ||
@@ -98,7 +100,11 @@ impl NotifyActor { | |||
98 | } | 100 | } |
99 | let files = self.load_entry(entry, watch); | 101 | let files = self.load_entry(entry, watch); |
100 | self.send(loader::Message::Loaded { files }); | 102 | self.send(loader::Message::Loaded { files }); |
101 | self.send(loader::Message::Progress { n_total, n_done: i + 1 }); | 103 | self.send(loader::Message::Progress { |
104 | n_total, | ||
105 | n_done: i + 1, | ||
106 | config_version, | ||
107 | }); | ||
102 | } | 108 | } |
103 | } | 109 | } |
104 | Message::Invalidate(path) => { | 110 | Message::Invalidate(path) => { |
diff --git a/crates/vfs/src/loader.rs b/crates/vfs/src/loader.rs index d3bdae562..473b29fcb 100644 --- a/crates/vfs/src/loader.rs +++ b/crates/vfs/src/loader.rs | |||
@@ -32,6 +32,9 @@ pub struct Directories { | |||
32 | /// [`Handle`]'s configuration. | 32 | /// [`Handle`]'s configuration. |
33 | #[derive(Debug)] | 33 | #[derive(Debug)] |
34 | pub struct Config { | 34 | pub struct Config { |
35 | /// Version number to associate progress updates to the right config | ||
36 | /// version. | ||
37 | pub version: u32, | ||
35 | /// Set of initially loaded files. | 38 | /// Set of initially loaded files. |
36 | pub load: Vec<Entry>, | 39 | pub load: Vec<Entry>, |
37 | /// Index of watched entries in `load`. | 40 | /// Index of watched entries in `load`. |
@@ -45,7 +48,7 @@ pub enum Message { | |||
45 | /// Indicate a gradual progress. | 48 | /// Indicate a gradual progress. |
46 | /// | 49 | /// |
47 | /// This is supposed to be the number of loaded files. | 50 | /// This is supposed to be the number of loaded files. |
48 | Progress { n_total: usize, n_done: usize }, | 51 | Progress { n_total: usize, n_done: usize, config_version: u32 }, |
49 | /// The handle loaded the following files' content. | 52 | /// The handle loaded the following files' content. |
50 | Loaded { files: Vec<(AbsPathBuf, Option<Vec<u8>>)> }, | 53 | Loaded { files: Vec<(AbsPathBuf, Option<Vec<u8>>)> }, |
51 | } | 54 | } |
@@ -196,10 +199,11 @@ impl fmt::Debug for Message { | |||
196 | Message::Loaded { files } => { | 199 | Message::Loaded { files } => { |
197 | f.debug_struct("Loaded").field("n_files", &files.len()).finish() | 200 | f.debug_struct("Loaded").field("n_files", &files.len()).finish() |
198 | } | 201 | } |
199 | Message::Progress { n_total, n_done } => f | 202 | Message::Progress { n_total, n_done, config_version } => f |
200 | .debug_struct("Progress") | 203 | .debug_struct("Progress") |
201 | .field("n_total", n_total) | 204 | .field("n_total", n_total) |
202 | .field("n_done", n_done) | 205 | .field("n_done", n_done) |
206 | .field("config_version", config_version) | ||
203 | .finish(), | 207 | .finish(), |
204 | } | 208 | } |
205 | } | 209 | } |