From 1f1376b09e86a5b1185d158575ced874d927bcca Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Thu, 14 May 2026 22:59:48 -0400 Subject: [PATCH 1/2] ide: refactor test fixtures --- .../src/code_actions/rewrite_from.rs | 9 +-- .../src/code_actions/rewrite_leading_from.rs | 12 ++-- .../squawk_ide/src/code_actions/test_utils.rs | 60 +++++++++++-------- crates/squawk_ide/src/completion.rs | 15 ++--- crates/squawk_ide/src/expand_selection.rs | 19 +++--- crates/squawk_ide/src/find_references.rs | 16 +++-- crates/squawk_ide/src/goto_definition.rs | 18 +++--- crates/squawk_ide/src/hover.rs | 17 ++---- crates/squawk_ide/src/test_utils.rs | 24 +++++++- 9 files changed, 100 insertions(+), 90 deletions(-) diff --git a/crates/squawk_ide/src/code_actions/rewrite_from.rs b/crates/squawk_ide/src/code_actions/rewrite_from.rs index 7341ca7f..ad70f082 100644 --- a/crates/squawk_ide/src/code_actions/rewrite_from.rs +++ b/crates/squawk_ide/src/code_actions/rewrite_from.rs @@ -39,14 +39,15 @@ mod test { use insta::assert_snapshot; use crate::code_actions::test_utils::{ - apply_code_action, code_action_not_applicable, code_action_not_applicable_with_errors, + apply_code_action_with_errors, code_action_not_applicable, + code_action_not_applicable_with_errors, }; use super::rewrite_from; #[test] fn rewrite_from_simple() { - assert_snapshot!(apply_code_action( + assert_snapshot!(apply_code_action_with_errors( rewrite_from, "from$0 t;"), @"select * from t;" @@ -55,7 +56,7 @@ mod test { #[test] fn rewrite_from_qualified() { - assert_snapshot!(apply_code_action( + assert_snapshot!(apply_code_action_with_errors( rewrite_from, "from$0 s.t;"), @"select * from s.t;" @@ -64,7 +65,7 @@ mod test { #[test] fn rewrite_from_on_name() { - assert_snapshot!(apply_code_action( + assert_snapshot!(apply_code_action_with_errors( rewrite_from, "from t$0;"), @"select * from t;" diff --git a/crates/squawk_ide/src/code_actions/rewrite_leading_from.rs b/crates/squawk_ide/src/code_actions/rewrite_leading_from.rs index c8945aa7..561ac229 100644 --- a/crates/squawk_ide/src/code_actions/rewrite_leading_from.rs +++ b/crates/squawk_ide/src/code_actions/rewrite_leading_from.rs @@ -55,13 +55,15 @@ pub(super) fn rewrite_leading_from( mod test { use insta::assert_snapshot; - use crate::code_actions::test_utils::{apply_code_action, code_action_not_applicable}; + use crate::code_actions::test_utils::{ + apply_code_action_with_errors, code_action_not_applicable, + }; use super::rewrite_leading_from; #[test] fn rewrite_leading_from_simple() { - assert_snapshot!(apply_code_action( + assert_snapshot!(apply_code_action_with_errors( rewrite_leading_from, "from$0 t select c;"), @"select c from t;" @@ -70,7 +72,7 @@ mod test { #[test] fn rewrite_leading_from_multiple_cols() { - assert_snapshot!(apply_code_action( + assert_snapshot!(apply_code_action_with_errors( rewrite_leading_from, "from$0 t select a, b;"), @"select a, b from t;" @@ -79,7 +81,7 @@ mod test { #[test] fn rewrite_leading_from_with_where() { - assert_snapshot!(apply_code_action( + assert_snapshot!(apply_code_action_with_errors( rewrite_leading_from, "from$0 t select c where x = 1;"), @"select c from t where x = 1;" @@ -88,7 +90,7 @@ mod test { #[test] fn rewrite_leading_from_on_select() { - assert_snapshot!(apply_code_action( + assert_snapshot!(apply_code_action_with_errors( rewrite_leading_from, "from t sel$0ect c;"), @"select c from t;" diff --git a/crates/squawk_ide/src/code_actions/test_utils.rs b/crates/squawk_ide/src/code_actions/test_utils.rs index 2a2dd08e..3724e09d 100644 --- a/crates/squawk_ide/src/code_actions/test_utils.rs +++ b/crates/squawk_ide/src/code_actions/test_utils.rs @@ -3,7 +3,7 @@ use salsa::Database as Db; use squawk_linter::Edit; use squawk_syntax::ast; -use crate::db::{Database, File}; +use crate::db::File; use crate::test_utils::Fixture; use super::{ActionKind, CodeAction}; @@ -13,15 +13,33 @@ pub(super) fn apply_code_action( f: impl Fn(&dyn Db, File, &mut Vec, TextSize) -> Option<()>, sql: &str, ) -> String { - let fixture = Fixture::new(sql); + apply_code_action_(f, sql, false) +} + +#[must_use] +pub(super) fn apply_code_action_with_errors( + f: impl Fn(&dyn Db, File, &mut Vec, TextSize) -> Option<()>, + sql: &str, +) -> String { + apply_code_action_(f, sql, true) +} + +fn apply_code_action_( + f: impl Fn(&dyn Db, File, &mut Vec, TextSize) -> Option<()>, + sql: &str, + allow_errors: bool, +) -> String { + let fixture = if allow_errors { + Fixture::new_allow_errors(sql) + } else { + Fixture::new(sql) + }; let offset = fixture.marker().offset_before(); - let sql = fixture.sql(); - let db = Database::default(); - let file = File::new(&db, sql.into()); - let parse_result = crate::db::parse(&db, file); + let db = fixture.db(); + let file = fixture.file(); let mut actions = vec![]; - f(&db, file, &mut actions, offset); + f(db, file, &mut actions, offset); assert!( !actions.is_empty(), @@ -30,16 +48,7 @@ pub(super) fn apply_code_action( let action = &actions[0]; - match action.kind { - ActionKind::QuickFix => { - // Quickfixes can fix syntax errors so we don't assert - } - ActionKind::RefactorRewrite => { - assert_eq!(parse_result.errors(), vec![]); - } - } - - let mut result = sql.to_string(); + let mut result = file.content(db).to_string(); let mut edits = action.edits.clone(); edits.sort_by_key(|e| e.text_range.start()); @@ -94,18 +103,17 @@ fn code_action_not_applicable_( sql: &str, allow_errors: bool, ) -> bool { - let fixture = Fixture::new(sql); + let fixture = if allow_errors { + Fixture::new_allow_errors(sql) + } else { + Fixture::new(sql) + }; let offset = fixture.marker().offset_before(); - let sql = fixture.sql(); - let db = Database::default(); - let file = File::new(&db, sql.into()); - let parse_result = crate::db::parse(&db, file); - if !allow_errors { - assert_eq!(parse_result.errors(), vec![]); - } + let db = fixture.db(); + let file = fixture.file(); let mut actions = vec![]; - f(&db, file, &mut actions, offset); + f(db, file, &mut actions, offset); actions.is_empty() } diff --git a/crates/squawk_ide/src/completion.rs b/crates/squawk_ide/src/completion.rs index 2fd031c6..d294b554 100644 --- a/crates/squawk_ide/src/completion.rs +++ b/crates/squawk_ide/src/completion.rs @@ -935,7 +935,6 @@ pub struct CompletionItem { #[cfg(test)] mod tests { use super::completion; - use crate::db::{Database, File}; use crate::test_utils::Fixture; use insta::assert_snapshot; use tabled::builder::Builder; @@ -943,12 +942,9 @@ mod tests { #[must_use] fn completions(sql: &str) -> String { - let fixture = Fixture::new(sql); + let fixture = Fixture::new_allow_errors(sql); let offset = fixture.marker().offset(); - let sql = fixture.sql(); - let db = Database::default(); - let file = File::new(&db, sql.into()); - let items = completion(&db, file, offset); + let items = completion(fixture.db(), fixture.file(), offset); assert!( !items.is_empty(), "No completions found. If this was intended, use `completions_not_found` instead." @@ -957,12 +953,9 @@ mod tests { } fn completions_not_found(sql: &str) { - let fixture = Fixture::new(sql); + let fixture = Fixture::new_allow_errors(sql); let offset = fixture.marker().offset(); - let sql = fixture.sql(); - let db = Database::default(); - let file = File::new(&db, sql.into()); - let items = completion(&db, file, offset); + let items = completion(fixture.db(), fixture.file(), offset); assert_eq!( items, vec![], diff --git a/crates/squawk_ide/src/expand_selection.rs b/crates/squawk_ide/src/expand_selection.rs index 27275aa2..27e72ced 100644 --- a/crates/squawk_ide/src/expand_selection.rs +++ b/crates/squawk_ide/src/expand_selection.rs @@ -292,16 +292,15 @@ mod tests { use super::*; use crate::test_utils::Fixture; use insta::assert_debug_snapshot; - use squawk_syntax::{SourceFile, ast::AstNode}; + use squawk_syntax::ast::AstNode; #[must_use] fn expand(sql: &str) -> Vec { let fixture = Fixture::new(sql); let offset = fixture.marker().offset(); - let sql = fixture.sql(); - let parse = SourceFile::parse(sql); - let file = parse.tree(); - let root = file.syntax(); + let sql = fixture.file().content(fixture.db()).clone(); + let tree = crate::db::parse(fixture.db(), fixture.file()).tree(); + let root = tree.syntax(); let mut range = TextRange::empty(offset); let mut results = vec![]; @@ -346,15 +345,15 @@ select 'some stret$0ched out words in a string' #[test] fn string() { assert_debug_snapshot!(expand(r" -select b'foo$0 bar' +select e'foo$0 bar' 'buzz'; "), @r#" [ "foo", - "b'foo bar'", - "b'foo bar'\n'buzz'", - "select b'foo bar'\n'buzz'", - "\nselect b'foo bar'\n'buzz';\n", + "e'foo bar'", + "e'foo bar'\n'buzz'", + "select e'foo bar'\n'buzz'", + "\nselect e'foo bar'\n'buzz';\n", ] "#); } diff --git a/crates/squawk_ide/src/find_references.rs b/crates/squawk_ide/src/find_references.rs index 906df3bf..dcdd7465 100644 --- a/crates/squawk_ide/src/find_references.rs +++ b/crates/squawk_ide/src/find_references.rs @@ -39,7 +39,7 @@ pub fn find_references(db: &dyn Db, file: File, offset: TextSize) -> Vec> = FxHashMap::default(); for (i, location) in references.iter().enumerate() { @@ -75,7 +73,7 @@ mod test { let multi_file = refs_by_file.len() > 1 || !refs_by_file.contains_key(¤t_file); - let mut snippet = Snippet::source(sql).fold(true); + let mut snippet = Snippet::source(current_file.content(db).as_ref()).fold(true); if multi_file { snippet = snippet.path(*file_paths.get(¤t_file).unwrap()); } @@ -88,7 +86,7 @@ mod test { for (ref_file, refs) in refs_by_file { let path = file_paths.get(&ref_file).unwrap(); - let other_snippet = Snippet::source(ref_file.content(&db).as_ref()) + let other_snippet = Snippet::source(ref_file.content(db).as_ref()) .path(*path) .fold(true); let other_snippet = annotate_refs(other_snippet, refs); diff --git a/crates/squawk_ide/src/goto_definition.rs b/crates/squawk_ide/src/goto_definition.rs index 19966ab2..fb5cf2fc 100644 --- a/crates/squawk_ide/src/goto_definition.rs +++ b/crates/squawk_ide/src/goto_definition.rs @@ -132,7 +132,7 @@ fn find_following_commit_or_rollback( #[cfg(test)] mod test { use crate::builtins::builtins_file; - use crate::db::{Database, File}; + use crate::db::File; use crate::goto_definition::goto_definition; use crate::test_utils::Fixture; use annotate_snippets::{AnnotationKind, Level, Renderer, Snippet, renderer::DecorStyle}; @@ -149,25 +149,23 @@ mod test { #[track_caller] fn goto_(sql: &str) -> Option { - info!("starting"); let fixture = Fixture::new(sql); // For go to def we want the previous character since we usually put the // marker after the item we're trying to go to def on. let marker = fixture.marker(); let offset = marker.offset_before(); let source_span = marker.range(); - let sql = fixture.sql(); - let db = Database::default(); - let current_file = File::new(&db, sql.into()); - assert_eq!(crate::db::parse(&db, current_file).errors(), vec![]); - let results = goto_definition(&db, current_file, offset); + let db = fixture.db(); + let current_file = fixture.file(); + + let results = goto_definition(db, current_file, offset); if results.is_empty() { return None; } let mut file_paths = FxHashMap::default(); file_paths.insert(current_file, "current.sql"); - file_paths.insert(builtins_file(&db), "builtins.sql"); + file_paths.insert(builtins_file(db), "builtins.sql"); let mut dests_by_file: FxHashMap> = FxHashMap::default(); for (i, location) in results.iter().enumerate() { @@ -179,7 +177,7 @@ mod test { let multi_file = dests_by_file.len() > 1 || !dests_by_file.contains_key(¤t_file); - let mut snippet = Snippet::source(current_file.content(&db).as_ref()).fold(true); + let mut snippet = Snippet::source(current_file.content(db).as_ref()).fold(true); if multi_file { snippet = snippet.path(*file_paths.get(¤t_file).unwrap()); } @@ -192,7 +190,7 @@ mod test { for (dest_file, dests) in dests_by_file { let path = file_paths.get(&dest_file).unwrap(); - let other_snippet = Snippet::source(dest_file.content(&db).as_ref()) + let other_snippet = Snippet::source(dest_file.content(db).as_ref()) .path(*path) .fold(true); let other_snippet = annotate_destinations(other_snippet, dests); diff --git a/crates/squawk_ide/src/hover.rs b/crates/squawk_ide/src/hover.rs index 52ec534a..aa0d4c65 100644 --- a/crates/squawk_ide/src/hover.rs +++ b/crates/squawk_ide/src/hover.rs @@ -1599,7 +1599,6 @@ fn unqualified_star_in_arg_list_ptrs( #[cfg(test)] mod test { - use crate::db::{Database, File}; use crate::hover::hover; use crate::test_utils::Fixture; use annotate_snippets::{AnnotationKind, Level, Renderer, Snippet, renderer::DecorStyle}; @@ -1613,19 +1612,17 @@ mod test { #[track_caller] fn check_hover_(sql: &str) -> Option { - let db = Database::default(); let fixture = Fixture::new(sql); let marker = fixture.marker(); let offset = marker.offset_before(); let hover_span = marker.range(); - let sql = fixture.sql(); - let file = File::new(&db, sql.into()); - assert_eq!(crate::db::parse(&db, file).errors(), vec![]); + let db = fixture.db(); + let file = fixture.file(); - if let Some(type_info) = hover(&db, file, offset) { + if let Some(type_info) = hover(db, file, offset) { let title = format!("hover: {}", type_info.snippet); let group = Level::INFO.primary_title(&title).element( - Snippet::source(sql) + Snippet::source(file.content(db).as_ref()) .fold(true) .annotation(AnnotationKind::Context.span(hover_span).label("hover")), ); @@ -1644,14 +1641,10 @@ mod test { #[must_use] #[track_caller] fn check_hover_info(sql: &str) -> super::Hover { - let db = Database::default(); let fixture = Fixture::new(sql); let offset = fixture.marker().offset_before(); - let sql = fixture.sql(); - let file = File::new(&db, sql.into()); - assert_eq!(crate::db::parse(&db, file).errors(), vec![]); - hover(&db, file, offset).expect("should find hover information") + hover(fixture.db(), fixture.file(), offset).expect("should find hover information") } #[test] diff --git a/crates/squawk_ide/src/test_utils.rs b/crates/squawk_ide/src/test_utils.rs index 78430f59..47bb2040 100644 --- a/crates/squawk_ide/src/test_utils.rs +++ b/crates/squawk_ide/src/test_utils.rs @@ -1,5 +1,6 @@ use std::ops::Range; +use crate::db::{Database, File}; use rowan::TextSize; // TODO: we should probably use something else since `$0` is valid syntax, maybe `%0`? @@ -8,6 +9,8 @@ const MARKER: &str = "$0"; pub(crate) struct Fixture { marker_offset: TextSize, sql: String, + db: Database, + file: File, } pub(crate) struct Marker { @@ -19,10 +22,22 @@ pub(crate) struct Marker { impl Fixture { #[track_caller] pub(crate) fn new(sql: &str) -> Self { + let fixture = Self::new_allow_errors(sql); + assert_eq!(crate::db::parse(&fixture.db, fixture.file).errors(), vec![]); + fixture + } + + #[track_caller] + pub(crate) fn new_allow_errors(sql: &str) -> Self { if let Some(pos) = sql.find(MARKER) { + let sql = sql.replace(MARKER, ""); + let db = Database::default(); + let file = File::new(&db, sql.as_str().into()); return Self { marker_offset: TextSize::new(pos as u32), - sql: sql.replace(MARKER, ""), + sql, + db, + file, }; } panic!("No marker found in SQL. Did you forget to add a marker `$0`?"); @@ -57,8 +72,11 @@ impl Fixture { start..start + len } - pub(crate) fn sql(&self) -> &str { - &self.sql + pub(crate) fn db(&self) -> &Database { + &self.db + } + pub(crate) fn file(&self) -> File { + self.file } } From a8f8f151afc6e6f5d4ae1bdf7e7c813c1e5323af Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Fri, 15 May 2026 00:04:19 -0400 Subject: [PATCH 2/2] fix --- crates/squawk_ide/src/goto_definition.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/squawk_ide/src/goto_definition.rs b/crates/squawk_ide/src/goto_definition.rs index fb5cf2fc..cc171d71 100644 --- a/crates/squawk_ide/src/goto_definition.rs +++ b/crates/squawk_ide/src/goto_definition.rs @@ -137,7 +137,6 @@ mod test { use crate::test_utils::Fixture; use annotate_snippets::{AnnotationKind, Level, Renderer, Snippet, renderer::DecorStyle}; use insta::assert_snapshot; - use log::info; use rowan::TextRange; use rustc_hash::FxHashMap;