Skip to content

Commit ac5d220

Browse files
authored
[red-knot] Fix python setting in mdtests, and rewrite a site-packages test as an mdtest (#17222)
## Summary This PR does the following things: - Fixes the `python` configuration setting for mdtest (added in #17221) so that it expects a path pointing to a venv's `sys.prefix` variable rather than the a path pointing to the venv's `site-packages` subdirectory. This brings the `python` setting in mdtest in sync with our CLI `--python` flag. - Tweaks mdtest so that it automatically creates a valid `pyvenv.cfg` file for you if you don't specify one. This makes it much more ergonomic to write an mdtest with a custom `python` setting: red-knot will reject a `python` setting that points to a directory that doesn't have a `pyvenv.cfg` file in it - Tweaks mdtest so that it doesn't check a custom `pyvenv.cfg` as Python source code if you _do_ add a custom `pyvenv.cfg` file for your mock virtual environment in an mdtest. (You get a lot of diagnostics about Python syntax errors in the `pyvenv.cfg` file, otherwise!) - Rewrites the test added in #17178 as an mdtest, and deletes the original test that was added in that PR ## Test Plan I verified that the new mdtest fails if I revert the changes to `resolver.rs` that were added in #17178
1 parent 73a9974 commit ac5d220

File tree

6 files changed

+132
-51
lines changed

6 files changed

+132
-51
lines changed

crates/red_knot_python_semantic/resources/mdtest/import/relative.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,3 +236,36 @@ X: int = 42
236236
```py
237237
from .parser import X # error: [unresolved-import]
238238
```
239+
240+
## Relative imports in `site-packages`
241+
242+
Relative imports in `site-packages` are correctly resolved even when the `site-packages` search path
243+
is a subdirectory of the first-party search path. Note that mdtest sets the first-party search path
244+
to `/src/`, which is why the virtual environment in this test is a subdirectory of `/src/`, even
245+
though this is not how a typical Python project would be structured:
246+
247+
```toml
248+
[environment]
249+
python = "/src/.venv"
250+
python-version = "3.13"
251+
```
252+
253+
`/src/bar.py`:
254+
255+
```py
256+
from foo import A
257+
258+
reveal_type(A) # revealed: Literal[A]
259+
```
260+
261+
`/src/.venv/<path-to-site-packages>/foo/__init__.py`:
262+
263+
```py
264+
from .a import A as A
265+
```
266+
267+
`/src/.venv/<path-to-site-packages>/foo/a.py`:
268+
269+
```py
270+
class A: ...
271+
```

crates/red_knot_python_semantic/src/db.rs

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub(crate) mod tests {
1919
use std::sync::Arc;
2020

2121
use crate::program::{Program, SearchPathSettings};
22-
use crate::{default_lint_registry, ProgramSettings, PythonPath, PythonPlatform};
22+
use crate::{default_lint_registry, ProgramSettings, PythonPlatform};
2323

2424
use super::Db;
2525
use crate::lint::{LintRegistry, RuleSelection};
@@ -139,8 +139,6 @@ pub(crate) mod tests {
139139
python_version: PythonVersion,
140140
/// Target Python platform
141141
python_platform: PythonPlatform,
142-
/// Paths to the directory to use for `site-packages`
143-
site_packages: Vec<SystemPathBuf>,
144142
/// Path and content pairs for files that should be present
145143
files: Vec<(&'a str, &'a str)>,
146144
}
@@ -150,7 +148,6 @@ pub(crate) mod tests {
150148
Self {
151149
python_version: PythonVersion::default(),
152150
python_platform: PythonPlatform::default(),
153-
site_packages: vec![],
154151
files: vec![],
155152
}
156153
}
@@ -169,14 +166,6 @@ pub(crate) mod tests {
169166
self
170167
}
171168

172-
pub(crate) fn with_site_packages_search_path(
173-
mut self,
174-
path: &(impl AsRef<SystemPath> + ?Sized),
175-
) -> Self {
176-
self.site_packages.push(path.as_ref().to_path_buf());
177-
self
178-
}
179-
180169
pub(crate) fn build(self) -> anyhow::Result<TestDb> {
181170
let mut db = TestDb::new();
182171

@@ -186,15 +175,12 @@ pub(crate) mod tests {
186175
db.write_files(self.files)
187176
.context("Failed to write test files")?;
188177

189-
let mut search_paths = SearchPathSettings::new(vec![src_root]);
190-
search_paths.python_path = PythonPath::KnownSitePackages(self.site_packages);
191-
192178
Program::from_settings(
193179
&db,
194180
ProgramSettings {
195181
python_version: self.python_version,
196182
python_platform: self.python_platform,
197-
search_paths,
183+
search_paths: SearchPathSettings::new(vec![src_root]),
198184
},
199185
)
200186
.context("Failed to configure Program settings")?;

crates/red_knot_python_semantic/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ pub use module_resolver::{resolve_module, system_module_search_paths, KnownModul
1010
pub use program::{Program, ProgramSettings, PythonPath, SearchPathSettings};
1111
pub use python_platform::PythonPlatform;
1212
pub use semantic_model::{HasType, SemanticModel};
13+
pub use site_packages::SysPrefixPathOrigin;
1314

1415
pub mod ast_node_ref;
1516
mod db;

crates/red_knot_python_semantic/src/types/infer.rs

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7392,15 +7392,15 @@ impl StringPartsCollector {
73927392

73937393
#[cfg(test)]
73947394
mod tests {
7395-
use crate::db::tests::{setup_db, TestDb, TestDbBuilder};
7395+
use crate::db::tests::{setup_db, TestDb};
73967396
use crate::semantic_index::definition::Definition;
73977397
use crate::semantic_index::symbol::FileScopeId;
73987398
use crate::semantic_index::{global_scope, semantic_index, symbol_table, use_def_map};
73997399
use crate::symbol::global_symbol;
74007400
use crate::types::check_types;
74017401
use ruff_db::diagnostic::Diagnostic;
74027402
use ruff_db::files::{system_path_to_file, File};
7403-
use ruff_db::system::{DbWithWritableSystem as _, SystemPath};
7403+
use ruff_db::system::DbWithWritableSystem as _;
74047404
use ruff_db::testing::{assert_function_query_was_not_run, assert_function_query_was_run};
74057405

74067406
use super::*;
@@ -7556,26 +7556,6 @@ mod tests {
75567556
Ok(())
75577557
}
75587558

7559-
#[test]
7560-
fn relative_import_resolution_in_site_packages_when_site_packages_is_subdirectory_of_first_party_search_path(
7561-
) {
7562-
let project_root = SystemPath::new("/src");
7563-
let foo_dot_py = project_root.join("foo.py");
7564-
let site_packages = project_root.join(".venv/lib/python3.13/site-packages");
7565-
7566-
let db = TestDbBuilder::new()
7567-
.with_site_packages_search_path(&site_packages)
7568-
.with_file(&foo_dot_py, "from bar import A")
7569-
.with_file(&site_packages.join("bar/__init__.py"), "from .a import *")
7570-
.with_file(&site_packages.join("bar/a.py"), "class A: ...")
7571-
.build()
7572-
.unwrap();
7573-
7574-
assert_file_diagnostics(&db, foo_dot_py.as_str(), &[]);
7575-
let a_symbol = get_symbol(&db, foo_dot_py.as_str(), &[], "A");
7576-
assert!(a_symbol.expect_type().is_class_literal());
7577-
}
7578-
75797559
#[test]
75807560
fn pep695_type_params() {
75817561
let mut db = setup_db();

crates/red_knot_test/README.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,45 @@ typeshed = "/typeshed"
314314

315315
For more details, take a look at the [custom-typeshed Markdown test].
316316

317+
### Mocking a virtual environment
318+
319+
Mdtest supports mocking a virtual environment for a specific test at an arbitrary location, again
320+
using the `[environment]` configuration option:
321+
322+
````markdown
323+
```toml
324+
[environment]
325+
python = ".venv"
326+
```
327+
````
328+
329+
Red-knot will reject virtual environments that do not have valid `pyvenv.cfg` files at the
330+
virtual-environment directory root (here, `.venv/pyvenv.cfg`). However, if a `pyvenv.cfg` file does
331+
not have its contents specified by the test, mdtest will automatically generate one for you, to
332+
make mocking a virtual environment more ergonomic.
333+
334+
Mdtest also makes it easy to write Python packages to the mock virtual environment's
335+
`site-packages` directory using the `<path-to-site-packages>` magic path segment. This would
336+
otherwise be hard, due to the fact that the `site-packages` subdirectory in a virtual environment
337+
is located at a different relative path depending on the platform the virtual environment was
338+
created on. In the following test, mdtest will write the Python file to
339+
`.venv/Lib/site-packages/foo.py` in its in-memory filesystem used for the test if the test is being
340+
executed on Windows, and `.venv/lib/python3.13/site-packages/foo.py` otherwise:
341+
342+
````markdown
343+
```toml
344+
[environment]
345+
python = ".venv"
346+
python-version = "3.13"
347+
```
348+
349+
`.venv/<path-to-site-packages>/foo.py`:
350+
351+
```py
352+
X = 1
353+
```
354+
````
355+
317356
## Documentation of tests
318357

319358
Arbitrary Markdown syntax (including of course normal prose paragraphs) is permitted (and ignored by

crates/red_knot_test/src/lib.rs

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ use colored::Colorize;
55
use config::SystemKind;
66
use parser as test_parser;
77
use red_knot_python_semantic::types::check_types;
8-
use red_knot_python_semantic::{Program, ProgramSettings, PythonPath, SearchPathSettings};
8+
use red_knot_python_semantic::{
9+
Program, ProgramSettings, PythonPath, SearchPathSettings, SysPrefixPathOrigin,
10+
};
911
use ruff_db::diagnostic::{create_parse_diagnostic, Diagnostic, DisplayDiagnosticConfig};
1012
use ruff_db::files::{system_path_to_file, File};
1113
use ruff_db::panic::catch_unwind;
@@ -158,8 +160,12 @@ fn run_test(
158160

159161
let src_path = project_root.clone();
160162
let custom_typeshed_path = test.configuration().typeshed();
163+
let python_path = test.configuration().python();
164+
let python_version = test.configuration().python_version().unwrap_or_default();
165+
161166
let mut typeshed_files = vec![];
162167
let mut has_custom_versions_file = false;
168+
let mut has_custom_pyvenv_cfg_file = false;
163169

164170
let test_files: Vec<_> = test
165171
.files()
@@ -169,11 +175,11 @@ fn run_test(
169175
}
170176

171177
assert!(
172-
matches!(embedded.lang, "py" | "pyi" | "python" | "text"),
173-
"Supported file types are: py (or python), pyi, text, and ignore"
178+
matches!(embedded.lang, "py" | "pyi" | "python" | "text" | "cfg"),
179+
"Supported file types are: py (or python), pyi, text, cfg and ignore"
174180
);
175181

176-
let full_path = embedded.full_path(&project_root);
182+
let mut full_path = embedded.full_path(&project_root);
177183

178184
if let Some(typeshed_path) = custom_typeshed_path {
179185
if let Ok(relative_path) = full_path.strip_prefix(typeshed_path.join("stdlib")) {
@@ -183,11 +189,35 @@ fn run_test(
183189
typeshed_files.push(relative_path.to_path_buf());
184190
}
185191
}
192+
} else if let Some(python_path) = python_path {
193+
if let Ok(relative_path) = full_path.strip_prefix(python_path) {
194+
if relative_path.as_str() == "pyvenv.cfg" {
195+
has_custom_pyvenv_cfg_file = true;
196+
} else {
197+
let mut new_path = SystemPathBuf::new();
198+
for component in full_path.components() {
199+
let component = component.as_str();
200+
if component == "<path-to-site-packages>" {
201+
if cfg!(target_os = "windows") {
202+
new_path.push("Lib");
203+
new_path.push("site-packages");
204+
} else {
205+
new_path.push("lib");
206+
new_path.push(format!("python{python_version}"));
207+
new_path.push("site-packages");
208+
}
209+
} else {
210+
new_path.push(component);
211+
}
212+
}
213+
full_path = new_path;
214+
}
215+
}
186216
}
187217

188218
db.write_file(&full_path, &embedded.code).unwrap();
189219

190-
if !full_path.starts_with(&src_path) || embedded.lang == "text" {
220+
if !(full_path.starts_with(&src_path) && matches!(embedded.lang, "py" | "pyi")) {
191221
// These files need to be written to the file system (above), but we don't run any checks on them.
192222
return None;
193223
}
@@ -221,22 +251,34 @@ fn run_test(
221251
}
222252
}
223253

254+
if let Some(python_path) = python_path {
255+
if !has_custom_pyvenv_cfg_file {
256+
let pyvenv_cfg_file = python_path.join("pyvenv.cfg");
257+
let home_directory = SystemPathBuf::from(format!("/Python{python_version}"));
258+
db.create_directory_all(&home_directory).unwrap();
259+
db.write_file(&pyvenv_cfg_file, format!("home = {home_directory}"))
260+
.unwrap();
261+
}
262+
}
263+
224264
let configuration = test.configuration();
225265

226266
let settings = ProgramSettings {
227-
python_version: configuration.python_version().unwrap_or_default(),
267+
python_version,
228268
python_platform: configuration.python_platform().unwrap_or_default(),
229269
search_paths: SearchPathSettings {
230270
src_roots: vec![src_path],
231271
extra_paths: configuration.extra_paths().unwrap_or_default().to_vec(),
232272
custom_typeshed: custom_typeshed_path.map(SystemPath::to_path_buf),
233-
python_path: PythonPath::KnownSitePackages(
234-
configuration
235-
.python()
236-
.into_iter()
237-
.map(SystemPath::to_path_buf)
238-
.collect(),
239-
),
273+
python_path: configuration
274+
.python()
275+
.map(|sys_prefix| {
276+
PythonPath::SysPrefix(
277+
sys_prefix.to_path_buf(),
278+
SysPrefixPathOrigin::PythonCliFlag,
279+
)
280+
})
281+
.unwrap_or(PythonPath::KnownSitePackages(vec![])),
240282
},
241283
};
242284

0 commit comments

Comments
 (0)