Add missing Word Controller/Service tests#4151
Conversation
|
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. 📝 WalkthroughWalkthroughRefactors controller tests to consistently unwrap Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4151 +/- ##
===========================================
+ Coverage 74.75% 86.07% +11.31%
===========================================
Files 302 56 -246
Lines 11083 4839 -6244
Branches 1394 603 -791
===========================================
- Hits 8285 4165 -4120
+ Misses 2394 528 -1866
+ Partials 404 146 -258
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
If fixable (same as other recent review) please fix |
jasonleenaylor
left a comment
There was a problem hiding this comment.
@jasonleenaylor reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Backend.Tests/Controllers/WordControllerTests.cs (1)
104-106: Apply null-forgiving!after null assertions — pervasive CS8602 patternEvery changed test follows the pattern:
var result = await _wordController.Xxx(...) as OkObjectResult; // OkObjectResult? Assert.That(result, Is.Not.Null); // compiler flow unaffected Assert.That(result.Value, Is.True); // CS8602: result is still OkObjectResult?The compiler does not narrow the type through NUnit assertion calls, so every
.Value,.Count, or indexer access on the un-suffixed nullable produces CS8602/CS8604 warnings. The service-test file's existing lines 76–77 already use the correctresult!.convention.Affected variables (representative, not exhaustive):
result,countResult,frontier,reverted,id,updatedWord.🛠️ Representative fix pattern (apply to all similar occurrences)
- var result = await _wordController.HasFrontierWords(ProjId) as OkObjectResult; + var result = await _wordController.HasFrontierWords(ProjId) as OkObjectResult; Assert.That(result, Is.Not.Null); - Assert.That(result.Value, Is.False); + Assert.That(result!.Value, Is.False);For intermediate typed variables:
- var frontier = result.Value as List<Word>; + var frontier = result!.Value as List<Word>; Assert.That(frontier, Is.Not.Null); - Assert.That(frontier, Has.Count.EqualTo(2)); + Assert.That(frontier!, Has.Count.EqualTo(2));Also applies to: 114-116, 131-133, 140-142, 161-163, 183-185, 233-235, 242-244, 272-274, 302-304, 344-346, 375-377, 421-422, 436-438
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Backend.Tests/Controllers/WordControllerTests.cs` around lines 104 - 106, The tests assign controller responses to nullable locals (e.g., result, countResult, frontier, reverted, id, updatedWord) using "as OkObjectResult" and then Assert.That(..., Is.Not.Null) but the compiler doesn't recognize NUnit's assertion, so add the null-forgiving operator when dereferencing these variables (e.g., change result.Value to result!.Value) throughout the file; update all occurrences where properties or indexers are accessed after an Assert.That(..., Is.Not.Null) for methods like _wordController.HasFrontierWords, Count/Index assertions, GetFrontier, RevertWord, UpdateWord, etc., to use the pattern variable! before .Value, .Count or index access to silence CS8602/CS8604 warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Backend.Tests/Controllers/WordControllerTests.cs`:
- Around line 104-106: The tests assign controller responses to nullable locals
(e.g., result, countResult, frontier, reverted, id, updatedWord) using "as
OkObjectResult" and then Assert.That(..., Is.Not.Null) but the compiler doesn't
recognize NUnit's assertion, so add the null-forgiving operator when
dereferencing these variables (e.g., change result.Value to result!.Value)
throughout the file; update all occurrences where properties or indexers are
accessed after an Assert.That(..., Is.Not.Null) for methods like
_wordController.HasFrontierWords, Count/Index assertions, GetFrontier,
RevertWord, UpdateWord, etc., to use the pattern variable! before .Value, .Count
or index access to silence CS8602/CS8604 warnings.
jasonleenaylor
left a comment
There was a problem hiding this comment.
@jasonleenaylor reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).
This change is
Summary by CodeRabbit