Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

test: add buffer diagnostics editor tests

Browse source 
Update the `diagnostics::diagnostics_tests` module with three new tests
for the `BufferDiagnosticsEditor` view:

- `test_buffer_diagnostics` - Asserts that the `BufferDiagnosticsEditor`
displays diagnostics only for the provided project path, ignoring other
paths with diagnostics.
- `test_buffer_diagnostics_without_warnings` - Asserts that if
`include_warnings` is `false`, the `BufferDiagnosticsEditor` displays
only errors in the editor, without showing warnings.
- `test_buffer_diagnostics_multiple_servers` - Asserts that if the
project path has multiple diagnostics for different language servers,
all diagnostics are shown and included in the summary.

Additionally, the
`project::lsp_store::LspStore.diagnostic_summary_for_path` method has
been updated to return the total count of warnings and errors across all
language servers for the provided path, rather than just the first
language server. This change aligns it with the behavior when
diagnostics are displayed, as all language servers are considered.

This commit also updates the `BufferDiagnosticsEditor::deploy` function
in order to fetch the buffer directly from the active editor and provide
it to `BufferDiagnosticsEditor::new`. This avoids having to spawn the
background task that would open the buffer and update the
`BufferDiagnosticsEditor.buffer` field.
This commit is contained in:
dinocosta 2025-08-14 15:53:40 +01:00
parent 600358609b
commit da6546d51b
4 changed files with 503 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

73
crates/diagnostics/src/buffer_diagnostics.rs
View file

@ -26,7 +26,7 @@ use std::{
};
use text::{Anchor, BufferSnapshot, OffsetRangeExt};
use ui::{Button, ButtonStyle, Icon, IconName, Label, Tooltip, h_flex, prelude::*};
use util::{ResultExt, paths::PathExt};
use util::paths::PathExt;
use workspace::{
ItemHandle, ItemNavHistory, ToolbarItemLocation, Workspace,
item::{BreadcrumbText, Item, ItemEvent, TabContentParams},
@ -79,9 +79,10 @@ pub(crate) struct BufferDiagnosticsEditor {
impl BufferDiagnosticsEditor {
/// Creates new instance of the `BufferDiagnosticsEditor` which can then be
/// displayed by adding it to a pane.
fn new(
pub fn new(
project_path: ProjectPath,
project_handle: Entity<Project>,
buffer: Option<Entity<Buffer>>,
include_warnings: bool,
window: &mut Window,
cx: &mut Context<Self>,
@ -187,14 +188,14 @@ impl BufferDiagnosticsEditor {
let diagnostics = vec![];
let update_excerpts_task = None;
let buffer_diagnostics_editor = Self {
let mut buffer_diagnostics_editor = Self {
project: project_handle,
focus_handle,
editor,
diagnostics,
blocks: Default::default(),
multibuffer,
buffer: None,
buffer,
project_path,
summary,
include_warnings,
@ -202,23 +203,7 @@ impl BufferDiagnosticsEditor {
_subscription: project_event_subscription,
};
let project = buffer_diagnostics_editor.project.clone();
let project_path = buffer_diagnostics_editor.project_path.clone();
cx.spawn_in(window, async move |editor, cx| {
let buffer = project
.update(cx, |project, cx| project.open_buffer(project_path, cx))?
.await
.log_err();
editor.update_in(cx, |editor, window, cx| {
editor.buffer = buffer;
editor.update_all_diagnostics(window, cx);
cx.notify();
})
})
.detach();
buffer_diagnostics_editor.update_all_diagnostics(window, cx);
buffer_diagnostics_editor
}
@ -232,9 +217,8 @@ impl BufferDiagnosticsEditor {
// finding the project path for the buffer.
// If there's no active editor with a project path, avoiding deploying
// the buffer diagnostics view.
if let Some(project_path) = workspace
.active_item_as::<Editor>(cx)
.map_or(None, |editor| editor.project_path(cx))
if let Some(editor) = workspace.active_item_as::<Editor>(cx)
&& let Some(project_path) = editor.project_path(cx)
{
// Check if there's already a `BufferDiagnosticsEditor` tab for this
// same path, and if so, focus on that one instead of creating a new
@ -255,6 +239,7 @@ impl BufferDiagnosticsEditor {
Self::new(
project_path,
workspace.project().clone(),
editor.read(cx).buffer().read(cx).as_singleton(),
include_warnings,
window,
cx,
@ -512,6 +497,11 @@ impl BufferDiagnosticsEditor {
}
}
// Cloning the blocks before moving ownership so these can later
// be used to set the block contents for testing purposes.
#[cfg(test)]
let cloned_blocks = blocks.clone();
// Build new diagnostic blocks to be added to the editor's
// display map for the new diagnostics. Update the `blocks`
// property before finishing, to ensure the blocks are removed
@ -540,6 +530,30 @@ impl BufferDiagnosticsEditor {
})
});
// In order to be able to verify which diagnostic blocks are
// rendered in the editor, the `set_block_content_for_tests`
// function must be used, so that the
// `editor::test::editor_content_with_blocks` function can then
// be called to fetch these blocks.
#[cfg(test)]
{
for (block_id, block) in block_ids.iter().zip(cloned_blocks.iter()) {
let markdown = block.markdown.clone();
editor::test::set_block_content_for_tests(
&buffer_diagnostics_editor.editor,
*block_id,
cx,
move |cx| {
markdown::MarkdownElement::rendered_text(
markdown.clone(),
cx,
editor::hover_popover::diagnostics_markdown_style,
)
},
);
}
}
buffer_diagnostics_editor.blocks = block_ids;
cx.notify()
})
@ -610,6 +624,16 @@ impl BufferDiagnosticsEditor {
false => DiagnosticSeverity::Error,
}
}
#[cfg(test)]
pub fn editor(&self) -> &Entity<Editor> {
&self.editor
}
#[cfg(test)]
pub fn summary(&self) -> &DiagnosticSummary {
&self.summary
}
}
impl Focusable for BufferDiagnosticsEditor {
@ -674,6 +698,7 @@ impl Item for BufferDiagnosticsEditor {
BufferDiagnosticsEditor::new(
self.project_path.clone(),
self.project.clone(),
self.buffer.clone(),
self.include_warnings,
window,
cx,

434
crates/diagnostics/src/diagnostics_tests.rs
View file

@ -1566,6 +1566,440 @@ async fn go_to_diagnostic_with_severity(cx: &mut TestAppContext) {
cx.assert_editor_state(indoc! {"error ˇwarning info hint"});
}
#[gpui::test]
async fn test_buffer_diagnostics(cx: &mut TestAppContext) {
init_test(cx);
// We'll be creating two different files, both with diagnostics, so we can
// later verify that, since the `BufferDiagnosticsEditor` only shows
// diagnostics for the provided path, the diagnostics for the other file
// will not be shown, contrary to what happens with
// `ProjectDiagnosticsEditor`.
let fs = FakeFs::new(cx.executor());
fs.insert_tree(
path!("/test"),
json!({
"main.rs": "
fn main() {
let x = vec![];
let y = vec![];
a(x);
b(y);
c(y);
d(x);
}
"
.unindent(),
"other.rs": "
fn other() {
let unused = 42;
undefined_function();
}
"
.unindent(),
}),
)
.await;
let project = Project::test(fs.clone(), [path!("/test").as_ref()], cx).await;
let window = cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx));
let cx = &mut VisualTestContext::from_window(*window, cx);
let project_path = project::ProjectPath {
worktree_id: project.read_with(cx, |project, cx| {
project.worktrees(cx).next().unwrap().read(cx).id()
}),
path: Arc::from(Path::new("main.rs")),
};
let buffer = project
.update(cx, |project, cx| {
project.open_buffer(project_path.clone(), cx)
})
.await
.ok();
// Create the diagnostics for `main.rs`.
let language_server_id = LanguageServerId(0);
let uri = lsp::Url::from_file_path(path!("/test/main.rs")).unwrap();
let lsp_store = project.read_with(cx, |project, _| project.lsp_store());
lsp_store.update(cx, |lsp_store, cx| {
lsp_store.update_diagnostics(language_server_id, lsp::PublishDiagnosticsParams {
uri: uri.clone(),
diagnostics: vec![
lsp::Diagnostic{
range: lsp::Range::new(lsp::Position::new(5, 6), lsp::Position::new(5, 7)),
severity: Some(lsp::DiagnosticSeverity::WARNING),
message: "use of moved value\nvalue used here after move".to_string(),
related_information: Some(vec![
lsp::DiagnosticRelatedInformation {
location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(2, 8), lsp::Position::new(2, 9))),
message: "move occurs because `y` has type `Vec<char>`, which does not implement the `Copy` trait".to_string()
},
lsp::DiagnosticRelatedInformation {
location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(4, 6), lsp::Position::new(4, 7))),
message: "value moved here".to_string()
},
]),
..Default::default()
},
lsp::Diagnostic{
range: lsp::Range::new(lsp::Position::new(6, 6), lsp::Position::new(6, 7)),
severity: Some(lsp::DiagnosticSeverity::ERROR),
message: "use of moved value\nvalue used here after move".to_string(),
related_information: Some(vec![
lsp::DiagnosticRelatedInformation {
location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(1, 8), lsp::Position::new(1, 9))),
message: "move occurs because `x` has type `Vec<char>`, which does not implement the `Copy` trait".to_string()
},
lsp::DiagnosticRelatedInformation {
location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(3, 6), lsp::Position::new(3, 7))),
message: "value moved here".to_string()
},
]),
..Default::default()
}
],
version: None
}, None, DiagnosticSourceKind::Pushed, &[], cx).unwrap();
// Create diagnostics for other.rs to ensure that the file and
// diagnostics are not included in `BufferDiagnosticsEditor` when it is
// deployed for main.rs.
lsp_store.update_diagnostics(language_server_id, lsp::PublishDiagnosticsParams {
uri: lsp::Url::from_file_path(path!("/test/other.rs")).unwrap(),
diagnostics: vec![
lsp::Diagnostic{
range: lsp::Range::new(lsp::Position::new(1, 8), lsp::Position::new(1, 14)),
severity: Some(lsp::DiagnosticSeverity::WARNING),
message: "unused variable: `unused`".to_string(),
..Default::default()
},
lsp::Diagnostic{
range: lsp::Range::new(lsp::Position::new(2, 4), lsp::Position::new(2, 22)),
severity: Some(lsp::DiagnosticSeverity::ERROR),
message: "cannot find function `undefined_function` in this scope".to_string(),
..Default::default()
}
],
version: None
}, None, DiagnosticSourceKind::Pushed, &[], cx).unwrap();
});
let buffer_diagnostics = window.build_entity(cx, |window, cx| {
BufferDiagnosticsEditor::new(
project_path.clone(),
project.clone(),
buffer,
true,
window,
cx,
)
});
let editor = buffer_diagnostics.update(cx, |buffer_diagnostics, _| {
buffer_diagnostics.editor().clone()
});
// Since the excerpt updates is handled by a background task, we need to
// wait a little bit to ensure that the buffer diagnostic's editor content
// is rendered.
cx.executor()
.advance_clock(DIAGNOSTICS_UPDATE_DELAY + Duration::from_millis(10));
pretty_assertions::assert_eq!(
editor_content_with_blocks(&editor, cx),
indoc::indoc! {
"§ main.rs
§ -----
fn main() {
let x = vec![];
§ move occurs because `x` has type `Vec<char>`, which does not implement
§ the `Copy` trait (back)
let y = vec![];
§ move occurs because `y` has type `Vec<char>`, which does not implement
§ the `Copy` trait
a(x); § value moved here
b(y); § value moved here
c(y);
§ use of moved value
§ value used here after move
d(x);
§ use of moved value
§ value used here after move
§ hint: move occurs because `x` has type `Vec<char>`, which does not
§ implement the `Copy` trait
}"
}
);
}
#[gpui::test]
async fn test_buffer_diagnostics_without_warnings(cx: &mut TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.executor());
fs.insert_tree(
path!("/test"),
json!({
"main.rs": "
fn main() {
let x = vec![];
let y = vec![];
a(x);
b(y);
c(y);
d(x);
}
"
.unindent(),
}),
)
.await;
let project = Project::test(fs.clone(), [path!("/test").as_ref()], cx).await;
let window = cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx));
let cx = &mut VisualTestContext::from_window(*window, cx);
let project_path = project::ProjectPath {
worktree_id: project.read_with(cx, |project, cx| {
project.worktrees(cx).next().unwrap().read(cx).id()
}),
path: Arc::from(Path::new("main.rs")),
};
let buffer = project
.update(cx, |project, cx| {
project.open_buffer(project_path.clone(), cx)
})
.await
.ok();
let language_server_id = LanguageServerId(0);
let uri = lsp::Url::from_file_path(path!("/test/main.rs")).unwrap();
let lsp_store = project.read_with(cx, |project, _| project.lsp_store());
lsp_store.update(cx, |lsp_store, cx| {
lsp_store.update_diagnostics(language_server_id, lsp::PublishDiagnosticsParams {
uri: uri.clone(),
diagnostics: vec![
lsp::Diagnostic{
range: lsp::Range::new(lsp::Position::new(5, 6), lsp::Position::new(5, 7)),
severity: Some(lsp::DiagnosticSeverity::WARNING),
message: "use of moved value\nvalue used here after move".to_string(),
related_information: Some(vec![
lsp::DiagnosticRelatedInformation {
location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(2, 8), lsp::Position::new(2, 9))),
message: "move occurs because `y` has type `Vec<char>`, which does not implement the `Copy` trait".to_string()
},
lsp::DiagnosticRelatedInformation {
location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(4, 6), lsp::Position::new(4, 7))),
message: "value moved here".to_string()
},
]),
..Default::default()
},
lsp::Diagnostic{
range: lsp::Range::new(lsp::Position::new(6, 6), lsp::Position::new(6, 7)),
severity: Some(lsp::DiagnosticSeverity::ERROR),
message: "use of moved value\nvalue used here after move".to_string(),
related_information: Some(vec![
lsp::DiagnosticRelatedInformation {
location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(1, 8), lsp::Position::new(1, 9))),
message: "move occurs because `x` has type `Vec<char>`, which does not implement the `Copy` trait".to_string()
},
lsp::DiagnosticRelatedInformation {
location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(3, 6), lsp::Position::new(3, 7))),
message: "value moved here".to_string()
},
]),
..Default::default()
}
],
version: None
}, None, DiagnosticSourceKind::Pushed, &[], cx).unwrap();
});
let include_warnings = false;
let buffer_diagnostics = window.build_entity(cx, |window, cx| {
BufferDiagnosticsEditor::new(
project_path.clone(),
project.clone(),
buffer,
include_warnings,
window,
cx,
)
});
let editor = buffer_diagnostics.update(cx, |buffer_diagnostics, _cx| {
buffer_diagnostics.editor().clone()
});
// Since the excerpt updates is handled by a background task, we need to
// wait a little bit to ensure that the buffer diagnostic's editor content
// is rendered.
cx.executor()
.advance_clock(DIAGNOSTICS_UPDATE_DELAY + Duration::from_millis(10));
pretty_assertions::assert_eq!(
editor_content_with_blocks(&editor, cx),
indoc::indoc! {
"§ main.rs
§ -----
fn main() {
let x = vec![];
§ move occurs because `x` has type `Vec<char>`, which does not implement
§ the `Copy` trait (back)
let y = vec![];
a(x); § value moved here
b(y);
c(y);
d(x);
§ use of moved value
§ value used here after move
§ hint: move occurs because `x` has type `Vec<char>`, which does not
§ implement the `Copy` trait
}"
}
);
}
#[gpui::test]
async fn test_buffer_diagnostics_multiple_servers(cx: &mut TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.executor());
fs.insert_tree(
path!("/test"),
json!({
"main.rs": "
fn main() {
let x = vec![];
let y = vec![];
a(x);
b(y);
c(y);
d(x);
}
"
.unindent(),
}),
)
.await;
let project = Project::test(fs.clone(), [path!("/test").as_ref()], cx).await;
let window = cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx));
let cx = &mut VisualTestContext::from_window(*window, cx);
let project_path = project::ProjectPath {
worktree_id: project.read_with(cx, |project, cx| {
project.worktrees(cx).next().unwrap().read(cx).id()
}),
path: Arc::from(Path::new("main.rs")),
};
let buffer = project
.update(cx, |project, cx| {
project.open_buffer(project_path.clone(), cx)
})
.await
.ok();
// Create the diagnostics for `main.rs`.
// Two warnings are being created, one for each language server, in order to
// assert that both warnings are rendered in the editor.
let language_server_id_a = LanguageServerId(0);
let language_server_id_b = LanguageServerId(1);
let uri = lsp::Url::from_file_path(path!("/test/main.rs")).unwrap();
let lsp_store = project.read_with(cx, |project, _| project.lsp_store());
lsp_store.update(cx, |lsp_store, cx| {
lsp_store
.update_diagnostics(
language_server_id_a,
lsp::PublishDiagnosticsParams {
uri: uri.clone(),
diagnostics: vec![lsp::Diagnostic {
range: lsp::Range::new(lsp::Position::new(5, 6), lsp::Position::new(5, 7)),
severity: Some(lsp::DiagnosticSeverity::WARNING),
message: "use of moved value\nvalue used here after move".to_string(),
related_information: None,
..Default::default()
}],
version: None,
},
None,
DiagnosticSourceKind::Pushed,
&[],
cx,
)
.unwrap();
lsp_store
.update_diagnostics(
language_server_id_b,
lsp::PublishDiagnosticsParams {
uri: uri.clone(),
diagnostics: vec![lsp::Diagnostic {
range: lsp::Range::new(lsp::Position::new(6, 6), lsp::Position::new(6, 7)),
severity: Some(lsp::DiagnosticSeverity::WARNING),
message: "use of moved value\nvalue used here after move".to_string(),
related_information: None,
..Default::default()
}],
version: None,
},
None,
DiagnosticSourceKind::Pushed,
&[],
cx,
)
.unwrap();
});
let buffer_diagnostics = window.build_entity(cx, |window, cx| {
BufferDiagnosticsEditor::new(
project_path.clone(),
project.clone(),
buffer,
true,
window,
cx,
)
});
let editor = buffer_diagnostics.update(cx, |buffer_diagnostics, _| {
buffer_diagnostics.editor().clone()
});
// Since the excerpt updates is handled by a background task, we need to
// wait a little bit to ensure that the buffer diagnostic's editor content
// is rendered.
cx.executor()
.advance_clock(DIAGNOSTICS_UPDATE_DELAY + Duration::from_millis(10));
pretty_assertions::assert_eq!(
editor_content_with_blocks(&editor, cx),
indoc::indoc! {
"§ main.rs
§ -----
a(x);
b(y);
c(y);
§ use of moved value
§ value used here after move
d(x);
§ use of moved value
§ value used here after move
}"
}
);
buffer_diagnostics.update(cx, |buffer_diagnostics, _cx| {
assert_eq!(
*buffer_diagnostics.summary(),
DiagnosticSummary {
warning_count: 2,
error_count: 0
}
);
})
}
fn init_test(cx: &mut TestAppContext) {
cx.update(|cx| {
zlog::init_test();

5
crates/diagnostics/src/toolbar_controls.rs
View file

@ -40,11 +40,6 @@ impl Render for ToolbarControls {
if let Some(editor) = editor.upgrade() {
let diagnostics = editor.read(cx);
include_warnings = diagnostics.include_warnings;
// TODO: How to calculate this for the
// `BufferDiagnosticsEditor`? Should we simply keep track if
// there are any updates to the diagnostics for the path and
// mark that instead of automatically updating?
has_stale_excerpts = false;
is_updating = diagnostics.update_excerpts_task.is_some()
|| diagnostics
.project

26
crates/project/src/lsp_store.rs
View file

@ -7123,14 +7123,28 @@ impl LspStore {
project_path: &ProjectPath,
_: &App,
) -> DiagnosticSummary {
// TODO: If there's multiple `DiagnosticSummary` but for
// different language servers, which one should be returned?
self.diagnostic_summaries
if let Some(summaries) = self
.diagnostic_summaries
.get(&project_path.worktree_id)
.and_then(|map| map.get(&project_path.path))
.and_then(|summaries| summaries.iter().next())
.map(|(_language_server_id, summary)| summary.clone())
.unwrap_or_default()
{
let (error_count, warning_count) = summaries.iter().fold(
(0, 0),
|(error_count, warning_count), (_language_server_id, summary)| {
(
error_count + summary.error_count,
warning_count + summary.warning_count,
)
},
);
DiagnosticSummary {
error_count,
warning_count,
}
} else {
DiagnosticSummary::default()
}
}
pub fn diagnostic_summaries<'a>(