[code-simplifier] refactor: remove duplicate top-level interface declarations from LoggingManagerTests#8196
Merged
Evangelink merged 1 commit intoMay 14, 2026
Conversation
…ingManagerTests The three top-level namespace interfaces (IExtensionLoggerProvider, IInitializableLoggerProvider, IInitializableExtensionLoggerProvider) were stale leftovers. The test class already declares the interfaces it needs as private nested types (IExtensionLoggerProvider, IExtensionInitializableLoggerProvider, IInitializableLoggerProvider). The top-level IInitializableExtensionLoggerProvider was also unused. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Removes stale, duplicate namespace-scoped helper interfaces from LoggingManagerTests, relying solely on the already-present nested interfaces within the test class to avoid ambiguity and dead code in the test project.
Changes:
- Removed redundant top-level interface declarations (
IExtensionLoggerProvider,IInitializableLoggerProvider,IInitializableExtensionLoggerProvider) fromLoggingManagerTests.cs. - Kept/confirmed usage of the nested helper interfaces inside
LoggingManagerTests(no remaining references to the removed top-level interfaces).
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/Microsoft.Testing.Platform.UnitTests/Logging/LoggingManagerTests.cs | Deletes duplicate/unused namespace-scoped interfaces; tests continue using the nested interface helpers. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 0
Evangelink
commented
May 13, 2026
Member
Author
Evangelink
left a comment
There was a problem hiding this comment.
Code Review — PR #8196
Summary: Clean dead-code removal. ✅
Analysis across all 21 review dimensions
| Dimension | Status | Notes |
|---|---|---|
| Correctness | ✅ | All usages in the file reference the nested private interfaces inside LoggingManagerTests, not the removed top-level ones. |
| Breaking changes | ✅ | All three interfaces were internal and are unused outside LoggingManagerTests.cs. No other test or source file references them (grep confirms zero hits). |
| Dead code removal | ✅ | The three removed interfaces (IExtensionLoggerProvider, IInitializableLoggerProvider, IInitializableExtensionLoggerProvider) were stale top-level duplicates with no callers. |
| Duplicate detection | ✅ | The nested interfaces cover the same contracts: IExtensionLoggerProvider, IExtensionInitializableLoggerProvider (renamed from IInitializableExtensionLoggerProvider), and IInitializableLoggerProvider. |
| Test coverage | ✅ | No tests are affected; the Moq mocks all target the nested interfaces which remain intact. |
| Naming consistency | ✅ | The nested IExtensionInitializableLoggerProvider is actually a better name than the removed IInitializableExtensionLoggerProvider (adjective order matches the rest of the codebase). |
| Scope/visibility | ✅ | The nested interfaces are internal within a public sealed class — appropriate for test-only helpers. |
| API surface area | ✅ | Reduction only; no new public/internal API introduced. |
| PublicAPI.Unshipped.txt | ✅ | Not applicable (internal test interfaces). |
| Resource / localization files | ✅ | Not applicable. |
| Null safety | ✅ | Not applicable. |
| Style / EditorConfig | ✅ | No style changes introduced. |
| StyleCop (SA rules) | ✅ | Not applicable to this diff. |
| Init accessor policy | ✅ | Not applicable. |
| Logic errors | ✅ | None. |
| Security | ✅ | Not applicable. |
| Performance | ✅ | Not applicable. |
| Documentation | ✅ | No docs needed for internal test helpers. |
| Dependency hygiene | ✅ | No new dependencies. |
| Build impact | ✅ | Removing unused interfaces will only reduce compiler work. |
| Overall risk | ✅ | Very low — pure dead-code cleanup with no behavioral change. |
Verdict
The change is correct and safe to merge. The removed top-level interfaces were unused leftovers that the nested private types inside LoggingManagerTests already supersede. No action required.
Generated by Expert Code Review (on open) for issue #8196 · ● 3.6M
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The three top-level namespace interfaces (IExtensionLoggerProvider,
IInitializableLoggerProvider, IInitializableExtensionLoggerProvider)
were stale leftovers. The test class already declares the interfaces it
needs as private nested types (IExtensionLoggerProvider,
IExtensionInitializableLoggerProvider, IInitializableLoggerProvider).
The top-level IInitializableExtensionLoggerProvider was also unused.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com
Fixes #8182