From 60638c2902886498a6dc28f3b4ca8ca7d93a9947 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 4 Jan 2025 15:39:20 -0800 Subject: [PATCH 01/10] Defer to `cargo metadata` to evaluate the workspace root (#660) Not a big deal, but I think we can just defer --- cargo-insta/src/cli.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 62c92fc1..c91bf395 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -413,9 +413,6 @@ fn handle_target_args<'a>( ) -> Result, Box> { let mut cmd = cargo_metadata::MetadataCommand::new(); - // if a workspace root is provided we first check if it points to a - // `Cargo.toml`. If it does we instead treat it as manifest path. If both - // are provided we fail with an error. match ( target_args.workspace_root.as_deref(), target_args.manifest_path.as_deref(), @@ -429,14 +426,7 @@ fn handle_target_args<'a>( cmd.manifest_path(manifest); } (Some(root), None) => { - // TODO: should we do this ourselves? Probably fine, but are we - // adding anything by not just deferring to cargo? - let assumed_manifest = root.join("Cargo.toml"); - if assumed_manifest.is_file() { - cmd.manifest_path(assumed_manifest); - } else { - cmd.current_dir(root); - } + cmd.current_dir(root); } (None, None) => {} }; From f750c5b62ba61459758a930ea9bd00affba8bd09 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 4 Jan 2025 15:43:26 -0800 Subject: [PATCH 02/10] Update `dist` config (#700) Ignore shellcheck error --- .github/workflows/release.yml | 133 ++++++++++++++++++++-------------- .pre-commit-config.yaml | 2 + Cargo.toml | 14 +++- insta/src/runtime.rs | 2 +- 4 files changed, 94 insertions(+), 57 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 502a5ff8..1dffbcb3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,20 +1,21 @@ -# Copyright 2022-2023, axodotdev +# This file was autogenerated by dist: https://opensource.axo.dev/cargo-dist/ +# +# Copyright 2022-2024, axodotdev # SPDX-License-Identifier: MIT or Apache-2.0 # # CI that: # # * checks for a Git Tag that looks like a release -# * builds artifacts with cargo-dist (archives, installers, hashes) +# * builds artifacts with dist (archives, installers, hashes) # * uploads those artifacts to temporary workflow zip -# * on success, uploads the artifacts to a Github Release +# * on success, uploads the artifacts to a GitHub Release # -# Note that the Github Release will be created with a generated +# Note that the GitHub Release will be created with a generated # title/body based on your changelogs. name: Release - permissions: - contents: write + "contents": "write" # This task will run whenever you push a git tag that looks like a version # like "1.0.0", "v0.1.0-prerelease.1", "my-app/0.1.0", "releases/v1.0.0", etc. @@ -23,30 +24,30 @@ permissions: # must be a Cargo-style SemVer Version (must have at least major.minor.patch). # # If PACKAGE_NAME is specified, then the announcement will be for that -# package (erroring out if it doesn't have the given version or isn't cargo-dist-able). +# package (erroring out if it doesn't have the given version or isn't dist-able). # # If PACKAGE_NAME isn't specified, then the announcement will be for all -# (cargo-dist-able) packages in the workspace with that version (this mode is +# (dist-able) packages in the workspace with that version (this mode is # intended for workspaces with only one dist-able package, or with all dist-able # packages versioned/released in lockstep). # # If you push multiple tags at once, separate instances of this workflow will -# spin up, creating an independent announcement for each one. However Github +# spin up, creating an independent announcement for each one. However, GitHub # will hard limit this to 3 tags per commit, as it will assume more tags is a # mistake. # # If there's a prerelease-style suffix to the version, then the release(s) # will be marked as a prerelease. on: + pull_request: push: tags: - '**[0-9]+.[0-9]+.[0-9]+*' - pull_request: jobs: - # Run 'cargo dist plan' (or host) to determine what tasks we need to do + # Run 'dist plan' (or host) to determine what tasks we need to do plan: - runs-on: ubuntu-latest + runs-on: "ubuntu-20.04" outputs: val: ${{ steps.plan.outputs.manifest }} tag: ${{ !github.event.pull_request && github.ref_name || '' }} @@ -58,11 +59,16 @@ jobs: - uses: actions/checkout@v4 with: submodules: recursive - - name: Install cargo-dist + - name: Install dist # we specify bash to get pipefail; it guards against the `curl` command # failing. otherwise `sh` won't catch that `curl` returned non-0 shell: bash - run: "curl --proto '=https' --tlsv1.2 -LsSf https://github.com/axodotdev/cargo-dist/releases/download/v0.12.0/cargo-dist-installer.sh | sh" + run: "curl --proto '=https' --tlsv1.2 -LsSf https://github.com/axodotdev/cargo-dist/releases/download/v0.27.0/cargo-dist-installer.sh | sh" + - name: Cache dist + uses: actions/upload-artifact@v4 + with: + name: cargo-dist-cache + path: ~/.cargo/bin/dist # sure would be cool if github gave us proper conditionals... # so here's a doubly-nested ternary-via-truthiness to try to provide the best possible # functionality based on whether this is a pull_request, and whether it's from a fork. @@ -70,8 +76,8 @@ jobs: # but also really annoying to build CI around when it needs secrets to work right.) - id: plan run: | - cargo dist ${{ (!github.event.pull_request && format('host --steps=create --tag={0}', github.ref_name)) || 'plan' }} --output-format=json > plan-dist-manifest.json - echo "cargo dist ran successfully" + dist ${{ (!github.event.pull_request && format('host --steps=create --tag={0}', github.ref_name)) || 'plan' }} --output-format=json > plan-dist-manifest.json + echo "dist ran successfully" cat plan-dist-manifest.json echo "manifest=$(jq -c "." plan-dist-manifest.json)" >> "$GITHUB_OUTPUT" - name: "Upload dist-manifest.json" @@ -89,28 +95,38 @@ jobs: if: ${{ fromJson(needs.plan.outputs.val).ci.github.artifacts_matrix.include != null && (needs.plan.outputs.publishing == 'true' || fromJson(needs.plan.outputs.val).ci.github.pr_run_mode == 'upload') }} strategy: fail-fast: false - # Target platforms/runners are computed by cargo-dist in create-release. + # Target platforms/runners are computed by dist in create-release. # Each member of the matrix has the following arguments: # # - runner: the github runner - # - dist-args: cli flags to pass to cargo dist - # - install-dist: expression to run to install cargo-dist on the runner + # - dist-args: cli flags to pass to dist + # - install-dist: expression to run to install dist on the runner # # Typically there will be: # - 1 "global" task that builds universal installers # - N "local" tasks that build each platform's binaries and platform-specific installers matrix: ${{ fromJson(needs.plan.outputs.val).ci.github.artifacts_matrix }} runs-on: ${{ matrix.runner }} + container: ${{ matrix.container && matrix.container.image || null }} env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} BUILD_MANIFEST_NAME: target/distrib/${{ join(matrix.targets, '-') }}-dist-manifest.json steps: + - name: enable windows longpaths + run: | + git config --global core.longpaths true - uses: actions/checkout@v4 with: submodules: recursive - - uses: swatinem/rust-cache@v2 - - name: Install cargo-dist - run: ${{ matrix.install_dist }} + - name: Install Rust non-interactively if not already installed + if: ${{ matrix.container }} + run: | + if ! command -v cargo > /dev/null 2>&1; then + curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y + echo "$HOME/.cargo/bin" >> $GITHUB_PATH + fi + - name: Install dist + run: ${{ matrix.install_dist.run }} # Get the dist-manifest - name: Fetch local artifacts uses: actions/download-artifact@v4 @@ -124,8 +140,8 @@ jobs: - name: Build artifacts run: | # Actually do builds and make zips and whatnot - cargo dist build ${{ needs.plan.outputs.tag-flag }} --print=linkage --output-format=json ${{ matrix.dist_args }} > dist-manifest.json - echo "cargo dist ran successfully" + dist build ${{ needs.plan.outputs.tag-flag }} --print=linkage --output-format=json ${{ matrix.dist_args }} > dist-manifest.json + echo "dist ran successfully" - id: cargo-dist name: Post-build # We force bash here just because github makes it really hard to get values up @@ -135,7 +151,7 @@ jobs: run: | # Parse out what we just built and upload it to scratch storage echo "paths<> "$GITHUB_OUTPUT" - jq --raw-output ".artifacts[]?.path | select( . != null )" dist-manifest.json >> "$GITHUB_OUTPUT" + dist print-upload-files-from-manifest --manifest dist-manifest.json >> "$GITHUB_OUTPUT" echo "EOF" >> "$GITHUB_OUTPUT" cp dist-manifest.json "$BUILD_MANIFEST_NAME" @@ -160,9 +176,12 @@ jobs: - uses: actions/checkout@v4 with: submodules: recursive - - name: Install cargo-dist - shell: bash - run: "curl --proto '=https' --tlsv1.2 -LsSf https://github.com/axodotdev/cargo-dist/releases/download/v0.12.0/cargo-dist-installer.sh | sh" + - name: Install cached dist + uses: actions/download-artifact@v4 + with: + name: cargo-dist-cache + path: ~/.cargo/bin/ + - run: chmod +x ~/.cargo/bin/dist # Get all the local artifacts for the global tasks to use (for e.g. checksums) - name: Fetch local artifacts uses: actions/download-artifact@v4 @@ -173,12 +192,12 @@ jobs: - id: cargo-dist shell: bash run: | - cargo dist build ${{ needs.plan.outputs.tag-flag }} --output-format=json "--artifacts=global" > dist-manifest.json - echo "cargo dist ran successfully" + dist build ${{ needs.plan.outputs.tag-flag }} --output-format=json "--artifacts=global" > dist-manifest.json + echo "dist ran successfully" # Parse out what we just built and upload it to scratch storage echo "paths<> "$GITHUB_OUTPUT" - jq --raw-output ".artifacts[]?.path | select( . != null )" dist-manifest.json >> "$GITHUB_OUTPUT" + jq --raw-output ".upload_files[]" dist-manifest.json >> "$GITHUB_OUTPUT" echo "EOF" >> "$GITHUB_OUTPUT" cp dist-manifest.json "$BUILD_MANIFEST_NAME" @@ -206,8 +225,12 @@ jobs: - uses: actions/checkout@v4 with: submodules: recursive - - name: Install cargo-dist - run: "curl --proto '=https' --tlsv1.2 -LsSf https://github.com/axodotdev/cargo-dist/releases/download/v0.12.0/cargo-dist-installer.sh | sh" + - name: Install cached dist + uses: actions/download-artifact@v4 + with: + name: cargo-dist-cache + path: ~/.cargo/bin/ + - run: chmod +x ~/.cargo/bin/dist # Fetch artifacts from scratch-storage - name: Fetch artifacts uses: actions/download-artifact@v4 @@ -215,11 +238,10 @@ jobs: pattern: artifacts-* path: target/distrib/ merge-multiple: true - # This is a harmless no-op for Github Releases, hosting for that happens in "announce" - id: host shell: bash run: | - cargo dist host ${{ needs.plan.outputs.tag-flag }} --steps=upload --steps=release --output-format=json > dist-manifest.json + dist host ${{ needs.plan.outputs.tag-flag }} --steps=upload --steps=release --output-format=json > dist-manifest.json echo "artifacts uploaded and released successfully" cat dist-manifest.json echo "manifest=$(jq -c "." dist-manifest.json)" >> "$GITHUB_OUTPUT" @@ -229,8 +251,29 @@ jobs: # Overwrite the previous copy name: artifacts-dist-manifest path: dist-manifest.json + # Create a GitHub Release while uploading all files to it + - name: "Download GitHub Artifacts" + uses: actions/download-artifact@v4 + with: + pattern: artifacts-* + path: artifacts + merge-multiple: true + - name: Cleanup + run: | + # Remove the granular manifests + rm -f artifacts/*-dist-manifest.json + - name: Create GitHub Release + env: + PRERELEASE_FLAG: "${{ fromJson(steps.host.outputs.manifest).announcement_is_prerelease && '--prerelease' || '' }}" + ANNOUNCEMENT_TITLE: "${{ fromJson(steps.host.outputs.manifest).announcement_title }}" + ANNOUNCEMENT_BODY: "${{ fromJson(steps.host.outputs.manifest).announcement_github_body }}" + RELEASE_COMMIT: "${{ github.sha }}" + run: | + # Write and read notes from a file to avoid quoting breaking things + echo "$ANNOUNCEMENT_BODY" > $RUNNER_TEMP/notes.txt + + gh release create "${{ needs.plan.outputs.tag }}" --target "$RELEASE_COMMIT" $PRERELEASE_FLAG --title "$ANNOUNCEMENT_TITLE" --notes-file "$RUNNER_TEMP/notes.txt" artifacts/* - # Create a Github Release while uploading all files to it announce: needs: - plan @@ -246,21 +289,3 @@ jobs: - uses: actions/checkout@v4 with: submodules: recursive - - name: "Download Github Artifacts" - uses: actions/download-artifact@v4 - with: - pattern: artifacts-* - path: artifacts - merge-multiple: true - - name: Cleanup - run: | - # Remove the granular manifests - rm -f artifacts/*-dist-manifest.json - - name: Create Github Release - uses: ncipollo/release-action@v1 - with: - tag: ${{ needs.plan.outputs.tag }} - name: ${{ fromJson(needs.host.outputs.val).announcement_title }} - body: ${{ fromJson(needs.host.outputs.val).announcement_github_body }} - prerelease: ${{ fromJson(needs.host.outputs.val).announcement_is_prerelease }} - artifacts: "artifacts/*" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 51d044b2..9b5dd254 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -16,6 +16,8 @@ repos: rev: v1.6.27 hooks: - id: actionlint + # auto-generated with `dist` + exclude: .github/workflows/release.yml - repo: https://github.com/tcort/markdown-link-check rev: v3.12.1 hooks: diff --git a/Cargo.toml b/Cargo.toml index 50d664d6..42be8c0e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,13 +2,21 @@ members = ["cargo-insta"] resolver = "2" +# Config for 'dist' [workspace.metadata.dist] -cargo-dist-version = "0.12.0" -ci = ["github"] +# The preferred dist version to use in CI (Cargo.toml SemVer syntax) +cargo-dist-version = "0.27.0" +# CI backends to support +ci = "github" +# Whether to install an updater program install-updater = false +# The installers to generate for each app installers = ["shell", "powershell"] +# Which actions to run on pull requests pr-run-mode = "plan" +# Build only the required packages, and individually precise-builds = true +# Target platforms to build apps for (Rust target-triple syntax) targets = [ "aarch64-apple-darwin", "x86_64-apple-darwin", @@ -16,6 +24,8 @@ targets = [ "x86_64-unknown-linux-musl", "x86_64-pc-windows-msvc", ] +# Path that installers should place binaries in +install-path = "CARGO_HOME" [profile.dist] inherits = "release" diff --git a/insta/src/runtime.rs b/insta/src/runtime.rs index 0a4152b8..f2a0ff18 100644 --- a/insta/src/runtime.rs +++ b/insta/src/runtime.rs @@ -529,7 +529,7 @@ impl<'a> SnapshotAssertionContext<'a> { // use `NewFile`, since we can't use `InPlace` for inline. `cargo-insta` // then accepts all snapshots at the end of the test. let snapshot_update = - // TOOD: could match on the snapshot kind instead of whether snapshot_file is None + // TODO: could match on the snapshot kind instead of whether snapshot_file is None if snapshot_update == SnapshotUpdateBehavior::InPlace && self.snapshot_file.is_none() { SnapshotUpdateBehavior::NewFile } else { From 3f1e9a1e26a78808e17afb85d9f034a226e9b470 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 4 Jan 2025 16:10:06 -0800 Subject: [PATCH 03/10] Fix clippy error & run `--all-features` in CI (#701) --- Makefile | 2 +- insta/src/glob.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 64e647aa..0cb26419 100644 --- a/Makefile +++ b/Makefile @@ -39,6 +39,6 @@ format-check: lint: @rustup component add clippy 2> /dev/null - @cargo clippy --all-targets --workspace -- --deny warnings + @cargo clippy --all-targets --all-features -- --deny warnings .PHONY: all doc test cargotest format format-check lint update-readme diff --git a/insta/src/glob.rs b/insta/src/glob.rs index 2b587b87..15a9e89f 100644 --- a/insta/src/glob.rs +++ b/insta/src/glob.rs @@ -18,7 +18,7 @@ pub(crate) struct GlobCollector { /// the glob stack holds failure count and an indication if `cargo insta review` /// should be run. -pub(crate) static GLOB_STACK: Lazy>> = Lazy::new(|| Mutex::default()); +pub(crate) static GLOB_STACK: Lazy>> = Lazy::new(Mutex::default); static GLOB_FILTER: Lazy> = Lazy::new(|| { env::var("INSTA_GLOB_FILTER") From aca7fe4d040b412686e6952d834576a16417bd90 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 4 Jan 2025 16:17:02 -0800 Subject: [PATCH 04/10] Update changelog for 1.42; add `snapshot_kind` explanation (#702) --- CHANGELOG.md | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4305e18c..80ba30ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,16 +2,25 @@ All notable changes to insta and cargo-insta are documented here. -## [Unreleased] - +## 1.42.0 + +- Text snapshots no longer contain `snapshot_type: text` in their metadata. For + context, we originally added this in the prior release (1.41.0) to support + binary snapshots, but some folks disliked the diff noise on any snapshot + changes, and the maintainers' weighted votes favored reverting. I apologize + that this will cause some additional churn for those who used `cargo test + --force-update-snapshots` to update their snapshots to the 1.41 format; + running this again with 1.42 will remove those metadata entries. To confirm: + this doesn't affect whether snapshot tests pass or fail — the worst impact is + some additional diffs in metadata. #690 - Pending snapshots are no longer removed throughout the workspace by `cargo-insta` before running tests. Instead, running a test will overwrite or remove its own pending snapshot. To remove all pending snapshots, use `cargo insta reject` or run tests with `--unreferenced=delete`. #651 - `insta::internals::SettingsBindDropGuard` (returned from - `Settings::bind_to_scope`) no longer implements `Send`. This was an error and - any tests relying on this behavior where not working properly. - Fixes #694 in #695 by @jalil-salame + `Settings::bind_to_scope`) no longer implements `Send`. This was incorrect and + any tests relying on this behavior where not working properly. Fixes #694 in + #695 by @jalil-salame ## 1.41.1 From c20f566d29d6ca1b1b5a852066b0d5ef5ab36ce5 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 4 Jan 2025 16:21:57 -0800 Subject: [PATCH 05/10] Bump version to 1.42.0 (#703) --- Cargo.lock | 4 ++-- cargo-insta/Cargo.toml | 4 ++-- insta/Cargo.toml | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 28359256..b7a458f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -66,7 +66,7 @@ dependencies = [ [[package]] name = "cargo-insta" -version = "1.41.1" +version = "1.42.0" dependencies = [ "cargo_metadata", "clap", @@ -353,7 +353,7 @@ dependencies = [ [[package]] name = "insta" -version = "1.41.1" +version = "1.42.0" dependencies = [ "clap", "console", diff --git a/cargo-insta/Cargo.toml b/cargo-insta/Cargo.toml index 8b77fa63..ac5bd923 100644 --- a/cargo-insta/Cargo.toml +++ b/cargo-insta/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo-insta" -version = "1.41.1" +version = "1.42.0" license = "Apache-2.0" authors = ["Armin Ronacher "] description = "A review tool for the insta snapshot testing library for Rust" @@ -14,7 +14,7 @@ readme = "README.md" rust-version = "1.65.0" [dependencies] -insta = { version = "=1.41.1", path = "../insta", features = [ +insta = { version = "=1.42.0", path = "../insta", features = [ "json", "yaml", "redactions", diff --git a/insta/Cargo.toml b/insta/Cargo.toml index e18e11a5..ba312bb3 100644 --- a/insta/Cargo.toml +++ b/insta/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "insta" -version = "1.41.1" +version = "1.42.0" license = "Apache-2.0" authors = ["Armin Ronacher "] description = "A snapshot testing library for Rust" From 1da675c5a5c3d7d08e2e316f5eeb5b2134341498 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Mon, 6 Jan 2025 17:27:23 -0800 Subject: [PATCH 06/10] Fix command in changelog (#705) Closes #704 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80ba30ec..88b25c83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ All notable changes to insta and cargo-insta are documented here. context, we originally added this in the prior release (1.41.0) to support binary snapshots, but some folks disliked the diff noise on any snapshot changes, and the maintainers' weighted votes favored reverting. I apologize - that this will cause some additional churn for those who used `cargo test + that this will cause some additional churn for those who used `cargo insta test --force-update-snapshots` to update their snapshots to the 1.41 format; running this again with 1.42 will remove those metadata entries. To confirm: this doesn't affect whether snapshot tests pass or fail — the worst impact is From 949ac940aec545dad5d971e83abe44d11e35982d Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Mon, 6 Jan 2025 17:37:48 -0800 Subject: [PATCH 07/10] Update test output for `snapshot_text` change (#706) --- cargo-insta/tests/functional/main.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cargo-insta/tests/functional/main.rs b/cargo-insta/tests/functional/main.rs index ccb43206..39de30ef 100644 --- a/cargo-insta/tests/functional/main.rs +++ b/cargo-insta/tests/functional/main.rs @@ -375,13 +375,12 @@ Hello, world! assert_snapshot!(test_insta_1_40_0.diff("src/snapshots/test_force_update_1_40_0__force_update.snap"), @r#" --- Original: src/snapshots/test_force_update_1_40_0__force_update.snap +++ Updated: src/snapshots/test_force_update_1_40_0__force_update.snap - @@ -1,8 +1,6 @@ + @@ -1,8 +1,5 @@ - --- source: src/lib.rs -expression: +expression: "\"Hello, world!\"" - +snapshot_kind: text --- Hello, world! - From f778d2964f011e91f95767718563eb7169b2320f Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 23 Jan 2025 11:45:12 -0800 Subject: [PATCH 08/10] Replace use of unsafe with pin_project (#711) --- Cargo.lock | 21 +++++++++++++++++++++ insta/Cargo.toml | 1 + insta/src/settings.rs | 5 +++-- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b7a458f6..292b53a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -363,6 +363,7 @@ dependencies = [ "once_cell", "pest", "pest_derive", + "pin-project", "regex", "ron", "rustc_version", @@ -561,6 +562,26 @@ dependencies = [ "sha2", ] +[[package]] +name = "pin-project" +version = "1.1.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e2ec53ad785f4d35dac0adea7f7dc6f1bb277ad84a680c7afefeae05d1f5916" +dependencies = [ + "pin-project-internal", +] + +[[package]] +name = "pin-project-internal" +version = "1.1.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d56a66c0c55993aa927429d0f8a0abfd74f084e4d9c192cffed01e418d83eefb" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "proc-macro2" version = "1.0.86" diff --git a/insta/Cargo.toml b/insta/Cargo.toml index ba312bb3..747a1ab3 100644 --- a/insta/Cargo.toml +++ b/insta/Cargo.toml @@ -63,6 +63,7 @@ once_cell = "1.20.2" # Not yet supported in our MSRV of 1.60.0 # clap = { workspace=true, optional = true } clap = { version = "4.1", features = ["derive", "env"], optional = true } +pin-project = "1" [dev-dependencies] rustc_version = "0.4.0" diff --git a/insta/src/settings.rs b/insta/src/settings.rs index 6b9bf007..fc34715b 100644 --- a/insta/src/settings.rs +++ b/insta/src/settings.rs @@ -523,14 +523,15 @@ impl Settings { /// # } /// ``` pub fn bind_async, T>(&self, future: F) -> impl Future { - struct BindingFuture(Arc, F); + #[pin_project::pin_project] + struct BindingFuture(Arc, #[pin] F); impl Future for BindingFuture { type Output = F::Output; fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll { let inner = self.0.clone(); - let future = unsafe { self.map_unchecked_mut(|s| &mut s.1) }; + let future = self.project().1; CURRENT_SETTINGS.with(|x| { let old = { let mut current = x.borrow_mut(); From c22d1ee26013fad95e2a2fcf68572353c41c26d0 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Thu, 23 Jan 2025 15:42:05 -0800 Subject: [PATCH 09/10] Improved handling of control characters in inline snapshots (#713) Fixes at least some of #712 (not confident it fixes all of it, but wanted to get it out quickly) --- CHANGELOG.md | 4 ++ insta/src/snapshot.rs | 109 +++++++++++++++++++++++++++++------------- 2 files changed, 80 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88b25c83..a1895834 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to insta and cargo-insta are documented here. +## 1.42.1 + +- Improved handling of control characters in inline snapshots. #713 + ## 1.42.0 - Text snapshots no longer contain `snapshot_type: text` in their metadata. For diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs index bdc7a8d6..1d01d517 100644 --- a/insta/src/snapshot.rs +++ b/insta/src/snapshot.rs @@ -689,45 +689,59 @@ impl TextSnapshotContents { let contents = self.normalize(); let mut out = String::new(); - // We don't technically need to escape on newlines, but it reduces diffs - let is_escape = contents.contains(['\\', '"', '\n']); - // Escape the string if needed, with `r#`, using the required number of `#`s - let delimiter = if is_escape { + // Some characters can't be escaped in a raw string literal, so we need + // to escape the string if it contains them. We prefer escaping control + // characters which except for newlines, which we prefer to see as + // actual newlines. + let has_control_chars = contents + .chars() + .any(|c| c != '\n' && c.is_control() || c == '\0'); + + // We prefer raw strings for strings containing a quote or an escape + // character, and for strings containing newlines (which reduces diffs). + // We can't use raw strings for some control characters. + if !has_control_chars && contents.contains(['\\', '"', '\n']) { out.push('r'); - "#".repeat(required_hashes(&contents)) - } else { - "".to_string() - }; + } + + let delimiter = "#".repeat(required_hashes(&contents)); out.push_str(&delimiter); - out.push('"'); - - // if we have more than one line we want to change into the block - // representation mode - if contents.contains('\n') { - out.extend( - contents - .lines() - // Adds an additional newline at the start of multiline - // string (not sure this is the clearest way of representing - // it, but it works...) - .map(|l| { - format!( - "\n{:width$}{l}", - "", - width = if l.is_empty() { 0 } else { indentation }, - l = l - ) - }) - // `lines` removes the final line ending — add back. Include - // indentation so the closing delimited aligns with the full string. - .chain(Some(format!("\n{:width$}", "", width = indentation))), - ); + + // If there are control characters, then we have to just use a simple + // string with unicode escapes from the debug output. We don't attempt + // block mode (though not impossible to do so). + if has_control_chars { + out.push_str(format!("{:?}", contents).as_str()); } else { - out.push_str(contents.as_str()); + out.push('"'); + // if we have more than one line we want to change into the block + // representation mode + if contents.contains('\n') { + out.extend( + contents + .lines() + // Adds an additional newline at the start of multiline + // string (not sure this is the clearest way of representing + // it, but it works...) + .map(|l| { + format!( + "\n{:width$}{l}", + "", + width = if l.is_empty() { 0 } else { indentation }, + l = l + ) + }) + // `lines` removes the final line ending — add back. Include + // indentation so the closing delimited aligns with the full string. + .chain(Some(format!("\n{:width$}", "", width = indentation))), + ); + } else { + out.push_str(contents.as_str()); + } + out.push('"'); } - out.push('"'); out.push_str(&delimiter); out } @@ -969,6 +983,35 @@ b TextSnapshotContents::new("ab".to_string(), TextSnapshotKind::Inline).to_inline(0), r#""ab""# ); + + // Test control and special characters + assert_eq!( + TextSnapshotContents::new("a\tb".to_string(), TextSnapshotKind::Inline).to_inline(0), + r##""a\tb""## + ); + + assert_eq!( + TextSnapshotContents::new("a\t\nb".to_string(), TextSnapshotKind::Inline).to_inline(0), + // No block mode for control characters + r##""a\t\nb""## + ); + + assert_eq!( + TextSnapshotContents::new("a\rb".to_string(), TextSnapshotKind::Inline).to_inline(0), + r##""a\rb""## + ); + + assert_eq!( + TextSnapshotContents::new("a\0b".to_string(), TextSnapshotKind::Inline).to_inline(0), + // Nul byte is printed as `\0` in Rust string literals + r##""a\0b""## + ); + + assert_eq!( + TextSnapshotContents::new("a\u{FFFD}b".to_string(), TextSnapshotKind::Inline).to_inline(0), + // Replacement character is returned as the character in literals + r##""a�b""## + ); } #[test] From 272d628a5087dc6d37a19386dca43b9839624e51 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 25 Jan 2025 10:14:35 -0800 Subject: [PATCH 10/10] Bump version to 1.42.1 (#714) --- Cargo.lock | 4 ++-- cargo-insta/Cargo.toml | 4 ++-- insta/Cargo.toml | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 292b53a0..2ce4ca71 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -66,7 +66,7 @@ dependencies = [ [[package]] name = "cargo-insta" -version = "1.42.0" +version = "1.42.1" dependencies = [ "cargo_metadata", "clap", @@ -353,7 +353,7 @@ dependencies = [ [[package]] name = "insta" -version = "1.42.0" +version = "1.42.1" dependencies = [ "clap", "console", diff --git a/cargo-insta/Cargo.toml b/cargo-insta/Cargo.toml index ac5bd923..44c7e72a 100644 --- a/cargo-insta/Cargo.toml +++ b/cargo-insta/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo-insta" -version = "1.42.0" +version = "1.42.1" license = "Apache-2.0" authors = ["Armin Ronacher "] description = "A review tool for the insta snapshot testing library for Rust" @@ -14,7 +14,7 @@ readme = "README.md" rust-version = "1.65.0" [dependencies] -insta = { version = "=1.42.0", path = "../insta", features = [ +insta = { version = "=1.42.1", path = "../insta", features = [ "json", "yaml", "redactions", diff --git a/insta/Cargo.toml b/insta/Cargo.toml index 747a1ab3..2cefab13 100644 --- a/insta/Cargo.toml +++ b/insta/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "insta" -version = "1.42.0" +version = "1.42.1" license = "Apache-2.0" authors = ["Armin Ronacher "] description = "A snapshot testing library for Rust"