From 9f3235a37f5402c30a64278ec4dc09f396fcbf91 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 15 Nov 2024 17:09:15 +0100 Subject: [PATCH] [red-knot] Expand test corpus (#14360) ## Summary - Add 383 files from `crates/ruff_python_parser/resources` to the test corpus - Add 1296 files from `crates/ruff_linter/resources` to the test corpus - Use in-memory file system for tests - Improve test isolation by cleaning the test environment between checks - Add a mechanism for "known failures". Mark ~80 files as known failures. - The corpus test is now a lot slower (6 seconds). Note: While `red_knot` as a command line tool can run over all of these files without panicking, we still have a lot of test failures caused by explicitly "pulling" all types. ## Test Plan Run `cargo test -p red_knot_workspace` while making sure that - Introducing code that is known to lead to a panic fails the test - Removing code that is known to lead to a panic from `KNOWN_FAILURES`-files also fails the test --- Cargo.lock | 2 +- crates/red_knot_workspace/Cargo.toml | 2 +- crates/red_knot_workspace/tests/check.rs | 217 ++++++++++++++++++++--- crates/ruff_db/src/system/path.rs | 6 + crates/ruff_db/src/system/test.rs | 4 +- 5 files changed, 201 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1f7584b8fd..8841efcacf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2353,6 +2353,7 @@ version = "0.0.0" dependencies = [ "anyhow", "crossbeam", + "glob", "notify", "rayon", "red_knot_python_semantic", @@ -2363,7 +2364,6 @@ dependencies = [ "ruff_text_size", "rustc-hash 2.0.0", "salsa", - "tempfile", "tracing", ] diff --git a/crates/red_knot_workspace/Cargo.toml b/crates/red_knot_workspace/Cargo.toml index 1c0d326df1..944aab459a 100644 --- a/crates/red_knot_workspace/Cargo.toml +++ b/crates/red_knot_workspace/Cargo.toml @@ -30,7 +30,7 @@ tracing = { workspace = true } [dev-dependencies] ruff_db = { workspace = true, features = ["testing"] } -tempfile = { workspace = true } +glob = { workspace = true } [features] default = ["zstd"] diff --git a/crates/red_knot_workspace/tests/check.rs b/crates/red_knot_workspace/tests/check.rs index 6097247590..94b9c57316 100644 --- a/crates/red_knot_workspace/tests/check.rs +++ b/crates/red_knot_workspace/tests/check.rs @@ -1,48 +1,122 @@ -use std::fs; -use std::path::PathBuf; - use red_knot_python_semantic::{HasTy, SemanticModel}; use red_knot_workspace::db::RootDatabase; use red_knot_workspace::workspace::WorkspaceMetadata; use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; -use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf}; +use ruff_db::system::{SystemPath, SystemPathBuf, TestSystem}; use ruff_python_ast::visitor::source_order; use ruff_python_ast::visitor::source_order::SourceOrderVisitor; use ruff_python_ast::{self as ast, Alias, Expr, Parameter, ParameterWithDefault, Stmt}; -fn setup_db(workspace_root: &SystemPath) -> anyhow::Result { - let system = OsSystem::new(workspace_root); +fn setup_db(workspace_root: &SystemPath, system: TestSystem) -> anyhow::Result { let workspace = WorkspaceMetadata::from_path(workspace_root, &system, None)?; RootDatabase::new(workspace, system) } -/// Test that all snippets in testcorpus can be checked without panic +fn get_workspace_root() -> anyhow::Result { + Ok(SystemPathBuf::from(String::from_utf8( + std::process::Command::new("cargo") + .args(["locate-project", "--workspace", "--message-format", "plain"]) + .output()? + .stdout, + )?) + .parent() + .unwrap() + .to_owned()) +} + +/// Test that all snippets in testcorpus can be checked without panic (except for [`KNOWN_FAILURES`]) #[test] #[allow(clippy::print_stdout)] fn corpus_no_panic() -> anyhow::Result<()> { - let root = SystemPathBuf::from_path_buf(tempfile::TempDir::new()?.into_path()).unwrap(); - let db = setup_db(&root)?; + let root = SystemPathBuf::from("/src"); - let corpus = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("resources/test/corpus"); + let system = TestSystem::default(); + let memory_fs = system.memory_file_system(); + memory_fs.create_directory_all(root.as_ref())?; - for path in fs::read_dir(&corpus)? { - let source = path?.path(); - println!("checking {source:?}"); - let source_fn = source.file_name().unwrap().to_str().unwrap(); - let py_dest = root.join(source_fn); - fs::copy(&source, py_dest.as_std_path())?; - // this test is only asserting that we can pull every expression type without a panic - // (and some non-expressions that clearly define a single type) - let file = system_path_to_file(&db, py_dest).unwrap(); - pull_types(&db, file); + let mut db = setup_db(&root, system.clone())?; - // try the file as a stub also - println!("re-checking as .pyi"); - let pyi_dest = root.join(format!("{source_fn}i")); - std::fs::copy(source, pyi_dest.as_std_path())?; - let file = system_path_to_file(&db, pyi_dest).unwrap(); - pull_types(&db, file); + let crate_root = String::from(env!("CARGO_MANIFEST_DIR")); + let workspace_root = get_workspace_root()?; + let workspace_root = workspace_root.to_string(); + + let corpus = vec![ + format!("{crate_root}/resources/test/corpus/**/*.py"), + format!("{workspace_root}/crates/ruff_python_parser/resources/**/*.py"), + format!("{workspace_root}/crates/ruff_linter/resources/**/*.py"), + // TODO: Enable running over typeshed stubs once there are fewer failures: + // format!("{workspace_root}/crates/red_knot_vendored/vendor/typeshed/**/*.pyi"), + ] + .into_iter() + .flat_map(|pattern| glob::glob(&pattern).unwrap()); + + for path in corpus { + let path = path?; + let relative_path = path.strip_prefix(&workspace_root)?; + + let (py_expected_to_fail, pyi_expected_to_fail) = KNOWN_FAILURES + .iter() + .find_map(|(path, py_fail, pyi_fail)| { + if Some(*path) + == relative_path + .to_str() + .map(|p| p.replace('\\', "/")) + .as_deref() + { + Some((*py_fail, *pyi_fail)) + } else { + None + } + }) + .unwrap_or((false, false)); + + let source = path.as_path(); + let source_filename = source.file_name().unwrap().to_str().unwrap(); + + let code = std::fs::read_to_string(source)?; + + let mut check_with_file_name = |path: &SystemPath| { + memory_fs.write_file(path, &code).unwrap(); + File::sync_path(&mut db, path); + + // this test is only asserting that we can pull every expression type without a panic + // (and some non-expressions that clearly define a single type) + let file = system_path_to_file(&db, path).unwrap(); + + let result = std::panic::catch_unwind(|| pull_types(&db, file)); + + let expected_to_fail = if path.extension().map(|e| e == "pyi").unwrap_or(false) { + pyi_expected_to_fail + } else { + py_expected_to_fail + }; + if let Err(err) = result { + if !expected_to_fail { + println!("Check failed for {relative_path:?}. Consider fixing it or adding it to KNOWN_FAILURES"); + std::panic::resume_unwind(err); + } + } else { + assert!(!expected_to_fail, "Expected to panic, but did not. Consider removing this path from KNOWN_FAILURES"); + } + + memory_fs.remove_all(); + file.sync(&mut db); + }; + + if source.extension().map(|e| e == "pyi").unwrap_or(false) { + println!("checking {relative_path:?}"); + let pyi_dest = root.join(source_filename); + check_with_file_name(&pyi_dest); + } else { + println!("checking {relative_path:?}"); + let py_dest = root.join(source_filename); + check_with_file_name(&py_dest); + + let pyi_dest = root.join(format!("{source_filename}i")); + println!("re-checking as stub file: {pyi_dest:?}"); + check_with_file_name(&pyi_dest); + } } Ok(()) } @@ -144,3 +218,94 @@ impl SourceOrderVisitor<'_> for PullTypesVisitor<'_> { source_order::walk_alias(self, alias); } } + +/// Whether or not the .py/.pyi version of this file is expected to fail +const KNOWN_FAILURES: &[(&str, bool, bool)] = &[ + // Probably related to missing support for type aliases / type params: + ("crates/ruff_python_parser/resources/inline/err/type_param_invalid_bound_expr.py", true, true), + ("crates/ruff_python_parser/resources/inline/err/type_param_type_var_invalid_default_expr.py", true, true), + ("crates/ruff_python_parser/resources/inline/err/type_param_param_spec_invalid_default_expr.py", true, true), + ("crates/ruff_python_parser/resources/inline/err/type_param_type_var_missing_default.py", true, true), + ("crates/ruff_python_parser/resources/inline/err/type_param_type_var_tuple_invalid_default_expr.py", true, true), + ("crates/ruff_python_parser/resources/inline/ok/type_param_param_spec.py", true, true), + ("crates/ruff_python_parser/resources/inline/ok/type_param_type_var.py", true, true), + ("crates/ruff_python_parser/resources/inline/ok/type_param_type_var_tuple.py", true, true), + ("crates/ruff_python_parser/resources/valid/statement/type.py", true, true), + // Fails for unknown reasons: + ("crates/ruff_python_parser/resources/valid/expressions/f_string.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_future_annotations/no_future_import_uses_union_inner.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI011.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI015.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI016.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI019.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI020.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI020.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI030.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI034.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI034.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI035.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI035.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI036.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI036.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI041.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI041.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI051.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI051.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI052.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI052.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI063.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI063.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI064.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI064.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH004_13.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH004_13.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH004_15.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TCH004_15.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote2.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote2.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote3.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote3.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F401_19.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F401_19.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F541.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F541.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F632.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F632.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F811_19.py", true, false), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_0.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_0.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_14.py", false, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_15.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_15.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_17.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_17.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_2.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_2.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_20.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_20.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_26.py", true, false), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/project/foo/bar.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/project/foo/bar.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/project/foo/bop/baz.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyflakes/project/foo/bop/baz.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pylint/single_string_slots.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pylint/single_string_slots.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_0.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_0.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyupgrade/UP039.py", true, false), + ("crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/pyupgrade/UP044.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/ruff/RUF013_0.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/ruff/RUF013_0.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/ruff/RUF013_3.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/ruff/RUF013_3.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/ruff/RUF022.py", true, true), + ("crates/ruff_linter/resources/test/fixtures/ruff/RUF022.py", true, true), +]; diff --git a/crates/ruff_db/src/system/path.rs b/crates/ruff_db/src/system/path.rs index 25cc854c43..346009183d 100644 --- a/crates/ruff_db/src/system/path.rs +++ b/crates/ruff_db/src/system/path.rs @@ -491,6 +491,12 @@ impl From<&str> for SystemPathBuf { } } +impl From for SystemPathBuf { + fn from(value: String) -> Self { + SystemPathBuf::from_utf8_path_buf(Utf8PathBuf::from(value)) + } +} + impl Default for SystemPathBuf { fn default() -> Self { Self::new() diff --git a/crates/ruff_db/src/system/test.rs b/crates/ruff_db/src/system/test.rs index 2b8ecdfc29..4194c90dc0 100644 --- a/crates/ruff_db/src/system/test.rs +++ b/crates/ruff_db/src/system/test.rs @@ -21,7 +21,7 @@ use super::walk_directory::WalkDirectoryBuilder; /// /// ## Warning /// Don't use this system for production code. It's intended for testing only. -#[derive(Default, Debug)] +#[derive(Default, Debug, Clone)] pub struct TestSystem { inner: TestSystemInner, } @@ -229,7 +229,7 @@ pub trait DbWithTestSystem: Db + Sized { } } -#[derive(Debug)] +#[derive(Debug, Clone)] enum TestSystemInner { Stub(MemoryFileSystem), System(Arc),