Visitar URL original
test: migrate `show_settings` and `version` tests to use `CliTest` by Beltranss · Pull Request #23702 · astral-sh/ruff · GitHub
Skip to content

test: migrate show_settings and version tests to use CliTest - #23702

Merged
ntBre merged 6 commits into
astral-sh:mainfrom
Beltranss:refactor/cli-test-cleanup
Mar 18, 2026
Merged

ntBre merged 6 commits into
astral-sh:mainfrom
Beltranss:refactor/cli-test-cleanup

Conversation

@Beltranss

Copy link
Copy Markdown
Contributor

Summary

Part of #19016.

Migrates two standalone test files into the cli/ module to use the shared CliTest test fixture, reducing boilerplate:

  • show_settings.rs — Removed manual TempDir creation, dunce::canonicalize, insta::with_settings! filters, and Command::new(get_cargo_bin(...)). Now uses CliTest::with_files and test.command().
  • version.rs — Wrapped in a VersionTest struct (same pattern as AnalyzeTest in cli/analyze_graph.rs) to add the version-specific regex filter via CliTest::with_settings. Removed direct TempDir and get_cargo_bin usage.

AI Assistance Disclosure

This PR was written with the assistance of Claude Code (Claude Opus). The migration, test verification, and snapshot regeneration were done with AI assistance.

Test plan

  • All 193 CLI tests pass (cargo test -p ruff --test cli)
  • No warnings from the compiler
  • Snapshots regenerated and accepted for the new module paths

🤖 Generated with Claude Code

Move `show_settings.rs` and `version.rs` from standalone test files into
the `cli/` module to use the shared `CliTest` test fixture, reducing
boilerplate for tempdir management, snapshot filtering, and command
creation.

Part of astral-sh#19016.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@astral-sh-bot
astral-sh-bot Bot requested a review from ntBre March 3, 2026 22:30

@ntBre ntBre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this looks reasonable to me. I just had a few small questions/suggestions.

Comment thread crates/ruff/tests/cli/version.rs Resolved
Comment thread crates/ruff/tests/cli/version.rs Resolved
Comment thread crates/ruff/tests/cli/version.rs Resolved
@ntBre ntBre added the testing Related to testing Ruff or ty label Mar 3, 2026
@astral-sh-bot

astral-sh-bot Bot commented Mar 3, 2026

Copy link
Copy Markdown

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@statxc

statxc commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

@Beltranss Sorry to interrupt. I just want to know whether you’re working on this right now. If not, would it be okay for me to take it and finish it?

Cc: @ntBre

@ntBre

ntBre commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

I think I'll just push a couple of commits here to address my own feedback, but thank you for the offer.

@statxc

statxc commented Mar 18, 2026 •

Copy link
Copy Markdown
Contributor

@ntBre Thanks for the quick response. I mean, it doesn’t look like there’s been any progress on this for a while, so I asked @Beltranss whether he is working on it now. If not, I can open my PR that reflects your feedback.

@ntBre ntBre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@ntBre
ntBre enabled auto-merge (squash) March 18, 2026 19:36
@ntBre
ntBre merged commit 6e20f22 into astral-sh:main Mar 18, 2026
41 checks passed
carljm added a commit that referenced this pull request Mar 25, 2026
* main:
  [`pylint`] Improve phrasing (`PLC0208`) (#24033)
  test: migrate `show_settings` and `version` tests to use `CliTest` (#23702)
  Drain file watcher events during test setup (#24030)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Related to testing Ruff or ty

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants