From 614a19cb4e18d5807673caf9e1c6d32ff5eca5d8 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Fri, 29 Sep 2023 10:31:54 -0500 Subject: [PATCH] Remove unused black compatibility tests (#7712) Previously attempted to repair these tests at https://github.com/astral-sh/ruff/pull/6992 but I don't think we should prioritize that and instead I would like to remove this dead code. --- .github/workflows/ci.yaml | 5 - .../tests/black_compatibility_test.rs | 204 ------------------ 2 files changed, 209 deletions(-) delete mode 100644 crates/ruff_cli/tests/black_compatibility_test.rs diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ef17f2df33..382488af3a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -96,7 +96,6 @@ jobs: uses: taiki-e/install-action@v2 with: tool: cargo-insta - - run: pip install black[d]==23.1.0 - uses: Swatinem/rust-cache@v2 - name: "Run tests (Ubuntu)" if: ${{ matrix.os == 'ubuntu-latest' }} @@ -106,10 +105,6 @@ jobs: shell: bash # We can't reject unreferenced snapshots on windows because flake8_executable can't run on windows run: cargo insta test --all --all-features - - run: cargo test --package ruff_cli --test black_compatibility_test -- --ignored - # TODO: Skipped as it's currently broken. The resource were moved from the - # ruff_cli to ruff crate, but this test was not updated. - if: false # Check for broken links in the documentation. - run: cargo doc --all --no-deps env: diff --git a/crates/ruff_cli/tests/black_compatibility_test.rs b/crates/ruff_cli/tests/black_compatibility_test.rs deleted file mode 100644 index 53159716e7..0000000000 --- a/crates/ruff_cli/tests/black_compatibility_test.rs +++ /dev/null @@ -1,204 +0,0 @@ -#![cfg(not(target_family = "wasm"))] - -use std::io::{ErrorKind, Read, Write}; -use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4, TcpListener, TcpStream}; -use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; -use std::thread::sleep; -use std::time::Duration; -use std::{fs, process, str}; - -use anyhow::{anyhow, bail, Context, Result}; -use insta_cmd::get_cargo_bin; -use log::info; -use walkdir::WalkDir; - -use ruff_linter::logging::{set_up_logging, LogLevel}; - -/// Handles `blackd` process and allows submitting code to it for formatting. -struct Blackd { - address: SocketAddr, - server: process::Child, - client: ureq::Agent, -} - -const BIN_NAME: &str = "ruff"; - -impl Blackd { - pub(crate) fn new() -> Result { - // Get free TCP port to run on - let address = TcpListener::bind(SocketAddrV4::new(Ipv4Addr::LOCALHOST, 0))?.local_addr()?; - - let args = [ - "--bind-host", - &address.ip().to_string(), - "--bind-port", - &address.port().to_string(), - ]; - let server = Command::new("blackd") - .args(args) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - .context("Starting blackd")?; - - // Wait up to four seconds for `blackd` to be ready. - for _ in 0..20 { - match TcpStream::connect(address) { - Err(e) if e.kind() == ErrorKind::ConnectionRefused => { - info!("`blackd` not ready yet; retrying..."); - sleep(Duration::from_millis(200)); - } - Err(e) => return Err(e.into()), - Ok(_) => { - info!("`blackd` ready"); - return Ok(Self { - address, - server, - client: ureq::agent(), - }); - } - } - } - - bail!("blackd {:?} failed to start", args) - } - - /// Format given code with blackd. - pub(crate) 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 mut step_1 = Command::new(get_cargo_bin(BIN_NAME)) - .args(ruff_args) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .spawn()?; - if let Some(mut stdin) = step_1.stdin.take() { - stdin.write_all(input.as_ref())?; - } - let step_1_output = step_1.wait_with_output()?; - if !step_1_output.status.success() { - return Err(anyhow!( - "Running input through ruff failed:\n{}", - str::from_utf8(&step_1_output.stderr)? - )); - } - - // Step 2: Run `blackd` on the input. - let step_2_output = blackd.check(&step_1_output.stdout.clone())?; - - // Step 3: Re-run `ruff` on the input. - let mut step_3 = Command::new(get_cargo_bin(BIN_NAME)) - .args(ruff_args) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .spawn()?; - if let Some(mut stdin) = step_3.stdin.take() { - stdin.write_all(step_2_output.as_ref())?; - } - let step_3_output = step_3.wait_with_output()?; - if !step_3_output.status.success() { - return Err(anyhow!( - "Running input through ruff after black failed:\n{}", - str::from_utf8(&step_3_output.stderr)? - )); - } - let step_3_output = step_3_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", "W605_1.py"]; - - let paths = WalkDir::new(fixtures_dir) - .into_iter() - .flatten() - .filter(|entry| { - entry - .path() - .extension() - .is_some_and(|ext| ext == "py" || ext == "pyi") - && !excludes.contains(&entry.path().file_name().unwrap().to_str().unwrap()) - }); - - let ruff_args = [ - "-", - "--silent", - "--exit-zero", - "--fix", - "--line-length", - "88", - "--select", - "ALL", - // 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. - "--ignore", - "RUF", - ]; - - for entry in paths { - let path = entry.path(); - run_test(path, &blackd, &ruff_args).context(format!("Testing {}", path.display()))?; - } - - Ok(()) -}