From 809c6d676f775d78eb0d37061204d2cde3fa9678 Mon Sep 17 00:00:00 2001 From: konsti Date: Thu, 1 Feb 2024 16:12:30 +0100 Subject: [PATCH] Use normalized display in tests and other small windows fixes (#1228) Split out from the large test refactoring PR. Use `normalized_display` in tests and two more thiserror derives to match snapshots and output, and other small windows fixes. --- crates/distribution-types/src/index_url.rs | 1 + crates/install-wheel-rs/src/lib.rs | 7 ++-- crates/puffin/src/commands/pip_compile.rs | 9 +++- crates/puffin/tests/pip_compile.rs | 48 +++++++++++++++------- crates/puffin/tests/pip_sync.rs | 23 +++++++---- crates/puffin/tests/venv.rs | 12 +++--- 6 files changed, 68 insertions(+), 32 deletions(-) diff --git a/crates/distribution-types/src/index_url.rs b/crates/distribution-types/src/index_url.rs index fa7786e15..a7d888b56 100644 --- a/crates/distribution-types/src/index_url.rs +++ b/crates/distribution-types/src/index_url.rs @@ -269,6 +269,7 @@ impl From for IndexUrls { } #[cfg(test)] +#[cfg(unix)] mod test { use super::*; diff --git a/crates/install-wheel-rs/src/lib.rs b/crates/install-wheel-rs/src/lib.rs index 80e3390c3..0336593c2 100644 --- a/crates/install-wheel-rs/src/lib.rs +++ b/crates/install-wheel-rs/src/lib.rs @@ -1,4 +1,4 @@ -//! Takes a wheel and installs it into a venv.. +//! Takes a wheel and installs it into a venv. use std::io; use std::io::{Read, Seek}; @@ -14,6 +14,7 @@ use distribution_filename::WheelFilename; pub use install_location::{normalize_name, InstallLocation, LockedDir}; use pep440_rs::Version; use platform_host::{Arch, Os}; +use puffin_fs::NormalizedDisplay; use puffin_normalize::PackageName; pub use record::RecordEntry; pub use script::Script; @@ -37,7 +38,7 @@ pub enum Error { #[error(transparent)] Io(#[from] io::Error), /// Custom error type to add a path to error reading a file from a zip - #[error("Failed to reflink {from} to {to}")] + #[error("Failed to reflink {} to {}", from.normalized_display(), to.normalized_display())] Reflink { from: PathBuf, to: PathBuf, @@ -78,7 +79,7 @@ pub enum Error { DirectUrlJson(#[from] serde_json::Error), #[error("No .dist-info directory found")] MissingDistInfo, - #[error("Cannot uninstall package; RECORD file not found at: {0}")] + #[error("Cannot uninstall package; RECORD file not found at: {}", _0.normalized_display())] MissingRecord(PathBuf), #[error("Multiple .dist-info directories found: {0}")] MultipleDistInfo(String), diff --git a/crates/puffin/src/commands/pip_compile.rs b/crates/puffin/src/commands/pip_compile.rs index ccf8608a1..f583c2a0d 100644 --- a/crates/puffin/src/commands/pip_compile.rs +++ b/crates/puffin/src/commands/pip_compile.rs @@ -353,7 +353,14 @@ pub(crate) async fn pip_compile( writeln!( writer, "{}", - format!("# puffin {}", env::args().skip(1).join(" ")).green() + format!( + "# puffin {}", + env::args_os() + .skip(1) + .map(|arg| arg.normalized_display().to_string()) + .join(" ") + ) + .green() )?; } diff --git a/crates/puffin/tests/pip_compile.rs b/crates/puffin/tests/pip_compile.rs index df1aee6d1..418cd0e30 100644 --- a/crates/puffin/tests/pip_compile.rs +++ b/crates/puffin/tests/pip_compile.rs @@ -1,8 +1,8 @@ #![cfg(all(feature = "python", feature = "pypi"))] +use std::fs; use std::path::PathBuf; use std::process::Command; -use std::{fs, iter}; use anyhow::{bail, Context, Result}; use assert_fs::prelude::*; @@ -15,6 +15,7 @@ use itertools::Itertools; use url::Url; use common::{puffin_snapshot, TestContext, BIN_NAME, INSTA_FILTERS}; +use puffin_fs::NormalizedDisplay; use crate::common::EXCLUDE_NEWER; @@ -683,7 +684,8 @@ fn compile_git_https_dependency() -> Result<()> { requirements_in.write_str("flask @ git+https://github.com/pallets/flask.git")?; // In addition to the standard filters, remove the `main` commit, which will change frequently. - let filters: Vec<_> = iter::once((r"@(\d|\w){40}", "@[COMMIT]")) + let filters: Vec<_> = [(r"@(\d|\w){40}", "@[COMMIT]")] + .into_iter() .chain(INSTA_FILTERS.to_vec()) .collect(); @@ -1501,7 +1503,8 @@ fn compile_wheel_path_dependency() -> Result<()> { ))?; // In addition to the standard filters, remove the temporary directory from the snapshot. - let filters: Vec<_> = iter::once((r"file://.*/", "file://[TEMP_DIR]/")) + let filters: Vec<_> = [(r"file://.*/", "file://[TEMP_DIR]/")] + .into_iter() .chain(INSTA_FILTERS.to_vec()) .collect(); @@ -1648,7 +1651,8 @@ fn compile_source_distribution_path_dependency() -> Result<()> { ))?; // In addition to the standard filters, remove the temporary directory from the snapshot. - let filters: Vec<_> = iter::once((r"file://.*/", "file://[TEMP_DIR]/")) + let filters: Vec<_> = [(r"file://.*/", "file://[TEMP_DIR]/")] + .into_iter() .chain(INSTA_FILTERS.to_vec()) .collect(); @@ -1690,7 +1694,8 @@ fn compile_wheel_path_dependency_missing() -> Result<()> { requirements_in.write_str("flask @ file:///path/to/flask-3.0.0-py3-none-any.whl")?; // In addition to the standard filters, remove the temporary directory from the snapshot. - let filters: Vec<_> = iter::once((r"file://.*/", "file://[TEMP_DIR]/")) + let filters: Vec<_> = [(r"file://.*/", "file://[TEMP_DIR]/")] + .into_iter() .chain(INSTA_FILTERS.to_vec()) .collect(); @@ -2014,8 +2019,9 @@ fn compile_editable() -> Result<()> { " })?; - let filter_path = regex::escape(&requirements_in.display().to_string()); - let filters: Vec<_> = iter::once((filter_path.as_str(), "requirements.in")) + let filter_path = regex::escape(&requirements_in.normalized_display().to_string()); + let filters: Vec<_> = [(filter_path.as_str(), "requirements.in")] + .into_iter() .chain(INSTA_FILTERS.to_vec()) .collect(); @@ -2125,7 +2131,7 @@ fn cache_errors_are_non_fatal() -> Result<()> { for file in &cache_files { let file = context.cache_dir.join(file); if !file.is_file() { - bail!("Missing cache file {}", file.display()); + bail!("Missing cache file {}", file.normalized_display()); } fs_err::write(file, "I borken you cache")?; } @@ -2140,7 +2146,7 @@ fn cache_errors_are_non_fatal() -> Result<()> { for file in cache_files { let file = context.cache_dir.join(file); if !file.is_file() { - bail!("Missing cache file {}", file.display()); + bail!("Missing cache file {}", file.normalized_display()); } fs_err::OpenOptions::new() @@ -2421,10 +2427,18 @@ fn find_links_directory() -> Result<()> { "})?; let project_root = fs_err::canonicalize(std::env::current_dir()?.join("../.."))?; - let project_root_string = regex::escape(&project_root.display().to_string()); - let filters: Vec<_> = iter::once((project_root_string.as_str(), "[PROJECT_ROOT]")) - .chain(INSTA_FILTERS.to_vec()) - .collect(); + let project_root_string = regex::escape(&project_root.normalized_display().to_string()); + let filters: Vec<_> = [ + (project_root_string.as_str(), "[PROJECT_ROOT]"), + // Unify trailing (back)slash between Windows and Unix. + ( + "[PROJECT_ROOT]/scripts/wheels/", + "[PROJECT_ROOT]/scripts/wheels", + ), + ] + .into_iter() + .chain(INSTA_FILTERS.to_vec()) + .collect(); puffin_snapshot!(filters, context.compile() .arg("requirements.in") @@ -2691,9 +2705,13 @@ fn upgrade_package() -> Result<()> { fn missing_path_requirement() -> Result<()> { let context = TestContext::new("3.12"); let requirements_in = context.temp_dir.child("requirements.in"); - requirements_in.write_str("django @ file:///tmp/django-3.2.8.tar.gz")?; + requirements_in.write_str(if cfg!(windows) { + "django @ file://C:/tmp/django-3.2.8.tar.gz" + } else { + "django @ file:///tmp/django-3.2.8.tar.gz" + })?; - let filters: Vec<_> = [(r"/[A-Z]:/", "/")] + let filters: Vec<_> = [(r"/C:/", "/")] .into_iter() .chain(INSTA_FILTERS.to_vec()) .collect(); diff --git a/crates/puffin/tests/pip_sync.rs b/crates/puffin/tests/pip_sync.rs index 380f4f511..08249cc32 100644 --- a/crates/puffin/tests/pip_sync.rs +++ b/crates/puffin/tests/pip_sync.rs @@ -13,6 +13,7 @@ use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; use url::Url; use common::{create_venv, venv_to_interpreter, BIN_NAME, INSTA_FILTERS}; +use puffin_fs::NormalizedDisplay; mod common; @@ -2726,10 +2727,10 @@ fn sync_editable() -> Result<()> { # via poetry-editable -e file://{current_dir}/../../scripts/editable-installs/poetry_editable ", - current_dir = current_dir.display(), + current_dir = current_dir.normalized_display(), })?; - let filter_path = regex::escape(&requirements_txt.display().to_string()); + let filter_path = regex::escape(&requirements_txt.normalized_display().to_string()); let filters = INSTA_FILTERS .iter() .chain(&[ @@ -2885,7 +2886,7 @@ fn sync_editable_and_registry() -> Result<()> { " })?; - let filter_path = regex::escape(&requirements_txt.display().to_string()); + let filter_path = regex::escape(&requirements_txt.normalized_display().to_string()); let filters = INSTA_FILTERS .iter() .chain(&[ @@ -2931,7 +2932,7 @@ fn sync_editable_and_registry() -> Result<()> { " })?; - let filter_path = requirements_txt.display().to_string(); + let filter_path = regex::escape(&requirements_txt.normalized_display().to_string()); let filters = INSTA_FILTERS .iter() .chain(&[ @@ -2973,7 +2974,7 @@ fn sync_editable_and_registry() -> Result<()> { " })?; - let filter_path = requirements_txt.display().to_string(); + let filter_path = requirements_txt.normalized_display().to_string(); let filters = INSTA_FILTERS .iter() .chain(&[ @@ -3010,7 +3011,7 @@ fn sync_editable_and_registry() -> Result<()> { " })?; - let filter_path = requirements_txt.display().to_string(); + let filter_path = requirements_txt.normalized_display().to_string(); let filters = INSTA_FILTERS .iter() .chain(&[ @@ -3069,7 +3070,13 @@ fn incompatible_wheel() -> Result<()> { Url::from_file_path(wheel.path()).unwrap() ))?; - let wheel_dir = regex::escape(&wheel_dir.path().canonicalize()?.display().to_string()); + let wheel_dir = regex::escape( + &wheel_dir + .path() + .canonicalize()? + .normalized_display() + .to_string(), + ); let filters: Vec<_> = iter::once((wheel_dir.as_str(), "[TEMP_DIR]")) .chain(INSTA_FILTERS.to_vec()) .collect(); @@ -3188,7 +3195,7 @@ fn find_links() -> Result<()> { "})?; let project_root = fs_err::canonicalize(std::env::current_dir()?.join("../.."))?; - let project_root_string = regex::escape(&project_root.display().to_string()); + let project_root_string = regex::escape(&project_root.normalized_display().to_string()); let filters: Vec<_> = iter::once((project_root_string.as_str(), "[PROJECT_ROOT]")) .chain(INSTA_FILTERS.to_vec()) .collect(); diff --git a/crates/puffin/tests/venv.rs b/crates/puffin/tests/venv.rs index 3a7ab6a12..d31ec4a34 100644 --- a/crates/puffin/tests/venv.rs +++ b/crates/puffin/tests/venv.rs @@ -7,6 +7,8 @@ use assert_fs::prelude::*; use insta_cmd::_macro_support::insta; use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; +use puffin_fs::NormalizedDisplay; + use common::BIN_NAME; mod common; @@ -17,7 +19,7 @@ fn create_venv() -> Result<()> { let cache_dir = assert_fs::TempDir::new()?; let venv = temp_dir.child(".venv"); - let filter_venv = regex::escape(&venv.display().to_string()); + let filter_venv = regex::escape(&venv.normalized_display().to_string()); insta::with_settings!({ filters => vec![ (r"Using Python 3\.\d+\.\d+ interpreter at .+", "Using Python [VERSION] interpreter at [PATH]"), @@ -53,7 +55,7 @@ fn create_venv_defaults_to_cwd() -> Result<()> { let cache_dir = assert_fs::TempDir::new()?; let venv = temp_dir.child(".venv"); - let filter_venv = regex::escape(&venv.display().to_string()); + let filter_venv = regex::escape(&venv.normalized_display().to_string()); insta::with_settings!({ filters => vec![ (r"Using Python 3\.\d+\.\d+ interpreter at .+", "Using Python [VERSION] interpreter at [PATH]"), @@ -88,7 +90,7 @@ fn seed() -> Result<()> { let cache_dir = assert_fs::TempDir::new()?; let venv = temp_dir.child(".venv"); - let filter_venv = regex::escape(&venv.display().to_string()); + let filter_venv = regex::escape(&venv.normalized_display().to_string()); insta::with_settings!({ filters => vec![ (r"Using Python 3\.\d+\.\d+ interpreter at .+", "Using Python [VERSION] interpreter at [PATH]"), @@ -163,7 +165,7 @@ fn create_venv_unknown_python_patch() -> Result<()> { let cache_dir = assert_fs::TempDir::new()?; let venv = temp_dir.child(".venv"); - let filter_venv = regex::escape(&venv.display().to_string()); + let filter_venv = regex::escape(&venv.normalized_display().to_string()); insta::with_settings!({ filters => vec![ (r"Using Python 3\.\d+\.\d+ interpreter at .+", "Using Python [VERSION] interpreter at [PATH]"), @@ -198,7 +200,7 @@ fn create_venv_python_patch() -> Result<()> { let cache_dir = assert_fs::TempDir::new()?; let venv = temp_dir.child(".venv"); - let filter_venv = regex::escape(&venv.display().to_string()); + let filter_venv = regex::escape(&venv.normalized_display().to_string()); insta::with_settings!({ filters => vec![ (r"interpreter at .+", "interpreter at [PATH]"),