Skip to content

Feat/test quality execution#33

Merged
SaschaOnTour merged 2 commits into
mainfrom
feat/test-quality-execution
Jun 4, 2026
Merged

Feat/test quality execution#33
SaschaOnTour merged 2 commits into
mainfrom
feat/test-quality-execution

Conversation

@SaschaOnTour

Copy link
Copy Markdown
Owner

No description provided.

Copilot AI review requested due to automatic review settings June 4, 2026 16:57

Copilot AI 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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

SaschaOnTour and others added 2 commits June 4, 2026 19:03
…aren't flagged (v1.4.2)

SIT (single-impl trait) counted only production impls — the structural metadata
walk skips test code — so a non-pub trait with one production impl plus
#[cfg(test)] test-double impls (the idiomatic DI / test-seam pattern) was wrongly
flagged as an over-abstraction. It now counts #[cfg(test)] trait impls toward the
trait's total implementor count and fires only when that total is 1. A genuine
single-impl trait (one prod impl, no doubles) is still flagged.

"Test" covers a whole #![cfg(test)] / integration-test file, an inline
#[cfg(test)] mod, and an item-level #[cfg(test)] on the impl (collect_item_metadata
no longer miscounts the latter as a production impl). Counting is name-based (the
metadata models no trait identity), so it is scoped per module: a test impl whose
trait is a bare name defined as a test-only trait in the same module is attributed
to that local trait, not a same-named production trait — so an unrelated test-only
`trait Clock` does not suppress the production `Clock`, while a real double in
another test module still counts. Known residual: a test impl of an
imported/external trait sharing a production trait's last-segment name still
collides (symmetric with the pre-existing production-side name collision; full
resolution needs real trait identity).

Sibling detector OI is unaffected — it matches impl locations, it does not count
(pinned). Regressed when v1.4.x improved #[cfg(test)] mod foo;-chain test-file
recognition, which correctly excluded companion test files and shrank SIT's
denominator.

Four gates green: cargo fmt, 1860 nextest, self-analysis 0 findings, clippy.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… SIT counts impls purely by name

SIT now counts #[cfg(test)] trait impls toward a trait's total implementor count
(so a test-seam trait — one prod impl plus test doubles — is not flagged), using
deliberately name-based counting: it never drops a real double (no false positive
on a legitimate seam), accepting a safe, documented false negative on a rare name
collision with an unrelated test-only trait. Earlier scope/path heuristics were
removed because resolving self::/super::/glob/absolute paths is trait identity,
which the name-keyed metadata models on neither side.

Making SIT count item-level #[cfg(test)] impls surfaced a broader inconsistency:
the other structural detectors treated #[cfg(test)] code in a production file as
production. They now honour #[cfg(test)] at every level that can occur in
compiling code:
- item-level #[cfg(test)] impl: BTC, SLM, NMS, DEH, IET (+ SIT/OI via metadata)
- #[cfg(test)] on an impl method: BTC, SLM, NMS, DEH
- #[cfg(test)] pub fn: IET, DEH
consistent with whole test files and #[cfg(test)] mod blocks.

Four gates green: cargo fmt, 1870 nextest, self-analysis 0 findings, clippy.

Copilot AI 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.

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.

@SaschaOnTour SaschaOnTour merged commit 4f5b5a8 into main Jun 4, 2026
2 checks passed
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.

2 participants