Skip to content

fix(registry): add --tests to cargo-clippy, add test coverage#176

Open
zeitlinger wants to merge 6 commits intomainfrom
fix/clippy-tests
Open

fix(registry): add --tests to cargo-clippy, add test coverage#176
zeitlinger wants to merge 6 commits intomainfrom
fix/clippy-tests

Conversation

@zeitlinger
Copy link
Copy Markdown
Member

Summary

  • Add --tests to cargo-clippy registry entry — test code was previously unlinted
  • Apply resulting clippy fixes (unused var, if-let chains, type_complexity allow)
  • Add failure-in-tests e2e case to verify test code linting works
  • Fix existing failure fixture: (lib)(lib test) in error message (changed by --tests)

Motivated by a bug caught immediately: missing arg in test call sites went undetected until CI.

Test plan

  • CI passes (Linux, macOS, Windows)

…errors

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
… in existing fixture

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger zeitlinger marked this pull request as ready for review April 16, 2026 13:40
@zeitlinger zeitlinger requested a review from a team as a code owner April 16, 2026 13:40
Copilot AI review requested due to automatic review settings April 16, 2026 13:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Rust cargo-clippy registry check to lint test code as well, and adds an e2e fixture to ensure clippy failures originating from test modules are caught by CI.

Changes:

  • Update the cargo-clippy registry entry to include test linting and adjust the fix command accordingly.
  • Apply clippy-driven refactors in test/helpers and internal code (if let chains, type_complexity allow, minor collection cleanup).
  • Add a new failure-in-tests e2e case and update existing cargo-clippy failure snapshot output.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/registry.rs Adds --tests to the cargo-clippy check/fix commands; minor test helper adjustment (headers.to_vec()).
tests/e2e.rs Refactors snapshot writer to use if let chains when serializing optional TOML tables.
tests/cases/cargo-clippy/failure/test.toml Updates expected clippy failure output to match new target diagnostics.
tests/cases/cargo-clippy/failure-in-tests/test.toml Adds new snapshot asserting clippy fails on test-only code.
tests/cases/cargo-clippy/failure-in-tests/files/src/lib.rs Introduces a unit test that triggers an unused_variables failure under clippy.
tests/cases/cargo-clippy/failure-in-tests/files/Cargo.toml New fixture crate manifest for the e2e case.
tests/cases/cargo-clippy/failure-in-tests/files/mise.toml Uses rust = "latest" for the fixture environment.
src/linters/renovate_deps.rs Adds a local #[allow(clippy::type_complexity)] in tests.
src/init/mod.rs Refactors component insertion logic using an if let chain.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/registry.rs
--tests runs clippy in test configuration, which excludes #[cfg(not(test))]
code paths. --all-targets checks lib, bins, tests, and examples, giving
broader coverage without dropping any build targets.

Update failure fixture: --all-targets emits two "could not compile" lines
(lib then lib test) vs one with --tests.
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
Copy link
Copy Markdown
Member Author

@martincostello can you check?

@martincostello
Copy link
Copy Markdown
Member

Will look tomorrow - been busy with upstream work most of this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants