diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 42cde35764..5863256f95 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -115,7 +115,9 @@ jobs: ${{ runner.os }}-build-${{ env.cache-name }}- ${{ runner.os }}-build- ${{ runner.os }}- + - run: pip install black[d]==22.12.0 - run: cargo test --all + - run: cargo test --package ruff --test black_compatibility_test -- --ignored maturin_build: name: "maturin build" diff --git a/Cargo.lock b/Cargo.lock index 1835373563..bec2329217 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1895,6 +1895,7 @@ dependencies = [ "titlecase", "toml", "update-informer", + "ureq", "walkdir", ] diff --git a/Cargo.toml b/Cargo.toml index f63f1fd65d..cc9e5a7f8d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -71,6 +71,7 @@ assert_cmd = { version = "2.0.4" } criterion = { version = "0.4.0" } insta = { version = "1.19.1", features = ["yaml"] } test-case = { version = "2.2.2" } +ureq = { version = "2.5.0", features = [] } [features] default = ["update-informer"] diff --git a/tests/black_compatibility_test.rs b/tests/black_compatibility_test.rs new file mode 100644 index 0000000000..f666088808 --- /dev/null +++ b/tests/black_compatibility_test.rs @@ -0,0 +1,197 @@ +use std::io::{ErrorKind, Read}; +use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4, TcpListener, TcpStream}; +use std::path::{Path, PathBuf}; +use std::process::Stdio; +use std::thread::sleep; +use std::time::Duration; +use std::{fs, process, str}; + +use anyhow::{anyhow, Context, Result}; +use assert_cmd::{crate_name, Command}; +use itertools::Itertools; +use log::info; +use ruff::checks::CheckCategory; +use ruff::logging::{set_up_logging, LogLevel}; +use strum::IntoEnumIterator; +use walkdir::WalkDir; + +/// Handles blackd process and allows submitting code to it for formatting. +struct Blackd { + address: SocketAddr, + server: process::Child, + client: ureq::Agent, +} + +impl Blackd { + pub fn new() -> Result { + // Get free TCP port to run on + let address = TcpListener::bind(SocketAddrV4::new(Ipv4Addr::LOCALHOST, 0))?.local_addr()?; + + let server = process::Command::new("blackd") + .args([ + "--bind-host", + &address.ip().to_string(), + "--bind-port", + &address.port().to_string(), + ]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .context("Starting blackd")?; + + // Wait for `blackd` to be ready. + for _ in 0..10 { + match TcpStream::connect(address) { + Err(e) if e.kind() == ErrorKind::ConnectionRefused => { + info!("`blackd` not ready yet; retrying..."); + sleep(Duration::from_millis(100)); + } + Err(e) => return Err(e.into()), + Ok(_) => { + info!("`blackd` ready"); + break; + } + } + } + + Ok(Self { + address, + server, + client: ureq::agent(), + }) + } + + /// Format given code with blackd. + pub fn check(&self, code: &[u8]) -> Result> { + match self + .client + .post(&format!("http://{}/", self.address)) + .set("X-Line-Length", "88") + .send_bytes(code) + { + // 204 indicates the input wasn't changed during formatting, so + // we return the original. + Ok(response) => { + if response.status() == 204 { + Ok(code.to_vec()) + } else { + let mut buf = vec![]; + response + .into_reader() + .take((1024 * 1024) as u64) + .read_to_end(&mut buf)?; + Ok(buf) + } + } + Err(ureq::Error::Status(_, response)) => Err(anyhow::anyhow!( + "Formatting with `black` failed: {}", + response.into_string()? + )), + Err(e) => Err(e.into()), + } + } +} + +impl Drop for Blackd { + fn drop(&mut self) { + self.server.kill().expect("Couldn't end `blackd` process"); + } +} + +fn run_test(path: &Path, blackd: &Blackd, ruff_args: &[&str]) -> Result<()> { + let input = fs::read(path)?; + + // Step 1: Run `ruff` on the input. + let step_1 = &Command::cargo_bin(crate_name!())? + .args(ruff_args) + .write_stdin(input) + .assert() + .append_context("step", "running input through ruff"); + if !step_1.get_output().status.success() { + return Err(anyhow!( + "Running input through ruff failed:\n{}", + str::from_utf8(&step_1.get_output().stderr)? + )); + } + let step_1_output = step_1.get_output().stdout.clone(); + + // Step 2: Run `blackd` on the input. + let step_2_output = blackd.check(&step_1_output)?; + + // Step 3: Re-run `ruff` on the input. + let step_3 = &Command::cargo_bin(crate_name!())? + .args(ruff_args) + .write_stdin(step_2_output.clone()) + .assert(); + if !step_3.get_output().status.success() { + return Err(anyhow!( + "Running input through ruff after black failed:\n{}", + str::from_utf8(&step_3.get_output().stderr)? + )); + } + let step_3_output = step_3.get_output().stdout.clone(); + + assert_eq!( + str::from_utf8(&step_2_output), + str::from_utf8(&step_3_output), + "Mismatch found for {}", + path.display() + ); + + Ok(()) +} + +#[test] +#[ignore] +fn test_ruff_black_compatibility() -> Result<()> { + set_up_logging(&LogLevel::Default)?; + + let blackd = Blackd::new()?; + + let fixtures_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("resources") + .join("test") + .join("fixtures"); + + // Ignore some fixtures that currently trigger errors. `E999.py` especially, as + // that is triggering a syntax error on purpose. + let excludes = ["E999.py", "B009_B010.py", "W605_1.py", "leading_prefix.py"]; + + let paths: Vec = WalkDir::new(fixtures_dir) + .into_iter() + .filter(|entry| { + entry.as_ref().map_or(true, |entry| { + let file_name = entry.path().file_name().unwrap().to_string_lossy(); + + (file_name.ends_with(".py") || file_name.ends_with(".pyi")) + && !excludes.contains(&file_name.to_string().as_str()) + }) + }) + .filter_map(std::result::Result::ok) + .collect(); + + let codes = CheckCategory::iter() + // Exclude ruff codes, specifically RUF100, because it causes differences that are not a + // problem. Ruff would add a `# noqa: W292` after the first run, black introduces a + // newline, and ruff removes the `# noqa: W292` again. + .filter(|category| *category != CheckCategory::Ruff) + .map(|category| category.codes().iter().map(AsRef::as_ref).join(",")) + .join(","); + let ruff_args = [ + "-", + "--silent", + "--exit-zero", + "--fix", + "--line-length", + "88", + "--select", + &codes, + ]; + + for entry in paths { + let path = entry.path(); + run_test(path, &blackd, &ruff_args).context(format!("Testing {}", path.display()))?; + } + + Ok(()) +}