Merge pull request #399 from nanotaboada/refactor/rename-test-methods

refactor(tests): rename test methods to Microsoft naming standard

Author: nanotaboada

.github/copilot-instructions.md

@@ -46,7 +46,9 @@ test/Dotnet.Samples.AspNetCore.WebApi.Tests/
 - **Reads**: Use `AsNoTracking()` for all EF Core read queries
 - **Errors**: RFC 7807 Problem Details for all error responses
 - **Logging**: Structured logging via `ILogger<T>`; never `Console.Write`
-- **Tests**: xUnit + Moq + FluentAssertions; test naming mirrors method under test
+- **Tests**: xUnit + Moq + FluentAssertions; naming convention per layer:
+  - Controller: `{HttpMethod}_{Resource}_{Condition}_Returns{Outcome}` (e.g. `Get_Players_Existing_ReturnsPlayers`)
+  - Service / Validator: `{MethodName}_{StateUnderTest}_{ExpectedBehavior}` (e.g. `RetrieveAsync_CacheMiss_QueriesRepositoryAndCachesResult`)
 - **Avoid**: synchronous EF Core APIs, controller business logic, static service/repository classes
 
 ## Commands

CHANGELOG.md

@@ -46,12 +46,18 @@ This project uses famous football stadiums (A-Z) that hosted FIFA World Cup matc
 
 ### Changed
 
+- Rename test methods to follow Microsoft .NET naming standard (#396)
+
 ### Deprecated
 
 ### Removed
 
 ### Fixed
 
+- Fix broken 201 assertion in controller test for `Post_Players_NonExisting` (#396)
+- Add missing edge case tests for `UpdateAsync`, `DeleteAsync`, and
+  `DateOfBirth` boundary validation (#396)
+
 ### Security
 
 ---

src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs

@@ -18,9 +18,13 @@ public class PlayerRequestModelValidator : AbstractValidator<PlayerRequestModel>
 {
     private readonly IPlayerRepository _playerRepository;
 
-    public PlayerRequestModelValidator(IPlayerRepository playerRepository)
+    public PlayerRequestModelValidator(
+        IPlayerRepository playerRepository,
+        TimeProvider? timeProvider = null
+    )
     {
         _playerRepository = playerRepository;
+        var clock = timeProvider ?? TimeProvider.System;
 
         RuleFor(player => player.FirstName).NotEmpty().WithMessage("FirstName is required.");
 
@@ -45,7 +49,7 @@ public PlayerRequestModelValidator(IPlayerRepository playerRepository)
             () =>
             {
                 RuleFor(player => player.DateOfBirth)
-                    .Must(date => date!.Value.Date < DateTime.UtcNow.Date)
+                    .Must(date => date!.Value.Date < clock.GetUtcNow().Date)
                     .WithMessage("DateOfBirth must be a date in the past.")
                     .Must(date =>
                         date!.Value.Date >= new DateTime(1900, 1, 1, 0, 0, 0, DateTimeKind.Utc)

test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs

@@ -24,7 +24,7 @@ public PlayerControllerTests()
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenPostAsync_WhenValidatorReturnsErrors_ThenResponseStatusCodeShouldBe400BadRequest()
+    public async Task Post_Players_ValidationError_Returns400BadRequest()
     {
         // Arrange
         var request = PlayerFakes.MakeRequestModelForCreate();
@@ -65,7 +65,7 @@ public async Task GivenPostAsync_WhenValidatorReturnsErrors_ThenResponseStatusCo
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenPostAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsPlayer_ThenResponseStatusCodeShouldBe409Conflict()
+    public async Task Post_Players_Existing_Returns409Conflict()
     {
         // Arrange
         var request = PlayerFakes.MakeRequestModelForCreate();
@@ -108,7 +108,7 @@ public async Task GivenPostAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsPla
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenPostAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsNull_ThenResponseStatusCodeShouldBe201Created()
+    public async Task Post_Players_NonExisting_Returns201Created()
     {
         // Arrange
         var request = PlayerFakes.MakeRequestModelForCreate();
@@ -146,14 +146,12 @@ public async Task GivenPostAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsNul
                 ),
             Times.Once
         );
-        if (result is CreatedAtRoute<PlayerRequestModel> httpResult)
-        {
-            httpResult.Should().NotBeNull().And.BeOfType<Created<PlayerResponseModel>>();
-            httpResult.StatusCode.Should().Be(StatusCodes.Status201Created);
-            httpResult.Value.Should().BeEquivalentTo(response);
-            httpResult.RouteName.Should().Be("GetById");
-            httpResult.RouteValues.Should().NotBeNull().And.ContainKey("id");
-        }
+        var httpResult = result.Should().BeOfType<CreatedAtRoute<PlayerResponseModel>>().Subject;
+        httpResult.StatusCode.Should().Be(StatusCodes.Status201Created);
+        httpResult.Value.Should().BeEquivalentTo(response);
+        httpResult.RouteName.Should().Be("RetrieveBySquadNumber");
+        httpResult.RouteValues.Should().NotBeNull().And.ContainKey("squadNumber");
+        httpResult.RouteValues!["squadNumber"].Should().Be(response.Dorsal);
     }
 
     /* -------------------------------------------------------------------------
@@ -162,7 +160,7 @@ public async Task GivenPostAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsNul
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenGetAsync_WhenServiceRetrieveAsyncReturnsListOfPlayers_ThenResponseShouldBeEquivalentToListOfPlayers()
+    public async Task Get_Players_Existing_ReturnsPlayers()
     {
         // Arrange
         var response = PlayerFakes.MakeResponseModelsForRetrieve();
@@ -187,7 +185,7 @@ public async Task GivenGetAsync_WhenServiceRetrieveAsyncReturnsListOfPlayers_The
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenGetAsync_WhenServiceRetrieveAsyncReturnsEmptyList_ThenResponseStatusCodeShouldBe404NotFound()
+    public async Task Get_Players_NonExisting_Returns404NotFound()
     {
         // Arrange
         var (service, logger, validator) = PlayerMocks.InitControllerMocks();
@@ -209,7 +207,7 @@ public async Task GivenGetAsync_WhenServiceRetrieveAsyncReturnsEmptyList_ThenRes
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenGetByIdAsync_WhenServiceRetrieveByIdAsyncReturnsNull_ThenResponseStatusCodeShouldBe404NotFound()
+    public async Task Get_PlayerById_NonExisting_Returns404NotFound()
     {
         // Arrange
         var id = Guid.NewGuid();
@@ -234,7 +232,7 @@ public async Task GivenGetByIdAsync_WhenServiceRetrieveByIdAsyncReturnsNull_Then
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenGetByIdAsync_WhenServiceRetrieveByIdAsyncReturnsPlayer_ThenResponseStatusCodeShouldBe200Ok()
+    public async Task Get_PlayerById_Existing_Returns200OK()
     {
         // Arrange
         var id = Guid.NewGuid();
@@ -260,7 +258,7 @@ public async Task GivenGetByIdAsync_WhenServiceRetrieveByIdAsyncReturnsPlayer_Th
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenGetBySquadNumberAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsNull_ThenResponseStatusCodeShouldBe404NotFound()
+    public async Task Get_PlayerBySquadNumber_NonExisting_Returns404NotFound()
     {
         // Arrange
         var squadNumber = 999;
@@ -285,7 +283,7 @@ public async Task GivenGetBySquadNumberAsync_WhenServiceRetrieveBySquadNumberAsy
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenGetBySquadNumberAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsPlayer_ThenResponseStatusCodeShouldBe200Ok()
+    public async Task Get_PlayerBySquadNumber_Existing_Returns200OK()
     {
         // Arrange
         var squadNumber = 10;
@@ -317,7 +315,7 @@ public async Task GivenGetBySquadNumberAsync_WhenServiceRetrieveBySquadNumberAsy
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenPutAsync_WhenValidatorReturnsErrors_ThenResponseStatusCodeShouldBe400BadRequest()
+    public async Task Put_PlayerBySquadNumber_ValidationError_Returns400BadRequest()
     {
         // Arrange
         var squadNumber = 20;
@@ -361,7 +359,7 @@ public async Task GivenPutAsync_WhenValidatorReturnsErrors_ThenResponseStatusCod
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenPutAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsNull_ThenResponseStatusCodeShouldBe404NotFound()
+    public async Task Put_PlayerBySquadNumber_NonExisting_Returns404NotFound()
     {
         // Arrange
         var squadNumber = 999;
@@ -403,7 +401,7 @@ public async Task GivenPutAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsNull
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenPutAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsPlayer_ThenResponseStatusCodeShouldBe204NoContent()
+    public async Task Put_PlayerBySquadNumber_Existing_Returns204NoContent()
     {
         // Arrange
         var squadNumber = 23;
@@ -446,7 +444,7 @@ public async Task GivenPutAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsPlay
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenDeleteAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsNull_ThenResponseStatusCodeShouldBe404NotFound()
+    public async Task Delete_PlayerBySquadNumber_NonExisting_Returns404NotFound()
     {
         // Arrange
         var squadNumber = 999;
@@ -472,7 +470,7 @@ public async Task GivenDeleteAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsN
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenDeleteAsync_WhenServiceRetrieveBySquadNumberAsyncReturnsPlayer_ThenResponseStatusCodeShouldBe204NoContent()
+    public async Task Delete_PlayerBySquadNumber_Existing_Returns204NoContent()
     {
         // Arrange
         var squadNumber = 26;

test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerServiceTests.cs

@@ -15,7 +15,7 @@ public class PlayerServiceTests
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenCreateAsync_WhenRepositoryAddAsync_ThenAddsPlayerToRepositoryAndRemovesCache()
+    public async Task CreateAsync_WhenCalled_AddsPlayerAndRemovesCache()
     {
         // Arrange
         var request = PlayerFakes.MakeRequestModelForCreate();
@@ -49,7 +49,7 @@ public async Task GivenCreateAsync_WhenRepositoryAddAsync_ThenAddsPlayerToReposi
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenRetrieveAsync_WhenRepositoryGetAllAsyncReturnsPlayers_ThenCacheCreateEntryAndResultShouldBeListOfPlayers()
+    public async Task RetrieveAsync_CacheMiss_QueriesRepositoryAndCachesResult()
     {
         // Arrange
         var value = It.IsAny<object>();
@@ -83,7 +83,7 @@ public async Task GivenRetrieveAsync_WhenRepositoryGetAllAsyncReturnsPlayers_The
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenRetrieveAsync_WhenExecutedForTheSecondTime_ThenSecondExecutionTimeShouldBeLessThanFirst()
+    public async Task RetrieveAsync_CacheHit_ReturnsBeforeQueryingRepository()
     {
         // Arrange
         var value = It.IsAny<object>();
@@ -121,7 +121,7 @@ public async Task GivenRetrieveAsync_WhenExecutedForTheSecondTime_ThenSecondExec
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenRetrieveByIdAsync_WhenRepositoryFindByIdAsyncReturnsNull_TheResultShouldBeNull()
+    public async Task RetrieveByIdAsync_PlayerNotFound_ReturnsNull()
     {
         // Arrange
         var id = Guid.NewGuid();
@@ -147,7 +147,7 @@ public async Task GivenRetrieveByIdAsync_WhenRepositoryFindByIdAsyncReturnsNull_
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenRetrieveByIdAsync_WhenRepositoryFindByIdAsyncReturnsPlayer_TheResultShouldBePlayer()
+    public async Task RetrieveByIdAsync_PlayerFound_ReturnsMappedResponseModel()
     {
         // Arrange
         var id = Guid.NewGuid();
@@ -178,7 +178,7 @@ public async Task GivenRetrieveByIdAsync_WhenRepositoryFindByIdAsyncReturnsPlaye
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenRetrieveBySquadNumberAsync_WhenRepositoryFindBySquadNumberAsyncReturnsNull_ThenResultShouldBeNull()
+    public async Task RetrieveBySquadNumberAsync_PlayerNotFound_ReturnsNull()
     {
         // Arrange
         var squadNumber = 999;
@@ -209,7 +209,7 @@ public async Task GivenRetrieveBySquadNumberAsync_WhenRepositoryFindBySquadNumbe
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenRetrieveBySquadNumberAsync_WhenRepositoryFindBySquadNumberAsyncReturnsPlayer_ThenResultShouldBePlayer()
+    public async Task RetrieveBySquadNumberAsync_PlayerFound_ReturnsMappedResponseModel()
     {
         // Arrange
         var squadNumber = 10;
@@ -248,7 +248,7 @@ public async Task GivenRetrieveBySquadNumberAsync_WhenRepositoryFindBySquadNumbe
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenUpdateAsync_WhenRepositoryFindBySquadNumberAsyncReturnsPlayer_ThenRepositoryUpdateAsyncAndCacheRemove()
+    public async Task UpdateAsync_PlayerFound_UpdatesRepositoryAndRemovesCache()
     {
         // Arrange
         var squadNumber = 23;
@@ -283,13 +283,50 @@ public async Task GivenUpdateAsync_WhenRepositoryFindBySquadNumberAsyncReturnsPl
         );
     }
 
+    [Fact]
+    [Trait("Category", "Unit")]
+    public async Task UpdateAsync_PlayerNotFound_DoesNotUpdateRepository()
+    {
+        // Arrange
+        var squadNumber = 999;
+        var request = PlayerFakes.MakeRequestModelForCreate();
+        request.SquadNumber = squadNumber;
+        var (repository, logger, memoryCache, mapper, environment) = PlayerMocks.InitServiceMocks();
+        repository
+            .Setup(repository => repository.FindBySquadNumberAsync(squadNumber))
+            .ReturnsAsync(null as Player);
+
+        var service = new PlayerService(
+            repository.Object,
+            logger.Object,
+            memoryCache.Object,
+            mapper.Object,
+            environment.Object
+        );
+
+        // Act
+        await service.UpdateAsync(request);
+
+        // Assert
+        repository.Verify(
+            repository => repository.FindBySquadNumberAsync(It.IsAny<int>()),
+            Times.Once
+        );
+        repository.Verify(repository => repository.UpdateAsync(It.IsAny<Player>()), Times.Never);
+        memoryCache.Verify(cache => cache.Remove(It.IsAny<object>()), Times.Never);
+        mapper.Verify(
+            mapper => mapper.Map(It.IsAny<PlayerRequestModel>(), It.IsAny<Player>()),
+            Times.Never
+        );
+    }
+
     /* -------------------------------------------------------------------------
      * Delete
      * ---------------------------------------------------------------------- */
 
     [Fact]
     [Trait("Category", "Unit")]
-    public async Task GivenDeleteAsync_WhenRepositoryFindBySquadNumberAsyncReturnsPlayer_ThenRepositoryDeleteAsyncAndCacheRemove()
+    public async Task DeleteAsync_PlayerFound_RemovesFromRepositoryAndRemovesCache()
     {
         // Arrange
         var squadNumber = 26;
@@ -319,6 +356,37 @@ public async Task GivenDeleteAsync_WhenRepositoryFindBySquadNumberAsyncReturnsPl
         memoryCache.Verify(cache => cache.Remove(It.IsAny<object>()), Times.Once);
     }
 
+    [Fact]
+    [Trait("Category", "Unit")]
+    public async Task DeleteAsync_PlayerNotFound_DoesNotRemoveFromRepository()
+    {
+        // Arrange
+        var squadNumber = 999;
+        var (repository, logger, memoryCache, mapper, environment) = PlayerMocks.InitServiceMocks();
+        repository
+            .Setup(repository => repository.FindBySquadNumberAsync(squadNumber))
+            .ReturnsAsync(null as Player);
+
+        var service = new PlayerService(
+            repository.Object,
+            logger.Object,
+            memoryCache.Object,
+            mapper.Object,
+            environment.Object
+        );
+
+        // Act
+        await service.DeleteAsync(squadNumber);
+
+        // Assert
+        repository.Verify(
+            repository => repository.FindBySquadNumberAsync(It.IsAny<int>()),
+            Times.Once
+        );
+        repository.Verify(repository => repository.RemoveAsync(It.IsAny<Guid>()), Times.Never);
+        memoryCache.Verify(cache => cache.Remove(It.IsAny<object>()), Times.Never);
+    }
+
     private static async Task<long> ExecutionTimeAsync(Func<Task> awaitable)
     {
         var stopwatch = new Stopwatch();