Skip to content

test(rustfix): Use snapbox for snapshot testing #15429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 14, 2025

Conversation

Pyr0de
Copy link
Contributor

@Pyr0de Pyr0de commented Apr 13, 2025

What does this PR try to resolve?

  • separates each test into different test cases
  • snapbox is used to test the snapshots
    • difference in .json file alone should never cause a test to fail
    • .json files updated only if expected fix != actual fix && SNAPSHOTS=overwrite
  • when .json files are updated, the json is pretty printed
  • replaced environment variables RUSTFIX_TEST_* for overwriting test snapshots with SNAPSHOTS=overwrite
    • ❗ The RUSTFIX_TEST_RECORD_FIXED_RUST feature is removed (generate a *.fixed.rs on demand`). We can add it back whenever needed.

Fixes #13891

How should we test and review this PR?

Run tests with:

cargo test -p rustfix

All the test should run as different test cases
nightly tests run only when using nightly version of rustc is used

@rustbot
Copy link
Collaborator

rustbot commented Apr 13, 2025

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added Command-fix S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 13, 2025
@Pyr0de Pyr0de force-pushed the issue-13891 branch 2 times, most recently from 079c5f2 to 07b5413 Compare April 13, 2025 11:16
@Pyr0de Pyr0de changed the title Migrate away from ad-hoc snapshot testing for rustfix test suite test(rustfix): Use snapbox and #[cargo_test] for snapshot testing Apr 13, 2025
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Pyr0de
Copy link
Contributor Author

Pyr0de commented Apr 13, 2025

The test multiple-solutions.nightly.rs can be changed back to stable since rustc 1.80 is stable
rust-lang/rust#123344

@weihanglo weihanglo changed the title test(rustfix): Use snapbox and #[cargo_test] for snapshot testing test(rustfix): Use snapbox or snapshot testing Apr 14, 2025
@weihanglo weihanglo changed the title test(rustfix): Use snapbox or snapshot testing test(rustfix): Use snapbox for snapshot testing Apr 14, 2025
@weihanglo
Copy link
Member

The test multiple-solutions.nightly.rs can be changed back to stable since rustc 1.80 is stable rust-lang/rust#123344

That sounds good to me. Could you help do it in an extra commit?

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@weihanglo weihanglo added this pull request to the merge queue Apr 14, 2025
Merged via the queue into rust-lang:master with commit db99ddc Apr 14, 2025
25 checks passed
@Pyr0de Pyr0de deleted the issue-13891 branch April 15, 2025 10:40
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2025
Update cargo

4 commits in 864f74d4eadcaea3eeda37a2e7f4d34de233d51e..d811228b14ae2707323f37346aee3f4147e247e6
2025-04-11 20:37:27 +0000 to 2025-04-15 15:18:42 +0000
- use `zlib-rs` for gzip compression in rust code (rust-lang/cargo#15417)
- test(rustfix): Use `snapbox` for snapshot testing (rust-lang/cargo#15429)
- chore(deps): update rust crate gix to 0.71.0 [security] (rust-lang/cargo#15391)
- Make sure search paths inside OUT_DIR precede external paths (rust-lang/cargo#15221)

Also,

* The license exception of sha1_smol with BSD-3-Clause is no longer needed, as `gix-*` doesn't depend on it.
* Cargo depends on zlib-rs, which is distributed under Zlib license

r? ghost
@rustbot rustbot added this to the 1.88.0 milestone Apr 17, 2025
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Apr 19, 2025
Update cargo

4 commits in 864f74d4eadcaea3eeda37a2e7f4d34de233d51e..d811228b14ae2707323f37346aee3f4147e247e6
2025-04-11 20:37:27 +0000 to 2025-04-15 15:18:42 +0000
- use `zlib-rs` for gzip compression in rust code (rust-lang/cargo#15417)
- test(rustfix): Use `snapbox` for snapshot testing (rust-lang/cargo#15429)
- chore(deps): update rust crate gix to 0.71.0 [security] (rust-lang/cargo#15391)
- Make sure search paths inside OUT_DIR precede external paths (rust-lang/cargo#15221)

Also,

* The license exception of sha1_smol with BSD-3-Clause is no longer needed, as `gix-*` doesn't depend on it.
* Cargo depends on zlib-rs, which is distributed under Zlib license

r? ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Command-fix S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate away from ad-hoc snapshot testing for rustfix test suite
4 participants