Conversation
Summary of ChangesHello @rjmurillo, 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 for the Moq1203 analyzer, ensuring its robustness across a wider range of Moq usage patterns. The added tests cover complex scenarios like variable-based mock setups and custom return types, which were previously not fully validated. This expansion aims to prevent regressions and improve the accuracy of the analyzer's diagnostics for common Moq configurations. 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
|
📝 WalkthroughWalkthroughAdded comprehensive test data and test methods to the MethodSetupShouldSpecifyReturnValueAnalyzerTests to validate the analyzer's behavior with semantic variations in Moq setups and custom return types, including scenarios with variable mocks, MockBehavior parameters, and custom record types. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
DeepSource reviewed changes in the commit range For detailed review results, please see the PR on DeepSource ↗ PR Report CardCode Review Summary
How are these analyzer statuses calculated?Administrators can configure which issue categories are reported and cause analysis to be marked as failed when detected. This helps prevent bad and insecure code from being introduced in the codebase. If you're an administrator, you can modify this in the repository's settings. |
There was a problem hiding this comment.
Code Review
This pull request introduces new test cases for the Moq1203 analyzer, covering variable-based mock construction, MockBehavior parameters, variable arguments, callback chains, and custom record types. The changes aim to improve the analyzer's ability to detect missing return value specifications in various scenarios. The existing review comment addresses a potential issue and has been retained.
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 |
fbecc1d to
85a99fe
Compare
ba2f064 to
6040d3c
Compare
…tom types Existing Moq1203 tests exercised a single semantic profile: inline new Mock<IFoo>(), primitive Task<int>, and literal arguments. Line/branch coverage at 100% could not detect this gap because the analyzer traverses identical code paths regardless of mock construction form or return type. Add variable-based mock tests (variable construction, MockBehavior params, variable arguments, callback chains) and custom record type reproduction tests matching the exact pattern from issue #849 (Task<MyValue> with ReturnsAsync). Adds 50 new test permutations across both Moq versions. Refs: #849, #904 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6040d3c to
46e17b2
Compare
tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs`:
- Around line 452-471: Tests marked as "no diagnostics"
(ShouldNotFlagSemanticVariationSetups and
ShouldNotFlagSetupWithCustomReturnType) currently call VerifyMockAsync /
VerifyCustomSourceMockAsync which run only the single analyzer; replace those
calls so the tests use AllAnalyzersVerifier.VerifyAllAnalyzersAsync to ensure no
other Moq analyzers produce diagnostics. Specifically, change the body of
ShouldNotFlagSemanticVariationSetups to call
AllAnalyzersVerifier.VerifyAllAnalyzersAsync with the same parameters
(referenceAssemblyGroup, `@namespace`, mock), and change
ShouldNotFlagSetupWithCustomReturnType to call
AllAnalyzersVerifier.VerifyAllAnalyzersAsync passing referenceAssemblyGroup,
`@namespace`, mock, recordDeclaration, interfaceMethod (or the appropriate
overload) instead of VerifyCustomSourceMockAsync.
- Around line 334-392: Add ValueTask variants mirroring the existing
Task<MyValue> cases in both CustomReturnTypeTestData and
CustomReturnTypeMissingReturnValueTestData: for each Task entry duplicate it but
change the method signature string from "Task<MyValue> ..." to
"ValueTask<MyValue> ...", keep the same setup code but use
ReturnsAsync(expectedValue) (and ThrowsAsync for the throws case) for the
positive cases, and duplicate the missing-return cases with "ValueTask<MyValue>"
signatures so the {|Moq1203:...|} diagnostics are exercised for ValueTask as
well; update entries that reference MockBehavior.Loose and the parameterless
record reproduction similarly so CustomReturnTypeTestData and
CustomReturnTypeMissingReturnValueTestData both include Task and ValueTask
variants.
tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs
Show resolved
Hide resolved
tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs
Show resolved
Hide resolved
…tom types (#905) ## Summary - Add 50 new test permutations for Moq1203 covering variable-based mock construction, `MockBehavior` parameters, variable arguments, and callback chains - Add custom record type reproduction tests matching the exact pattern from issue #849 (`Task<MyValue>` with `ReturnsAsync`) - Existing tests only exercised inline `new Mock<IFoo>()` with primitive `Task<int>` and literal arguments, which 100% line/branch coverage could not distinguish from the failing pattern ## Test plan - [x] `dotnet build tests/Moq.Analyzers.Test` compiles successfully - [x] All 216 tests pass (50 new permutations added) - [x] Custom-type test (`ShouldNotFlagSetupWithCustomReturnType`) confirms the #849 fix works for `Task<MyValue>` Refs: #849, #904 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Expanded test coverage for Moq setup analysis scenarios, including semantic variations, custom return types, and callback configurations across multiple reference assembly groups. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
MockBehaviorparameters, variable arguments, and callback chainsMoq1203after upgrading tov0.4.0#849 (Task<MyValue>withReturnsAsync)new Mock<IFoo>()with primitiveTask<int>and literal arguments, which 100% line/branch coverage could not distinguish from the failing patternTest plan
dotnet build tests/Moq.Analyzers.Testcompiles successfullyShouldNotFlagSetupWithCustomReturnType) confirms the Incorrect detection/flagging of ruleMoq1203after upgrading tov0.4.0#849 fix works forTask<MyValue>Refs: #849, #904
🤖 Generated with Claude Code
Summary by CodeRabbit