refactor(tests): more idiomatic Fake methods and result assertions #206
Conversation
WalkthroughThis pull request focuses on renaming methods for consistency in generating and retrieving player data. The method previously named Changes
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
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 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #206 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 173 173
Branches 14 14
=========================================
Hits 173 173
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs (1)
284-299: Added method to create response models listThe new
MakeResponseModelsForRetrieve()method efficiently creates a list of response models from the starting eleven players, reducing code duplication in test methods.There's some code duplication between this method and other response model creation methods, such as
MakeResponseModelForCreate()andMakeResponseModelForRetrieve(). Consider extracting the player-to-response-model mapping logic into a separate private method to reduce duplication:+ private static PlayerResponseModel MapPlayerToResponseModel(Player player) + { + return new PlayerResponseModel + { + Id = player.Id, + FullName = + $"{player.FirstName} {(string.IsNullOrWhiteSpace(player.MiddleName) ? "" : player.MiddleName + " ")}{player.LastName}".Trim(), + Birth = $"{player.DateOfBirth:MMMM d, yyyy}", + Dorsal = player.SquadNumber, + Position = player.Position, + Club = player.Team, + League = player.League, + Starting11 = player.Starting11 ? "Yes" : "No" + }; + }Then update the response model methods to use this shared mapping method.
🧰 Tools
🪛 GitHub Check: Codeac Code Quality
[warning] 287-299: CodeDuplication
This block of 12 lines is too similar to test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs:226
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/Dotnet.Samples.AspNetCore.WebApi/Data/PlayerData.cs(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Utilities/ApplicationBuilderExtensions.cs(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Utilities/DbContextUtils.cs(1 hunks)test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs(30 hunks)test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerServiceTests.cs(14 hunks)test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/DatabaseFakes.cs(1 hunks)test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/Dotnet.Samples.AspNetCore.WebApi/Data/PlayerData.cs (2)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs (4)
List(15-171)List(284-299)Player(173-180)Player(182-198)src/Dotnet.Samples.AspNetCore.WebApi/Models/Player.cs (1)
Player(11-34)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/DatabaseFakes.cs (1)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs (1)
PlayerFakes(13-314)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs (1)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs (7)
PlayerFakes(13-314)PlayerResponseModel(222-238)PlayerResponseModel(264-282)PlayerResponseModel(310-313)PlayerRequestModel(204-220)PlayerRequestModel(244-262)PlayerRequestModel(305-308)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerServiceTests.cs (1)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs (8)
PlayerFakes(13-314)PlayerResponseModel(222-238)PlayerResponseModel(264-282)PlayerResponseModel(310-313)List(15-171)List(284-299)Player(173-180)Player(182-198)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs (4)
src/Dotnet.Samples.AspNetCore.WebApi/Data/PlayerData.cs (2)
List(9-165)List(173-331)src/Dotnet.Samples.AspNetCore.WebApi/Models/Player.cs (1)
Player(11-34)src/Dotnet.Samples.AspNetCore.WebApi/Models/PlayerRequestModel.cs (1)
PlayerRequestModel(13-32)src/Dotnet.Samples.AspNetCore.WebApi/Models/PlayerResponseModel.cs (1)
PlayerResponseModel(11-28)
🪛 GitHub Check: Codeac Code Quality
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs
[warning] 287-299: CodeDuplication
This block of 12 lines is too similar to test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs:226
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Codacy Security Scan
🔇 Additional comments (27)
src/Dotnet.Samples.AspNetCore.WebApi/Data/PlayerData.cs (1)
9-9: Consistent method naming looks good.The renaming from
CreateStarting11toMakeStarting11is a good refactoring that establishes a more consistent naming convention for factory methods across the codebase.src/Dotnet.Samples.AspNetCore.WebApi/Utilities/ApplicationBuilderExtensions.cs (1)
24-24: Updated method reference correctly.The reference to
PlayerData.MakeStarting11()has been correctly updated to match the renamed method in the PlayerData class.src/Dotnet.Samples.AspNetCore.WebApi/Utilities/DbContextUtils.cs (1)
38-38: Method reference updated properly.The call to
PlayerData.MakeStarting11()has been updated correctly to align with the renamed method.test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/DatabaseFakes.cs (1)
51-51: Method reference updated to use consistent naming.The change from
PlayerFakes.GetStarting11()toPlayerFakes.MakeStarting11()correctly aligns with the standardized naming convention using the "Make" prefix instead of "Get" for factory methods.test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs (9)
30-30: Variable renaming follows more idiomatic conventionThe variable name
requestis more descriptive of its purpose than the previous name, making it clear that this is a request object being passed to the controller.
59-63: Improved result assertion variable namingChanging from
responsetohttpResultmakes the code clearer by distinguishing between the HTTP result object and the response model content. This makes assertions more readable and aligns with ASP.NET Core minimal API patterns.
71-73: Consistent naming of mock setup variablesThe renaming from
payloadtorequestand updated response model creation method names provide better consistency throughout the test class, making the intent clearer.
114-117: More explicit test data setupUsing explicit ID value and the new fake methods improves test readability by clearly showing the relationship between test data objects.
150-157: Improved type validation in assertionsThe assertion is now correctly checking for
Created<PlayerResponseModel>instead of using a genericPlayerRequestModel, which more accurately reflects the expected return type from the controller method.
169-171: Using new response model creation methodThe change to
MakeResponseModelsForRetrieve()helps maintain consistency with the naming convention changes while ensuring the test behavior remains the same.
325-327: More explicit request setup for validation testUsing the new fake method and explicit invalidation of the squad number improves test readability by clearly showing what's being tested.
411-415: More comprehensive request model setupUsing the fake methods and explicit property assignments provides better clarity about what's being modified in the update test scenario.
435-439: Consistent HTTP result assertion patternThe renaming to
httpResultcontinues the pattern established throughout the class, improving readability and maintainability.test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerServiceTests.cs (7)
28-31: Using more idiomatic fake methodsChanged variable setup to use the renamed methods
MakeRequestModelForCreate()andMakeResponseModelForCreate(), which improves code readability and aligns with the standardized "Make" prefix convention.
61-66: Updated variable definitions with standardized method namesThe service test now correctly uses the renamed methods
MakeStarting11()andMakeResponseModelsForRetrieve(), maintaining test functionality while improving naming consistency.
131-133: Using explicit ID values instead of generic matchersUsing a specific ID value (999) makes the test more concrete and easier to understand compared to using
It.IsAny<long>(), which aligns with the goal of writing more idiomatic tests.
156-161: More specific test data initializationUsing explicit ID reference and the new
MakeFromStarting11ById(id)method creates a clearer relationship between the test data objects, improving test readability.
215-218: Better relationship between player ID and squad numberBy first retrieving a specific player by ID and then using that player's squad number, the test more accurately reflects real-world usage patterns and maintains data consistency.
254-257: Using more concrete and descriptive test dataThe test now uses specific player ID and request model values rather than generic ones, making the test more readable and maintainable.
289-294: More explicit test arrangementUsing a specific player ID and retrieving a player with the new helper method makes the test setup more clear and aligns with the new naming convention.
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs (7)
15-15: Renamed method follows consistent naming conventionChanged from
GetStarting11()toMakeStarting11()to align with the standardized "Make" prefix convention used throughout the class.
173-180: Added utility method to retrieve players by IDThis new method
MakeFromStarting11ById(long id)reduces code duplication by providing a standard way to retrieve a player by ID from the starting eleven list.
182-182: Renamed method for creating a new player instanceChanged from
CreateOneNew()toMakeNew()to align with the standardized "Make" prefix convention.
200-203: Added section comments for better code organizationThe addition of section comments helps organize the code by operation type (Create, Retrieve, Update), making it easier to navigate and understand.
204-220: Renamed request model creation methodChanged from
CreateRequestModelForOneNew()toMakeRequestModelForCreate()to align with the standardized "Make" prefix convention while maintaining the same functionality.
222-238: Renamed response model creation methodChanged from
CreateResponseModelForOneNew()toMakeResponseModelForCreate()to align with the standardized "Make" prefix convention while maintaining the same functionality.🧰 Tools
🪛 GitHub Check: Codeac Code Quality
[warning] 226-238: CodeDuplication
This block of 12 lines is too similar to test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs:287
305-313: Added update model creation methodsThe new methods
MakeRequestModelForUpdate(long id)andMakeResponseModelForUpdate(long id)provide a clean way to reuse the retrieve methods for update operations, maintaining consistency and reducing code duplication.
Summary by CodeRabbit
These changes do not affect functionality for end-users but enhance the application's internal quality and test reliability.