diff --git a/README.md b/README.md index 9100d91..5a45ae2 100644 --- a/README.md +++ b/README.md @@ -43,41 +43,41 @@ To learn more about using tryke, see the [documentation](https://tryke.dev/). ```python from typing import Annotated -import tryke as t +from tryke import Depends, describe, expect, fixture, test -@t.fixture(per="scope") +@fixture(per="scope") def database(): db = {} yield db db.clear() -with t.describe("users"): +with describe("users"): - @t.fixture - def users(database: Annotated[dict[str, dict[str, str]], t.Depends(database)]): + @fixture + def users(database: Annotated[dict[str, dict[str, str]], Depends(database)]): database["users"] = {} return database["users"] - with t.describe("get"): + with describe("get"): - @t.test("returns a stored user") - async def test_get(users: Annotated[dict[str, str], t.Depends(users)]): + @test("returns a stored user") + async def test_get(users: Annotated[dict[str, str], Depends(users)]): users["alice"] = "alice@example.com" - t.expect(users["alice"], name="returns stored email").to_equal( + expect(users["alice"], name="returns stored email").to_equal( "alice@example.com" ) - with t.describe("set"): + with describe("set"): - @t.test("stores a new user") - async def test_set(users: Annotated[dict[str, str], t.Depends(users)]): + @test("stores a new user") + async def test_set(users: Annotated[dict[str, str], Depends(users)]): users["bob"] = "bob@example.com" - t.expect(users["bob"], name="stores email under user key").to_equal( + expect(users["bob"], name="stores email under user key").to_equal( "bob@example.com" ) diff --git a/crates/tryke_discovery/src/db.rs b/crates/tryke_discovery/src/db.rs index a51bbdc..345b462 100644 --- a/crates/tryke_discovery/src/db.rs +++ b/crates/tryke_discovery/src/db.rs @@ -1,5 +1,8 @@ use std::path::PathBuf; +use log::trace; +use ruff_python_ast::{ModModule, Stmt}; +use ruff_python_parser::parse_module; use tryke_types::ParsedFile; #[salsa::db] @@ -17,6 +20,83 @@ pub struct SourceFile { pub path: PathBuf, } +/// Parsed Python source cached as the first incremental layer. +/// +/// Equality intentionally ignores raw source text. If the parser produces the +/// same AST body for a new source string, Salsa keeps the old value and +/// backdates dependents, so discovery is not re-run for trivia-only edits. +#[derive(Debug, Clone)] +pub(crate) struct ParsedAst { + source: String, + syntax: Option, +} + +impl ParsedAst { + pub(crate) fn parse(source: &str) -> Self { + let syntax = parse_module(source) + .ok() + .map(ruff_python_parser::Parsed::into_syntax); + Self { + source: source.to_owned(), + syntax, + } + } + + pub(crate) fn source(&self) -> &str { + &self.source + } + + pub(crate) fn syntax(&self) -> Option<&ModModule> { + self.syntax.as_ref() + } + + fn body(&self) -> Option<&[Stmt]> { + self.syntax.as_ref().map(|module| module.body.as_slice()) + } +} + +impl PartialEq for ParsedAst { + fn eq(&self, other: &Self) -> bool { + self.body() == other.body() + } +} + +#[cfg(test)] +static DISCOVER_FILE_EXECUTIONS: std::sync::atomic::AtomicUsize = + std::sync::atomic::AtomicUsize::new(0); + +#[cfg(test)] +static COUNTED_DISCOVER_FILE_PATH: std::sync::Mutex> = std::sync::Mutex::new(None); + +#[cfg(test)] +pub(crate) fn count_discover_file_executions_for(path: PathBuf) { + *COUNTED_DISCOVER_FILE_PATH.lock().expect("counter mutex") = Some(path); + DISCOVER_FILE_EXECUTIONS.store(0, std::sync::atomic::Ordering::SeqCst); +} + +#[cfg(test)] +pub(crate) fn discover_file_executions() -> usize { + DISCOVER_FILE_EXECUTIONS.load(std::sync::atomic::Ordering::SeqCst) +} + +#[cfg(test)] +fn count_discover_file_execution(path: &std::path::Path) { + let counted = COUNTED_DISCOVER_FILE_PATH.lock().expect("counter mutex"); + if counted.as_deref() == Some(path) { + DISCOVER_FILE_EXECUTIONS.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + } +} + +#[salsa::tracked(returns(ref))] +pub(crate) fn parse_file(db: &dyn Db, file: SourceFile) -> ParsedAst { + let path = file.path(db); + trace!( + "parsing {}", + path.strip_prefix(file.root(db)).unwrap_or(path).display() + ); + ParsedAst::parse(file.text(db)) +} + /// Everything derivable from a single parse of a Python source file: /// the `ParsedFile` (tests, hooks, guard-else lines, errors), the /// candidate import paths this file references, and the dynamic-import @@ -33,11 +113,14 @@ pub struct DiscoveredFile { #[salsa::tracked] pub fn discover_file(db: &dyn Db, file: SourceFile) -> DiscoveredFile { - crate::discover_file_from_source( + #[cfg(test)] + count_discover_file_execution(file.path(db)); + + crate::discover_file_from_ast( file.root(db), file.src_roots(db), file.path(db), - file.text(db), + parse_file(db, file), ) } diff --git a/crates/tryke_discovery/src/discoverer.rs b/crates/tryke_discovery/src/discoverer.rs index 6403ad0..13f178a 100644 --- a/crates/tryke_discovery/src/discoverer.rs +++ b/crates/tryke_discovery/src/discoverer.rs @@ -756,6 +756,44 @@ mod tests { assert_eq!(second.len(), 2); } + #[test] + fn discoverer_skips_discovery_when_ast_body_is_unchanged() { + let source_one = "from tryke import test\n\n@test\ndef test_one():\n pass\n# first\n"; + let dir = make_project(&[("test_example.py", source_one)]); + let path = dir + .path() + .join("test_example.py") + .canonicalize() + .expect("canonicalize test file"); + let mut discoverer = Discoverer::new(dir.path()); + crate::db::count_discover_file_executions_for(path.clone()); + + let first = discoverer.rediscover(); + assert_eq!(first.len(), 1); + assert_eq!(crate::db::discover_file_executions(), 1); + + let source_two = + "from tryke import test\n\n@test\ndef test_one():\n pass\n# second comment\n"; + fs::write(&path, source_two).expect("overwrite with trivia-only change"); + let second = discoverer.rediscover_changed(std::slice::from_ref(&path)); + assert_eq!(second.len(), 1); + assert_eq!( + crate::db::discover_file_executions(), + 1, + "trivia-only source changes should not re-run discovery" + ); + + let source_three = "from tryke import test\n\n@test\ndef test_one():\n pass\n\n@test\ndef test_two():\n pass\n# second comment\n"; + fs::write(&path, source_three).expect("overwrite with AST change"); + let third = discoverer.rediscover_changed(std::slice::from_ref(&path)); + assert_eq!(third.len(), 2); + assert_eq!( + crate::db::discover_file_executions(), + 2, + "AST changes should re-run discovery" + ); + } + #[test] fn discoverer_saves_cache_under_custom_cache_dir() { let source = "@test\ndef test_hello():\n pass\n"; diff --git a/crates/tryke_discovery/src/lib.rs b/crates/tryke_discovery/src/lib.rs index 2bd34f7..ca96bdc 100644 --- a/crates/tryke_discovery/src/lib.rs +++ b/crates/tryke_discovery/src/lib.rs @@ -17,11 +17,13 @@ pub use discoverer::Discoverer; use ignore::WalkBuilder; use ruff_python_ast::{Expr, Stmt}; -use ruff_python_parser::parse_module; use ruff_source_file::LineIndex; use ruff_text_size::{Ranged, TextRange, TextSize}; use tryke_types::{ExpectedAssertion, FixturePer, HookItem, ParsedFile, TestItem}; +#[cfg(test)] +use ruff_python_parser::parse_module; + pub(crate) fn find_project_root(start: &Path) -> Option { start .ancestors() @@ -1837,27 +1839,23 @@ fn collect_doctests_from_body( } } -/// Parse `source` once and produce everything discovery needs: the +/// Walk a parsed source file once and produce everything discovery needs: the /// `ParsedFile` (tests, hooks, guard-else lines, errors), the project-local /// imports this file depends on, and whether it contains dynamic imports. -/// Folding all three derivations into a single AST walk avoids the prior -/// cold-start cost of parsing each file twice. -pub(crate) fn discover_file_from_source( +/// Keeping this downstream of `db::parse_file` lets Salsa skip discovery when +/// source text changes but the parsed AST body is unchanged. +pub(crate) fn discover_file_from_ast( root: &Path, src_roots: &[PathBuf], file: &Path, - source: &str, + parsed: &crate::db::ParsedAst, ) -> crate::db::DiscoveredFile { - trace!( - "parsing {}", - file.strip_prefix(root).unwrap_or(file).display() - ); - let Ok(parsed) = parse_module(source) else { + let Some(module) = parsed.syntax() else { trace!("parse error in {}", file.display()); return crate::db::DiscoveredFile::default(); }; + let source = parsed.source(); let line_index = LineIndex::from_source_text(source); - let module = parsed.syntax(); let body = &module.body; let aliases = TrykeAliases::collect(body); let mut tests = Vec::new(); @@ -1901,7 +1899,8 @@ pub(crate) fn parse_tests_from_source( file: &Path, source: &str, ) -> ParsedFile { - discover_file_from_source(root, src_roots, file, source).parsed + let parsed = crate::db::ParsedAst::parse(source); + discover_file_from_ast(root, src_roots, file, &parsed).parsed } fn parse_tests_from_file(root: &Path, src_roots: &[PathBuf], file: &Path) -> ParsedFile { diff --git a/docs/index.md b/docs/index.md index 5790c82..d4a876a 100644 --- a/docs/index.md +++ b/docs/index.md @@ -59,41 +59,41 @@ Or, check out the [tryke playground](https://playground.tryke.dev) to try it out ```python from typing import Annotated -import tryke as t +from tryke import Depends, describe, expect, fixture, test -@t.fixture(per="scope") +@fixture(per="scope") def database(): db = {} yield db db.clear() -with t.describe("users"): +with describe("users"): - @t.fixture - def users(database: Annotated[dict[str, dict[str, str]], t.Depends(database)]): + @fixture + def users(database: Annotated[dict[str, dict[str, str]], Depends(database)]): database["users"] = {} return database["users"] - with t.describe("get"): + with describe("get"): - @t.test("returns a stored user") - async def test_get(users: Annotated[dict[str, str], t.Depends(users)]): + @test("returns a stored user") + async def test_get(users: Annotated[dict[str, str], Depends(users)]): users["alice"] = "alice@example.com" - t.expect(users["alice"], name="returns stored email").to_equal( + expect(users["alice"], name="returns stored email").to_equal( "alice@example.com" ) - with t.describe("set"): + with describe("set"): - @t.test("stores a new user") - async def test_set(users: Annotated[dict[str, str], t.Depends(users)]): + @test("stores a new user") + async def test_set(users: Annotated[dict[str, str], Depends(users)]): users["bob"] = "bob@example.com" - t.expect(users["bob"], name="stores email under user key").to_equal( + expect(users["bob"], name="stores email under user key").to_equal( "bob@example.com" )