mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-25 12:55:17 +00:00
test(ruff_python_formatter): Run all Black tests (#2993)
This PR changes the testing infrastructure to run all black tests and: * Pass if Ruff and Black generate the same formatting * Fail and write a markdown snapshot that shows the input code, the differences between Black and Ruff, Ruffs output, and Blacks output This is achieved by introducing a new `fixture` macro (open to better name suggestions) that "duplicates" the attributed test for every file that matches the specified glob pattern. Creating a new test for each file over having a test that iterates over all files has the advantage that you can run a single test, and that test failures indicate which case is failing. The `fixture` macro also makes it straightforward to e.g. setup our own spec tests that test very specific formatting by creating a new folder and use insta to assert the formatted output.
This commit is contained in:
parent
262e768fd3
commit
ed33b75bad
119 changed files with 12989 additions and 981 deletions
|
@ -20,8 +20,6 @@ mod format;
|
|||
mod newlines;
|
||||
mod parentheses;
|
||||
pub mod shared_traits;
|
||||
#[cfg(test)]
|
||||
mod test;
|
||||
pub mod trivia;
|
||||
|
||||
pub fn fmt(contents: &str) -> Result<Formatted<ASTFormatContext>> {
|
||||
|
@ -57,70 +55,98 @@ pub fn fmt(contents: &str) -> Result<Formatted<ASTFormatContext>> {
|
|||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
|
||||
use anyhow::Result;
|
||||
use test_case::test_case;
|
||||
|
||||
use crate::fmt;
|
||||
use crate::test::test_resource_path;
|
||||
use ruff_testing_macros::fixture;
|
||||
use similar::TextDiff;
|
||||
use std::fmt::{Formatter, Write};
|
||||
|
||||
#[fixture(
|
||||
pattern = "resources/test/fixtures/black/**/*.py",
|
||||
// Excluded tests because they reach unreachable when attaching tokens
|
||||
exclude = [
|
||||
"*comments.py",
|
||||
"*comments[3,5,8].py",
|
||||
"*comments_non_breaking_space.py",
|
||||
"*docstring_preview.py",
|
||||
"*docstring.py",
|
||||
"*fmtonoff.py",
|
||||
"*fmtskip8.py",
|
||||
])
|
||||
]
|
||||
#[test]
|
||||
fn black_test(input_path: &Path) -> Result<()> {
|
||||
let content = fs::read_to_string(input_path)?;
|
||||
|
||||
#[test_case(Path::new("simple_cases/beginning_backslash.py"); "beginning_backslash")]
|
||||
#[test_case(Path::new("simple_cases/class_blank_parentheses.py"); "class_blank_parentheses")]
|
||||
#[test_case(Path::new("simple_cases/class_methods_new_line.py"); "class_methods_new_line")]
|
||||
#[test_case(Path::new("simple_cases/import_spacing.py"); "import_spacing")]
|
||||
#[test_case(Path::new("simple_cases/one_element_subscript.py"); "one_element_subscript")]
|
||||
#[test_case(Path::new("simple_cases/power_op_spacing.py"); "power_op_spacing")]
|
||||
#[test_case(Path::new("simple_cases/remove_newline_after_code_block_open.py"); "remove_newline_after_code_block_open")]
|
||||
#[test_case(Path::new("simple_cases/slices.py"); "slices")]
|
||||
#[test_case(Path::new("simple_cases/tricky_unicode_symbols.py"); "tricky_unicode_symbols")]
|
||||
// Passing except that `1, 2, 3,` should be `(1, 2, 3)`.
|
||||
#[test_case(Path::new("simple_cases/tupleassign.py"); "tupleassign")]
|
||||
// Passing except that `CliRunner().invoke(...)` arguments are improperly wrapped.
|
||||
#[test_case(Path::new("simple_cases/function2.py"); "function2")]
|
||||
fn passing(path: &Path) -> Result<()> {
|
||||
let snapshot = format!("{}", path.display());
|
||||
let content = std::fs::read_to_string(test_resource_path(
|
||||
Path::new("fixtures/black").join(path).as_path(),
|
||||
))?;
|
||||
let formatted = fmt(&content)?;
|
||||
insta::assert_display_snapshot!(snapshot, formatted.print()?.as_code());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(Path::new("simple_cases/collections.py"); "collections")]
|
||||
#[test_case(Path::new("simple_cases/bracketmatch.py"); "bracketmatch")]
|
||||
fn passing_modulo_string_normalization(path: &Path) -> Result<()> {
|
||||
fn adjust_quotes(contents: &str) -> String {
|
||||
// Replace all single quotes with double quotes.
|
||||
contents.replace('\'', "\"")
|
||||
let expected_path = input_path.with_extension("py.expect");
|
||||
let expected_output = fs::read_to_string(&expected_path)
|
||||
.unwrap_or_else(|_| panic!("Expected Black output file '{expected_path:?}' to exist"));
|
||||
|
||||
let printed = formatted.print()?;
|
||||
let formatted_code = printed.as_code();
|
||||
|
||||
if formatted_code == expected_output {
|
||||
// Black and Ruff formatting matches. Delete any existing snapshot files because the Black output
|
||||
// already perfectly captures the expected output.
|
||||
// The following code mimics insta's logic generating the snapshot name for a test.
|
||||
let workspace_path = std::env::var("CARGO_MANIFEST_DIR").unwrap();
|
||||
let snapshot_name = insta::_function_name!()
|
||||
.strip_prefix(&format!("{}::", module_path!()))
|
||||
.unwrap();
|
||||
let module_path = module_path!().replace("::", "__");
|
||||
|
||||
let snapshot_path = Path::new(&workspace_path)
|
||||
.join("src/snapshots")
|
||||
.join(&format!(
|
||||
"{module_path}__{}.snap",
|
||||
snapshot_name.replace(&['/', '\\'][..], "__")
|
||||
));
|
||||
|
||||
if snapshot_path.exists() && snapshot_path.is_file() {
|
||||
// SAFETY: This is a convenience feature. That's why we don't want to abort
|
||||
// when deleting a no longer needed snapshot fails.
|
||||
fs::remove_file(&snapshot_path).ok();
|
||||
}
|
||||
|
||||
let new_snapshot_path = snapshot_path.with_extension("snap.new");
|
||||
if new_snapshot_path.exists() && new_snapshot_path.is_file() {
|
||||
// SAFETY: This is a convenience feature. That's why we don't want to abort
|
||||
// when deleting a no longer needed snapshot fails.
|
||||
fs::remove_file(&new_snapshot_path).ok();
|
||||
}
|
||||
} else {
|
||||
// Black and Ruff have different formatting. Write out a snapshot that covers the differences
|
||||
// today.
|
||||
let mut snapshot = String::new();
|
||||
write!(snapshot, "{}", Header::new("Input"))?;
|
||||
write!(snapshot, "{}", CodeFrame::new("py", &content))?;
|
||||
|
||||
write!(snapshot, "{}", Header::new("Black Differences"))?;
|
||||
|
||||
let diff = TextDiff::from_lines(expected_output.as_str(), formatted_code)
|
||||
.unified_diff()
|
||||
.header("Black", "Ruff")
|
||||
.to_string();
|
||||
|
||||
write!(snapshot, "{}", CodeFrame::new("diff", &diff))?;
|
||||
|
||||
write!(snapshot, "{}", Header::new("Ruff Output"))?;
|
||||
write!(snapshot, "{}", CodeFrame::new("py", formatted_code))?;
|
||||
|
||||
write!(snapshot, "{}", Header::new("Black Output"))?;
|
||||
write!(snapshot, "{}", CodeFrame::new("py", &expected_output))?;
|
||||
|
||||
insta::with_settings!({ omit_expression => false, input_file => input_path }, {
|
||||
insta::assert_snapshot!(snapshot);
|
||||
});
|
||||
}
|
||||
|
||||
let snapshot = format!("{}", path.display());
|
||||
let content = std::fs::read_to_string(test_resource_path(
|
||||
Path::new("fixtures/black").join(path).as_path(),
|
||||
))?;
|
||||
let formatted = fmt(&content)?;
|
||||
insta::assert_display_snapshot!(snapshot, adjust_quotes(formatted.print()?.as_code()));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[ignore]
|
||||
// Lots of deviations, _mostly_ related to string normalization and wrapping.
|
||||
#[test_case(Path::new("simple_cases/expression.py"); "expression")]
|
||||
// Passing apart from a trailing end-of-line comment within an if statement, which is being
|
||||
// inappropriately associated with the if statement rather than the line it's on.
|
||||
#[test_case(Path::new("simple_cases/comments.py"); "comments")]
|
||||
#[test_case(Path::new("simple_cases/function.py"); "function")]
|
||||
#[test_case(Path::new("simple_cases/function_trailing_comma.py"); "function_trailing_comma")]
|
||||
#[test_case(Path::new("simple_cases/composition.py"); "composition")]
|
||||
fn failing(path: &Path) -> Result<()> {
|
||||
let snapshot = format!("{}", path.display());
|
||||
let content = std::fs::read_to_string(test_resource_path(
|
||||
Path::new("fixtures/black").join(path).as_path(),
|
||||
))?;
|
||||
let formatted = fmt(&content)?;
|
||||
insta::assert_display_snapshot!(snapshot, formatted.print()?.as_code());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
@ -150,4 +176,41 @@ mod tests {
|
|||
}"#
|
||||
);
|
||||
}
|
||||
|
||||
struct Header<'a> {
|
||||
title: &'a str,
|
||||
}
|
||||
|
||||
impl<'a> Header<'a> {
|
||||
fn new(title: &'a str) -> Self {
|
||||
Self { title }
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Display for Header<'_> {
|
||||
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
|
||||
writeln!(f, "## {}", self.title)?;
|
||||
writeln!(f)
|
||||
}
|
||||
}
|
||||
|
||||
struct CodeFrame<'a> {
|
||||
language: &'a str,
|
||||
code: &'a str,
|
||||
}
|
||||
|
||||
impl<'a> CodeFrame<'a> {
|
||||
fn new(language: &'a str, code: &'a str) -> Self {
|
||||
Self { language, code }
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Display for CodeFrame<'_> {
|
||||
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
|
||||
writeln!(f, "```{}", self.language)?;
|
||||
writeln!(f, "{}", self.code)?;
|
||||
writeln!(f, "```")?;
|
||||
writeln!(f)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue