test: add direct unit tests for MockBehaviorDiagnosticAnalyzerBase#1038
test: add direct unit tests for MockBehaviorDiagnosticAnalyzerBase#1038
Conversation
) Add 18 tests exercising MockBehaviorDiagnosticAnalyzerBase shared logic through both concrete subclasses (SetExplicitMockBehaviorAnalyzer and SetStrictMockBehaviorAnalyzer). Tests cover: - IsMockReferenced() early return when Moq is not referenced - AnalyzeObjectCreation type guard for non-Mock types - AnalyzeInvocation method guard for non-Mock.Of invocations - Non-mock object creation with Moq referenced - Moq method invocations that are not Mock.Of Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the test suite by introducing new unit tests for the core logic of MockBehaviorDiagnosticAnalyzerBase. These tests ensure the robustness and correctness of the diagnostic analyzers by validating their behavior across various scenarios, including critical guard clauses and edge cases, thereby improving the overall reliability of the Moq analyzers. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new test file that directly exercises MockBehaviorDiagnosticAnalyzerBase via both explicit and strict analyzers, introducing data providers and multiple non-mock/no-Moq scenarios to validate diagnostic suppression paths. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
|
Overall Grade Focus Area: Hygiene |
Security Reliability Complexity Hygiene |
Feedback
- Missing 'Async' suffix in test methods
- Test methods returning Task repeatedly lack the Async suffix because existing test code/templates omitted it; update test templates and apply the IDE/codefix rename to restore the naming convention and stop manual drift.
- Analyzers or EditorConfig not applied to test project
- Identical violations across a file suggest the analyzer/editorconfig isn't active for that project; include the analyzer package or enable the rule (warning/error) in the test project's config so the build enforces the convention.
- Copy-paste scaffolding propagates the same mistake
- Multiple occurrences look like cloned test scaffolds carrying the same naming omission; fix the scaffold/template and add a small generation-time check to prevent future copies from repeating the error.
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# | Mar 8, 2026 1:16a.m. | Review ↗ |
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive set of unit tests for the shared logic in MockBehaviorDiagnosticAnalyzerBase. The tests are well-structured and cover important edge cases and guard conditions in the base analyzer. My main feedback is to reduce code duplication by combining the tests for the explicit and strict analyzers using a local helper method, which I've detailed in a specific comment. Overall, this is a valuable addition that improves test coverage.
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
…d helper Introduce VerifyBothAnalyzersAsync helper to eliminate 5 pairs of duplicate test methods, reducing them to 5 single methods that verify both analyzers concurrently via Task.WhenAll. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nalyzersAsync Task.WhenAll masks the second failure when both tasks fail, reporting only the first exception. Sequential awaits ensure each failure is reported independently, improving debuggability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/Moq.Analyzers.Test/MockBehaviorDiagnosticAnalyzerBaseTests.cs`:
- Around line 91-103: Remove the unused ISample interface declaration from the
test source: locate the string constant named source that contains "public
interface ISample { }" and delete that interface declaration so the test only
includes the necessary UnitTest class and its Test method creating new object();
ensure no other references to ISample remain in the file (e.g., in
MockBehaviorDiagnosticAnalyzerBaseTests) before committing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ba76e2af-ce73-496e-bfd7-92f0f541102a
📒 Files selected for processing (1)
tests/Moq.Analyzers.Test/MockBehaviorDiagnosticAnalyzerBaseTests.cs
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…haviorDiagnosticAnalyzerBaseTests Add ConfigureAwait(false) to all await calls in the test file to resolve MA0004 build warnings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/Moq.Analyzers.Test/MockBehaviorDiagnosticAnalyzerBaseTests.cs`:
- Line 44: Test methods call await
VerifyBothAnalyzersAsync(...).ConfigureAwait(false) which triggers xUnit1030;
remove the ConfigureAwait(false) suffix from each await in the test methods (the
calls in MockBehaviorDiagnosticAnalyzerBaseTests that invoke
VerifyBothAnalyzersAsync) so they read simply await
VerifyBothAnalyzersAsync(...). The helper VerifyBothAnalyzersAsync may keep
ConfigureAwait(false) if needed; only update the test method call sites to
eliminate ConfigureAwait(false).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 776fc797-bf31-4cc7-b5f3-15812d8c32b0
📒 Files selected for processing (1)
tests/Moq.Analyzers.Test/MockBehaviorDiagnosticAnalyzerBaseTests.cs
tests/Moq.Analyzers.Test/MockBehaviorDiagnosticAnalyzerBaseTests.cs
Outdated
Show resolved
Hide resolved
…t1030 xUnit1030 prohibits ConfigureAwait(false) in test methods because it bypasses xUnit's parallelization limits. Kept ConfigureAwait(false) in the private helper method VerifyBothAnalyzersAsync where MA0004 still requires it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…1038) ## Summary - Adds 18 tests for `MockBehaviorDiagnosticAnalyzerBase` shared logic, exercised through both concrete subclasses (`SetExplicitMockBehaviorAnalyzer` and `SetStrictMockBehaviorAnalyzer`) - Tests cover base class branches not previously exercised: `IsMockReferenced()` early return, `AnalyzeObjectCreation` type guards, and `AnalyzeInvocation` method guards - Documents the untestable `MockBehavior is null` branch (requires Moq.Mock without Moq.MockBehavior, which cannot occur with any real Moq assembly) Closes #926 ## Test plan - [x] All 18 new tests pass - [x] Full test suite passes (2919 tests, 0 failures) - [x] No build warnings 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added tests covering analyzer base behavior to ensure no reports for various non-mock creation and invocation scenarios. * Added coverage across multiple Moq/reference variants. * Introduced a shared helper to run and verify both analyzer modes (Explicit and Strict) across inputs. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Richard Murillo <rjmurillo@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
MockBehaviorDiagnosticAnalyzerBaseshared logic, exercised through both concrete subclasses (SetExplicitMockBehaviorAnalyzerandSetStrictMockBehaviorAnalyzer)IsMockReferenced()early return,AnalyzeObjectCreationtype guards, andAnalyzeInvocationmethod guardsMockBehavior is nullbranch (requires Moq.Mock without Moq.MockBehavior, which cannot occur with any real Moq assembly)Closes #926
Test plan
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Summary by CodeRabbit