Skip to content

Clippy rules (test related)#7968

Merged
youknowone merged 6 commits into
RustPython:mainfrom
ShaharNaveh:clippy-rules-0
May 25, 2026
Merged

Clippy rules (test related)#7968
youknowone merged 6 commits into
RustPython:mainfrom
ShaharNaveh:clippy-rules-0

Conversation

@ShaharNaveh
Copy link
Copy Markdown
Contributor

@ShaharNaveh ShaharNaveh commented May 24, 2026 •
edited by coderabbitai Bot
Loading

Summary by CodeRabbit

  • Tests

    • Reorganized many tests into dedicated test modules and standardized test naming to a non-test_ prefix for consistency and clarity.

  • Chores

    • Enabled additional Rust lints in workspace configuration to tighten code-quality checks.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 24, 2026 •
edited
Loading

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

This PR enables three new clippy restriction lints (cfg_not_test, redundant_test_prefix, tests_outside_test_module) in the workspace configuration, then systematically refactors test functions across the entire codebase to comply. Test functions are renamed by removing the test_ prefix, and scattered tests are reorganized into dedicated #[cfg(test)] mod tests modules.

Changes

Test naming and organization refactoring driven by new clippy lints

Layer / File(s) Summary
Workspace clippy lint configuration
Cargo.toml		 Added three new clippy restriction lints: cfg_not_test, redundant_test_prefix, and tests_outside_test_module to enforce test naming and module organization standards.
Common module test refactoring
crates/common/src/cformat.rs, crates/common/src/format.rs, crates/common/src/str.rs		 Renamed test functions by removing test_ prefix across formatting and string parsing tests (e.g., test_fill_and_align → fill_and_align, test_parse_key → parse_key).
Compiler-core and compiler test refactoring
crates/compiler-core/src/bytecode.rs, crates/compiler-core/src/varint.rs, crates/compiler/src/lib.rs		 Renamed exception-table and varint unit tests in bytecode.rs/varint.rs; reorganized smoke tests in compiler/lib.rs into a dedicated #[cfg(test)] mod tests module with test functions renamed from test_* to compile_*.
SRE engine, stdlib, and doc test refactoring
crates/sre_engine/tests/tests.rs, crates/stdlib/src/_opcode.rs, crates/stdlib/src/ssl/oid.rs, crates/doc/src/lib.rs		 Reorganized sre_engine tests into #[cfg(test)] module and renamed regex test functions; renamed stdlib opcode and OID lookup tests by removing test_ prefix; renamed doc database test function.
VM module test refactoring
crates/vm/src/... (many files)		 Renamed unit test functions across VM modules by removing test_ prefix; reorganized frozen-module and other scattered tests into #[cfg(test)] mod tests modules while preserving test logic and assertions.
C API & JIT test refactoring
crates/capi/*, crates/jit/tests/*		 Renamed C-API and JIT test functions, wrapped JIT test files in #[cfg(test)] mod tests, moved helper macros/imports into test modules, and applied small test fixes (syntax and additional assertions) where needed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • RustPython/RustPython#7916: Both PRs modify crates/vm/src/vm/python_run.rs tests for block-expression cases; this PR renames those tests while #7916 adds related test coverage.
  • RustPython/RustPython#7764: Both PRs modify Cargo.toml workspace Clippy lint configuration to enable additional lint rules.
  • RustPython/RustPython#7755: Both PRs update the workspace Clippy lint block; changes are related at the manifest configuration level.

Suggested reviewers

  • youknowone
  • coolreader18

Poem

🐰 In burrows of code I hop and play,
I chase the prefixes far away.
Tests tidy, modules snug and tight,
Clippy nods — the suite feels right.
A carrot of order, quick and spry, hooray!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Clippy rules (test related)' accurately reflects the main change: enabling new Clippy lints in Cargo.toml and refactoring test functions across multiple files to comply with the new lints.
Docstring Coverage ✅ Passed Docstring coverage is 99.43% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot reviewed May 24, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/sre_engine/tests/tests.rs`:
- Line 7: Fix the typo in the test attribute by changing the reason string in
the #[expect(unused, reason = "Nedded for automated script")] attribute to
"Needed for automated script" so the attribute reads #[expect(unused, reason =
"Needed for automated script")]; update the attribute where it appears in
tests.rs (the #[expect(...)] on the test) to correct the spelling only.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: bf39a243-7f5c-402e-92ed-57e4e899e5ee

📥 Commits

Reviewing files that changed from the base of the PR and between e1d9a11 and f97fdb2.

📒 Files selected for processing (23)
  • Cargo.toml
  • crates/codegen/src/compile.rs
  • crates/common/src/cformat.rs
  • crates/common/src/format.rs
  • crates/common/src/str.rs
  • crates/compiler-core/src/bytecode.rs
  • crates/compiler-core/src/varint.rs
  • crates/compiler/src/lib.rs
  • crates/doc/src/lib.rs
  • crates/sre_engine/tests/tests.rs
  • crates/stdlib/src/_opcode.rs
  • crates/stdlib/src/ssl/oid.rs
  • crates/vm/src/builtins/type.rs
  • crates/vm/src/dict_inner.rs
  • crates/vm/src/eval.rs
  • crates/vm/src/function/builtin.rs
  • crates/vm/src/gc_state.rs
  • crates/vm/src/getpath.rs
  • crates/vm/src/stdlib/_io.rs
  • crates/vm/src/types/slot_defs.rs
  • crates/vm/src/vm/interpreter.rs
  • crates/vm/src/vm/mod.rs
  • crates/vm/src/vm/python_run.rs

Comment thread crates/sre_engine/tests/tests.rs Outdated
youknowone
youknowone approved these changes May 25, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

i love this clippy rule. using test_ prefix for every test doesn't make sense

@youknowone youknowone merged commit 9701c46 into RustPython:main May 25, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ShaharNaveh @youknowone