Skip to content

Add unit tests for UsersController API endpoints #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Add unit tests for UsersController API endpoints #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ad2ac30
Initial plan
Copilot Dec 10, 2025
1581fbd
Add xUnit test project with comprehensive UsersController tests
Copilot Dec 10, 2025
1c7cebc
Address code review feedback: improve null handling in tests
Copilot Dec 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 188 additions & 0 deletions net-users-api.Tests/Controllers/UsersControllerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Logging;
using Moq;
using NetUsersApi.Controllers;
using NetUsersApi.Models;

namespace NetUsersApi.Tests.Controllers;

public class UsersControllerTests
{
private readonly Mock<ILogger<UsersController>> _mockLogger;
private readonly UsersController _controller;

public UsersControllerTests()
{
_mockLogger = new Mock<ILogger<UsersController>>();
_controller = new UsersController(_mockLogger.Object);
}

Comment on lines +17 to +19

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test setup/teardown: The test class should implement IDisposable or use a [Fact] setup method to reset the static _users list in UsersController to its original state before each test. This would prevent test isolation issues caused by shared mutable state.

Example:

public UsersControllerTests()
{
    _mockLogger = new Mock<ILogger<UsersController>>();
    _controller = new UsersController(_mockLogger.Object);
    ResetUserData();
}

private void ResetUserData()
{
    // Reset static data to original state
    UsersController._users = new List<UserProfile>
    {
        new UserProfile { Id = "1", FullName = "John Doe", Emoji = "😀" },
        new UserProfile { Id = "2", FullName = "Jane Smith", Emoji = "🚀" },
        new UserProfile { Id = "3", FullName = "Robert Johnson", Emoji = "🎸" }
    };
}

Note: This would require making _users accessible (currently it's private).

Suggested change
_controller = new UsersController(_mockLogger.Object);
}
_controller = new UsersController(_mockLogger.Object);
ResetUserData();
}
private void ResetUserData()
{
// Reset static data to original state
UsersController._users = new List<UserProfile>
{
new UserProfile { Id = "1", FullName = "John Doe", Emoji = "😀" },
new UserProfile { Id = "2", FullName = "Jane Smith", Emoji = "🚀" },
new UserProfile { Id = "3", FullName = "Robert Johnson", Emoji = "🎸" }
};
}

Copilot uses AI. Check for mistakes.
[Fact]
public void GetUsers_ReturnsAllUsers()
{
// Arrange
// The controller uses static data, so we can just call it

// Act
var result = _controller.GetUsers();

// Assert
var okResult = Assert.IsType<OkObjectResult>(result.Result);
var users = Assert.IsAssignableFrom<IEnumerable<UserProfile>>(okResult.Value);
Assert.NotEmpty(users);
}
Comment on lines +20 to +33

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test isolation issue: This test depends on shared static state in UsersController._users. Since tests may run in parallel or in different orders, this test could fail if other tests have modified the static user list. The test should either:

  1. Reset the static list to a known state before each test (e.g., in a setup method)
  2. Not assume a specific size or content, only that the list is not empty

Currently, if CreateUser_ValidUser_ReturnsCreatedAtAction runs before this test, the list will contain more than the original 3 users, but the test only verifies NotEmpty.

Copilot uses AI. Check for mistakes.

[Theory]
[InlineData("1")]
[InlineData("2")]
[InlineData("3")]
public void GetUser_ValidId_ReturnsUser(string id)
{
// Act
var result = _controller.GetUser(id);

// Assert
var okResult = Assert.IsType<OkObjectResult>(result.Result);
var user = Assert.IsType<UserProfile>(okResult.Value);
Assert.Equal(id, user.Id);
}
Comment on lines +35 to +48

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test case for empty/null ID: The GetUser method should have test coverage for edge cases like empty string ("") or whitespace-only IDs. These are valid string values but represent invalid user IDs that should return a 404.

Example:

[Theory]
[InlineData("")]
[InlineData(" ")]
[InlineData(null)]
public void GetUser_InvalidIdFormat_ReturnsNotFound(string id)
{
    var result = _controller.GetUser(id);
    Assert.IsType<NotFoundObjectResult>(result.Result);
}

Copilot uses AI. Check for mistakes.

[Fact]
public void GetUser_InvalidId_ReturnsNotFound()
{
// Arrange
var invalidId = "999";

// Act
var result = _controller.GetUser(invalidId);

// Assert
var notFoundResult = Assert.IsType<NotFoundObjectResult>(result.Result);
Assert.NotNull(notFoundResult.Value);
}

[Fact]
public void CreateUser_ValidUser_ReturnsCreatedAtAction()
{
// Arrange
var newUser = new UserProfile
{
Id = "100",
FullName = "Test User",
Emoji = "🧪"
};

// Act
var result = _controller.CreateUser(newUser);

// Assert
var createdResult = Assert.IsType<CreatedAtActionResult>(result.Result);
var returnedUser = Assert.IsType<UserProfile>(createdResult.Value);
Assert.Equal(newUser.Id, returnedUser.Id);
Assert.Equal(newUser.FullName, returnedUser.FullName);
Assert.Equal(newUser.Emoji, returnedUser.Emoji);
Assert.Equal(nameof(UsersController.GetUser), createdResult.ActionName);
}
Comment on lines +65 to +85

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test isolation issue: This test mutates the shared static _users list by adding a new user, which will affect all subsequent tests. Since tests may run in parallel or in different orders, this causes non-deterministic test failures. The test should either:

  1. Clean up by removing the added user after the test
  2. Use a setup method to reset the static list to a known state before each test
  3. Consider refactoring the controller to use dependency injection for the data store to allow proper mocking

Copilot uses AI. Check for mistakes.
Comment on lines +65 to +85

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test case for duplicate ID: The CreateUser method should have test coverage for attempting to create a user with an ID that already exists. While the current controller implementation doesn't explicitly check for duplicates, this is important behavior to validate (either as expected behavior or as a bug to fix later).

Example:

[Fact]
public void CreateUser_DuplicateId_AllowsDuplicate()
{
    // Arrange
    var duplicateUser = new UserProfile
    {
        Id = "1", // Already exists
        FullName = "Duplicate User",
        Emoji = "🔄"
    };

    // Act
    var result = _controller.CreateUser(duplicateUser);

    // Assert - documents current behavior
    var createdResult = Assert.IsType<CreatedAtActionResult>(result.Result);
}

Copilot uses AI. Check for mistakes.
Comment on lines +65 to +85

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test case for required properties validation: The CreateUser method should have test coverage for UserProfile objects with missing required properties (Id, FullName, Emoji). While these are marked as required in the model, the test should verify that attempting to create a user with incomplete data is properly handled.

Note: With C# 12's required keyword, the compiler prevents creating objects without required properties at compile time, so this may need to test the API's model binding behavior rather than the controller logic directly.

Copilot uses AI. Check for mistakes.

[Fact]
public void CreateUser_NullUser_ReturnsBadRequest()
{
// Arrange
UserProfile? nullUser = null;

// Act
var result = _controller.CreateUser(nullUser!);

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary null-forgiving operator: The null-forgiving operator (!) is used after explicitly setting nullUser to null. This suppresses a warning but doesn't add value since the test is specifically checking null handling. Consider removing the ! and addressing the compiler warning with a #pragma warning disable CS8625 if needed, or restructure the test to avoid the null assignment pattern.

Alternative approach:

[Fact]
public void CreateUser_NullUser_ReturnsBadRequest()
{
    // Act
#pragma warning disable CS8625
    var result = _controller.CreateUser(null);
#pragma warning restore CS8625

    // Assert
    var badRequestResult = Assert.IsType<BadRequestObjectResult>(result.Result);
    Assert.NotNull(badRequestResult.Value);
}
Suggested change
var result = _controller.CreateUser(nullUser!);
#pragma warning disable CS8625
var result = _controller.CreateUser(nullUser);
#pragma warning restore CS8625

Copilot uses AI. Check for mistakes.

// Assert
var badRequestResult = Assert.IsType<BadRequestObjectResult>(result.Result);
Assert.NotNull(badRequestResult.Value);
}

[Fact]
public void UpdateUser_ValidUser_ReturnsUpdatedUser()
{
// Arrange
var userId = "1";
var updatedUser = new UserProfile
{
Id = userId,
FullName = "Updated Name",
Emoji = "✨"
};

// Act
var result = _controller.UpdateUser(userId, updatedUser);

// Assert
var okResult = Assert.IsType<OkObjectResult>(result.Result);
var returnedUser = Assert.IsType<UserProfile>(okResult.Value);
Assert.Equal(userId, returnedUser.Id);
Assert.Equal(updatedUser.FullName, returnedUser.FullName);
Assert.Equal(updatedUser.Emoji, returnedUser.Emoji);
}
Comment on lines +102 to +122

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test isolation issue: This test mutates the shared static _users list by modifying user ID "1", which will affect all subsequent tests that depend on the original user data. This causes non-deterministic test failures when tests run in parallel or in different orders. The test should either:

  1. Restore the original user data after the test
  2. Use a setup method to reset the static list to a known state before each test
  3. Consider refactoring the controller to use dependency injection for the data store

Copilot uses AI. Check for mistakes.

[Fact]
public void UpdateUser_InvalidId_ReturnsNotFound()
{
// Arrange
var invalidId = "999";
var updatedUser = new UserProfile
{
Id = invalidId,
FullName = "Updated Name",
Emoji = "✨"
};

// Act
var result = _controller.UpdateUser(invalidId, updatedUser);

// Assert
Assert.IsType<NotFoundObjectResult>(result.Result);
}

[Fact]
public void UpdateUser_NullUser_ReturnsBadRequest()
{
// Arrange
var userId = "1";
UserProfile? nullUser = null;

// Act
var result = _controller.UpdateUser(userId, nullUser!);

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary null-forgiving operator: The null-forgiving operator (!) is used after explicitly setting nullUser to null. This suppresses a warning but doesn't add value since the test is specifically checking null handling. Consider removing the ! and addressing the compiler warning with a #pragma warning disable CS8625 if needed, or restructure the test to avoid the null assignment pattern.

Suggested change
var result = _controller.UpdateUser(userId, nullUser!);
var result = _controller.UpdateUser(userId, nullUser);

Copilot uses AI. Check for mistakes.

// Assert
var badRequestResult = Assert.IsType<BadRequestObjectResult>(result.Result);
Assert.NotNull(badRequestResult.Value);
}

[Fact]
public void UpdateUser_MismatchedId_UpdatesWithCorrectId()
{
// Arrange
var userId = "1";
var updatedUser = new UserProfile
{
Id = "999", // Different from the route parameter
FullName = "Updated Name",
Emoji = "✨"
};

// Act
var result = _controller.UpdateUser(userId, updatedUser);

// Assert
var okResult = Assert.IsType<OkObjectResult>(result.Result);
var returnedUser = Assert.IsType<UserProfile>(okResult.Value);
Assert.Equal(userId, returnedUser.Id); // Should use the route parameter ID
}
Comment on lines +159 to +177

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test isolation issue: This test mutates the shared static _users list by modifying user ID "1", which will affect all subsequent tests that depend on the original user data. This causes non-deterministic test failures when tests run in parallel or in different orders. The test should either:

  1. Restore the original user data after the test
  2. Use a setup method to reset the static list to a known state before each test
  3. Consider refactoring the controller to use dependency injection for the data store

Copilot uses AI. Check for mistakes.

[Fact]
public void DeleteUser_ThrowsNotImplementedException()
{
// Arrange
var userId = "1";

// Act & Assert
Assert.Throws<NotImplementedException>(() => _controller.DeleteUser(userId));
}
}
27 changes: 27 additions & 0 deletions net-users-api.Tests/net-users-api.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net9.0</TargetFramework>
<RootNamespace>NetUsersApi.Tests</RootNamespace>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<IsPackable>false</IsPackable>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="coverlet.collector" Version="6.0.2" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.12.0" />
<PackageReference Include="Moq" Version="4.20.72" />
<PackageReference Include="xunit" Version="2.9.2" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.8.2" />
</ItemGroup>

<ItemGroup>
<Using Include="Xunit" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\net-users-api\net-users-api.csproj" />
</ItemGroup>

</Project>
70 changes: 48 additions & 22 deletions net-users-demo.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,48 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 17
VisualStudioVersion = 17.0.31903.59
MinimumVisualStudioVersion = 10.0.40219.1
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "net-users-api", "net-users-api\net-users-api.csproj", "{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Release|Any CPU = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
EndGlobalSection
GlobalSection(ProjectConfigurationPlatforms) = postSolution
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Debug|Any CPU.Build.0 = Debug|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Release|Any CPU.ActiveCfg = Release|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
EndGlobal

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 17
VisualStudioVersion = 17.0.31903.59
MinimumVisualStudioVersion = 10.0.40219.1
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "net-users-api", "net-users-api\net-users-api.csproj", "{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "net-users-api.Tests", "net-users-api.Tests\net-users-api.Tests.csproj", "{4F388C16-F623-435E-904F-00E0C80D9F87}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Debug|x64 = Debug|x64
Debug|x86 = Debug|x86
Release|Any CPU = Release|Any CPU
Release|x64 = Release|x64
Release|x86 = Release|x86
EndGlobalSection
GlobalSection(ProjectConfigurationPlatforms) = postSolution
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Debug|Any CPU.Build.0 = Debug|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Debug|x64.ActiveCfg = Debug|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Debug|x64.Build.0 = Debug|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Debug|x86.ActiveCfg = Debug|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Debug|x86.Build.0 = Debug|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Release|Any CPU.ActiveCfg = Release|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Release|Any CPU.Build.0 = Release|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Release|x64.ActiveCfg = Release|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Release|x64.Build.0 = Release|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Release|x86.ActiveCfg = Release|Any CPU
{F2C0F837-02EA-42E5-BDC0-48C31DD4245D}.Release|x86.Build.0 = Release|Any CPU
{4F388C16-F623-435E-904F-00E0C80D9F87}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{4F388C16-F623-435E-904F-00E0C80D9F87}.Debug|Any CPU.Build.0 = Debug|Any CPU
{4F388C16-F623-435E-904F-00E0C80D9F87}.Debug|x64.ActiveCfg = Debug|Any CPU
{4F388C16-F623-435E-904F-00E0C80D9F87}.Debug|x64.Build.0 = Debug|Any CPU
{4F388C16-F623-435E-904F-00E0C80D9F87}.Debug|x86.ActiveCfg = Debug|Any CPU
{4F388C16-F623-435E-904F-00E0C80D9F87}.Debug|x86.Build.0 = Debug|Any CPU
{4F388C16-F623-435E-904F-00E0C80D9F87}.Release|Any CPU.ActiveCfg = Release|Any CPU
{4F388C16-F623-435E-904F-00E0C80D9F87}.Release|Any CPU.Build.0 = Release|Any CPU
{4F388C16-F623-435E-904F-00E0C80D9F87}.Release|x64.ActiveCfg = Release|Any CPU
{4F388C16-F623-435E-904F-00E0C80D9F87}.Release|x64.Build.0 = Release|Any CPU
{4F388C16-F623-435E-904F-00E0C80D9F87}.Release|x86.ActiveCfg = Release|Any CPU
{4F388C16-F623-435E-904F-00E0C80D9F87}.Release|x86.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
EndGlobalSection
EndGlobal