Skip to content

Commit

Permalink
red_knot_test: add support for diagnostic snapshotting
Browse files Browse the repository at this point in the history
This ties together everything from the previous commits.
Some interesting bits here are how the snapshot is generated
(where we include relevant info to make it easier to review
the snapshots) and also a tweak to how inline assertions are
processed.

This commit also includes some example snapshots just to get
a sense of what they look like. Follow-up work should add
more of these I think.
  • Loading branch information
BurntSushi committed Feb 5, 2025
1 parent 8d4679b commit a84b27e
Show file tree
Hide file tree
Showing 8 changed files with 308 additions and 18 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions crates/red_knot_python_semantic/resources/mdtest/import/basic.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,18 @@ reveal_type(c.C) # revealed: Literal[C]
class C: ...
```

## Unresolvable module import

<!-- snapshot-diagnostics -->

```py
import zqzqzqzqzqzqzq # error: [unresolved-import] "Cannot resolve import `zqzqzqzqzqzqzq`"
```

## Unresolvable submodule imports

<!-- snapshot-diagnostics -->

```py
# Topmost component resolvable, submodule not resolvable:
import a.foo # error: [unresolved-import] "Cannot resolve import `a.foo`"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
source: crates/red_knot_test/src/lib.rs
expression: snapshot
---
---
mdtest name: basic.md - Structures - Unresolvable module import
mdtest path: crates/red_knot_python_semantic/resources/mdtest/import/basic.md
---

# Python source files

## mdtest_snippet__1.py

```
1 | import zqzqzqzqzqzqzq # error: [unresolved-import] "Cannot resolve import `zqzqzqzqzqzqzq`"
```

# Diagnostics

```
error: lint:unresolved-import
--> /src/mdtest_snippet__1.py:1:8
|
1 | import zqzqzqzqzqzqzq # error: [unresolved-import] "Cannot resolve import `zqzqzqzqzqzqzq`"
| ^^^^^^^^^^^^^^ Cannot resolve import `zqzqzqzqzqzqzq`
|
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
source: crates/red_knot_test/src/lib.rs
expression: snapshot
---
---
mdtest name: basic.md - Structures - Unresolvable submodule imports
mdtest path: crates/red_knot_python_semantic/resources/mdtest/import/basic.md
---

# Python source files

## mdtest_snippet__1.py

```
1 | # Topmost component resolvable, submodule not resolvable:
2 | import a.foo # error: [unresolved-import] "Cannot resolve import `a.foo`"
3 |
4 | # Topmost component unresolvable:
5 | import b.foo # error: [unresolved-import] "Cannot resolve import `b.foo`"
```

## a/__init__.py

```
```

# Diagnostics

```
error: lint:unresolved-import
--> /src/mdtest_snippet__1.py:2:8
|
1 | # Topmost component resolvable, submodule not resolvable:
2 | import a.foo # error: [unresolved-import] "Cannot resolve import `a.foo`"
| ^^^^^ Cannot resolve import `a.foo`
3 |
4 | # Topmost component unresolvable:
|
```

```
error: lint:unresolved-import
--> /src/mdtest_snippet__1.py:5:8
|
4 | # Topmost component unresolvable:
5 | import b.foo # error: [unresolved-import] "Cannot resolve import `b.foo`"
| ^^^^^ Cannot resolve import `b.foo`
|
```
12 changes: 6 additions & 6 deletions crates/red_knot_python_semantic/tests/mdtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@ use dir_test::{dir_test, Fixture};
)]
#[allow(clippy::needless_pass_by_value)]
fn mdtest(fixture: Fixture<&str>) {
let fixture_path = Utf8Path::new(fixture.path());
let absolute_fixture_path = Utf8Path::new(fixture.path());
let crate_dir = Utf8Path::new(env!("CARGO_MANIFEST_DIR"));
let snapshot_path = crate_dir.join("resources").join("mdtest").join("snapshots");
let workspace_root = crate_dir.ancestors().nth(2).unwrap();

let long_title = fixture_path.strip_prefix(workspace_root).unwrap();
let short_title = fixture_path.file_name().unwrap();
let relative_fixture_path = absolute_fixture_path.strip_prefix(workspace_root).unwrap();
let short_title = absolute_fixture_path.file_name().unwrap();

let test_name = test_name("mdtest", fixture_path);
let test_name = test_name("mdtest", absolute_fixture_path);

red_knot_test::run(
fixture_path,
absolute_fixture_path,
relative_fixture_path,
&snapshot_path,
long_title.as_str(),
short_title,
&test_name,
);
Expand Down
1 change: 1 addition & 0 deletions crates/red_knot_test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ ruff_text_size = { workspace = true }
anyhow = { workspace = true }
camino = { workspace = true }
colored = { workspace = true }
insta = { workspace = true, features = ["filters"] }
memchr = { workspace = true }
regex = { workspace = true }
rustc-hash = { workspace = true }
Expand Down
103 changes: 91 additions & 12 deletions crates/red_knot_test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@ const MDTEST_TEST_FILTER: &str = "MDTEST_TEST_FILTER";
/// Panic on test failure, and print failure details.
#[allow(clippy::print_stdout)]
pub fn run(
fixture_path: &Utf8Path,
absolute_fixture_path: &Utf8Path,
relative_fixture_path: &Utf8Path,
snapshot_path: &Utf8Path,
long_title: &str,
short_title: &str,
test_name: &str,
) {
let source = std::fs::read_to_string(fixture_path).unwrap();
let source = std::fs::read_to_string(absolute_fixture_path).unwrap();
let suite = match test_parser::parse(short_title, &source) {
Ok(suite) => suite,
Err(err) => {
panic!("Error parsing `{fixture_path}`: {err:?}")
panic!("Error parsing `{absolute_fixture_path}`: {err:?}")
}
};

Expand All @@ -60,7 +60,7 @@ pub fn run(
db.memory_file_system().remove_all();
Files::sync_all(&mut db);

if let Err(failures) = run_test(&mut db, snapshot_path, &test) {
if let Err(failures) = run_test(&mut db, relative_fixture_path, snapshot_path, &test) {
any_failures = true;
println!("\n{}\n", test.name().bold().underline());

Expand All @@ -73,7 +73,8 @@ pub fn run(
for failure in failures {
let absolute_line_number =
backtick_line.checked_add(relative_line_number).unwrap();
let line_info = format!("{long_title}:{absolute_line_number}").cyan();
let line_info =
format!("{relative_fixture_path}:{absolute_line_number}").cyan();
println!(" {line_info} {failure}");
}
}
Expand All @@ -97,6 +98,7 @@ pub fn run(

fn run_test(
db: &mut db::Db,
relative_fixture_path: &Utf8Path,
snapshot_path: &Utf8Path,
test: &parser::MarkdownTest,
) -> Result<(), Failures> {
Expand Down Expand Up @@ -186,6 +188,10 @@ fn run_test(
)
.expect("Failed to update Program settings in TestDb");

// When snapshot testing is enabled, this is populated with
// all diagnostics. Otherwise it remains empty.
let mut snapshot_diagnostics = vec![];

let failures: Failures = test_files
.into_iter()
.filter_map(|test_file| {
Expand Down Expand Up @@ -234,16 +240,36 @@ fn run_test(
diagnostic
}));

match matcher::match_file(db, test_file.file, diagnostics) {
Ok(()) => None,
Err(line_failures) => Some(FileFailures {
backtick_offset: test_file.backtick_offset,
by_line: line_failures,
}),
let failure =
match matcher::match_file(db, test_file.file, diagnostics.iter().map(|d| &**d)) {
Ok(()) => None,
Err(line_failures) => Some(FileFailures {
backtick_offset: test_file.backtick_offset,
by_line: line_failures,
}),
};
if test.should_snapshot_diagnostics() {
snapshot_diagnostics.extend(diagnostics);
}
failure
})
.collect();

if !snapshot_diagnostics.is_empty() {
let snapshot =
create_diagnostic_snapshot(db, relative_fixture_path, test, snapshot_diagnostics);
let name = test.name().replace(' ', "_");
insta::with_settings!(
{
snapshot_path => snapshot_path,
input_file => name.clone(),
filters => vec![(r"\\", "/")],
prepend_module_to_snapshot => false,
},
{ insta::assert_snapshot!(name, snapshot) }
);
}

if failures.is_empty() {
Ok(())
} else {
Expand All @@ -254,6 +280,7 @@ fn run_test(
type Failures = Vec<FileFailures>;

/// The failures for a single file in a test by line number.
#[derive(Debug)]
struct FileFailures {
/// The offset of the backticks that starts the code block in the Markdown file
backtick_offset: TextSize,
Expand All @@ -268,3 +295,55 @@ struct TestFile {
// Offset of the backticks that starts the code block in the Markdown file
backtick_offset: TextSize,
}

fn create_diagnostic_snapshot<D: Diagnostic>(
db: &mut db::Db,
relative_fixture_path: &Utf8Path,
test: &parser::MarkdownTest,
diagnostics: impl IntoIterator<Item = D>,
) -> String {
// TODO(ag): Do something better than requiring this
// global state to be twiddled everywhere.
colored::control::set_override(false);

let mut snapshot = String::new();
writeln!(snapshot).unwrap();
writeln!(snapshot, "---").unwrap();
writeln!(snapshot, "mdtest name: {}", test.name()).unwrap();
writeln!(snapshot, "mdtest path: {relative_fixture_path}").unwrap();
writeln!(snapshot, "---").unwrap();
writeln!(snapshot).unwrap();

writeln!(snapshot, "# Python source files").unwrap();
writeln!(snapshot).unwrap();
for file in test.files() {
writeln!(snapshot, "## {}", file.path).unwrap();
writeln!(snapshot).unwrap();
// Note that we don't use ```py here because the line numbering
// we add makes it invalid Python. This sacrifices syntax
// highlighting when you look at the snapshot on GitHub,
// but the line numbers are extremely useful for analyzing
// snapshots. So we keep them.
writeln!(snapshot, "```").unwrap();

let line_number_width = file.code.lines().count().to_string().len();
for (i, line) in file.code.lines().enumerate() {
let line_number = i + 1;
writeln!(snapshot, "{line_number:>line_number_width$} | {line}").unwrap();
}
writeln!(snapshot, "```").unwrap();
writeln!(snapshot).unwrap();
}

writeln!(snapshot, "# Diagnostics").unwrap();
writeln!(snapshot).unwrap();
for (i, diag) in diagnostics.into_iter().enumerate() {
if i > 0 {
writeln!(snapshot).unwrap();
}
writeln!(snapshot, "```").unwrap();
writeln!(snapshot, "{}", diag.display(db)).unwrap();
writeln!(snapshot, "```").unwrap();
}
snapshot
}
Loading

0 comments on commit a84b27e

Please sign in to comment.