Validator test gaps and naming convention alignment

[nanotaboada]

Description

A test coverage audit of PlayerValidatorTests.cs identified two missing test cases and one naming inconsistency introduced in #424. All three items are in the same file and can be addressed together.

1. Rename: fix 4-segment test name (naming convention)

The test naming convention for services and validators is {MethodName}_{StateUnderTest}_{ExpectedBehavior} (3 underscore-delimited segments). One test added in #424 breaks this:

Current (4 segments)	Corrected (3 segments)
ValidateAsync_SquadNumber_BelongsToPlayerBeingUpdated_ReturnsNoErrors	ValidateAsync_SquadNumberBelongsToPlayerBeingUpdated_ReturnsNoErrors

2. Missing test: GreaterThan(0) on SquadNumber is never exercised

Suggested name: ValidateAsync_SquadNumberNegative_ReturnsValidationError

The existing ValidateAsync_SquadNumberNotGreaterThanZero_ReturnsValidationError uses SquadNumber = 0, which fails the NotEmpty() rule (FluentValidation treats the int default 0 as empty). The GreaterThan(0) rule is never specifically reached because NotEmpty() fires first.

A negative value (e.g. -5) passes NotEmpty() but fails GreaterThan(0), producing the message "SquadNumber must be greater than 0." — a different rule and a different message path, currently untested.

[Fact]
[Trait("Category", "Unit")]
public async Task ValidateAsync_SquadNumberNegative_ReturnsValidationError()
{
    // Arrange
    var request = PlayerFakes.MakeRequestModelForCreate();
    request.SquadNumber = -5;
    var validator = CreateValidator();

    // Act
    var result = await validator.ValidateAsync(
        request,
        options => options.IncludeRuleSets("Create")
    );

    // Assert
    result.IsValid.Should().BeFalse();
    result.Errors.Should().Contain(error =>
        error.PropertyName == "SquadNumber" &&
        error.ErrorMessage.Contains("greater than 0"));
}

3. Missing test: "Update" rule set has no structural validation test

Suggested name: ValidateAsync_FirstNameEmptyInUpdateRuleSet_ReturnsValidationError

The only test covering the "Update" rule set is ValidateAsync_SquadNumberBelongsToPlayerBeingUpdated_ReturnsNoErrors (after renaming above), which only exercises the happy path. No test verifies that the "Update" rule set actually enforces structural fields. If the rule set were accidentally emptied, no existing test would catch it.

[Fact]
[Trait("Category", "Unit")]
public async Task ValidateAsync_FirstNameEmptyInUpdateRuleSet_ReturnsValidationError()
{
    // Arrange
    var request = PlayerFakes.MakeRequestModelForUpdate(10);
    request.FirstName = string.Empty;
    var validator = CreateValidator();

    // Act
    var result = await validator.ValidateAsync(
        request,
        options => options.IncludeRuleSets("Update")
    );

    // Assert
    result.IsValid.Should().BeFalse();
    result.Errors.Should().Contain(error => error.PropertyName == "FirstName");
}

Audit scope

The full audit covered PlayerControllerTests, PlayerServiceTests, and PlayerValidatorTests against all production branches in their respective layers. Controller and service suites were found to be complete. Other validator gaps considered (date boundary tests, multiple validation errors in one request) were reviewed and ruled redundant or out of scope.

Possible Solution

All changes are confined to PlayerValidatorTests.cs:

1. Rename the offending test method
2. Add ValidateAsync_SquadNumberNegative_ReturnsValidationError in the SquadNumber section
3. Add ValidateAsync_FirstNameEmptyInUpdateRuleSet_ReturnsValidationError alongside ValidateAsync_SquadNumberBelongsToPlayerBeingUpdated_ReturnsNoErrors

[coderabbitai]

🔗 Related PRs

#397 - feat(validation): add DateOfBirth rules and fix Birth null mapping [merged]
#399 - refactor(tests): rename test methods to Microsoft naming standard [merged]
#420 - refactor(api)!: add /squadNumber/ path segment to routes [merged]
#426 - fix(validators): scope BeUniqueSquadNumber to "Create" rule set (#424) [merged]
#429 - docs: sync README, CHANGELOG, and CodeRabbit config with current codebase state [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

✅ Successfully generated implementation plan.

💡 Use @coderabbitai <feedback> to replan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[coderabbitai]

Coding Plan

Summary

• All changes are isolated to PlayerValidatorTests.cs with no production code modifications
• Follow existing test patterns: same attributes, assertion style, helper method usage, and section organization
• The rename fixes naming convention compliance; the two new tests close coverage gaps for distinct validation code paths (negative SquadNumber → GreaterThan rule; Update rule set → structural field enforcement)

Implementation Steps

Update PlayerValidatorTests with Naming Fix and New Test Cases

This phase addresses all three items identified in the audit: correcting the naming convention violation and closing two test coverage gaps. All changes are confined to a single test file and follow existing patterns established in the test suite.

Task 1: Rename Test Method to Follow Naming Convention

Correct the 4-segment test name to align with the project's 3-segment naming convention.

• In test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cs, rename ValidateAsync_SquadNumber_BelongsToPlayerBeingUpdated_ReturnsNoErrors to ValidateAsync_SquadNumberBelongsToPlayerBeingUpdated_ReturnsNoErrors
• This is a pure rename with no logic changes; the test body remains unchanged
• The corrected name combines "SquadNumberBelongsToPlayerBeingUpdated" into the second segment, matching the convention used by all other tests in the file

Task 2: Add Test for GreaterThan(0) Validation Rule

Add a test that specifically exercises the GreaterThan(0) rule on SquadNumber, which is currently never reached by existing tests.

• Add new test method ValidateAsync_SquadNumberNegative_ReturnsValidationError in the SquadNumber section of PlayerValidatorTests.cs
• Use PlayerFakes.MakeRequestModelForCreate() and set SquadNumber = -5 (or any negative value)
• Validate against the "Create" rule set
• Assert that validation fails and errors contain a SquadNumber error with message containing "greater than 0"
• Place this test near the existing ValidateAsync_SquadNumberNotGreaterThanZero_ReturnsValidationError test since both relate to SquadNumber value constraints

Task 3: Add Test for Update Rule Set Structural Validation

Add a test that verifies the Update rule set enforces field-level validation, providing regression protection if the rule set were accidentally emptied.

• Add new test method ValidateAsync_FirstNameEmptyInUpdateRuleSet_ReturnsValidationError in PlayerValidatorTests.cs
• Use PlayerFakes.MakeRequestModelForUpdate(10) to get a valid update request, then set FirstName = string.Empty
• Validate against the "Update" rule set (not "Create")
• Assert that validation fails and errors contain a FirstName property error
• Place this test alongside the renamed ValidateAsync_SquadNumberBelongsToPlayerBeingUpdated_ReturnsNoErrors since both exercise the Update rule set

🤖 Prompt for AI agents

This phase makes all changes exclusively in
`test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cs`. The
goal is to fix a naming convention violation and add two new test cases to close
coverage gaps — with no production code changes.

**Task 1: Rename test method**
- Locate the method named
`ValidateAsync_SquadNumber_BelongsToPlayerBeingUpdated_ReturnsNoErrors` and
rename it to
`ValidateAsync_SquadNumberBelongsToPlayerBeingUpdated_ReturnsNoErrors`
- Do not change the test body or any other code — this is a pure rename

**Task 2: Add `ValidateAsync_SquadNumberNegative_ReturnsValidationError`**
- Add this new `[Fact]` `[Trait("Category", "Unit")]` test method in the
SquadNumber section, near the existing
`ValidateAsync_SquadNumberNotGreaterThanZero_ReturnsValidationError` test
- Use `PlayerFakes.MakeRequestModelForCreate()` to get a request model and set
`SquadNumber = -5` (or any negative value)
- Validate using the `"Create"` rule set via the `CreateValidator()` factory
method
- Assert that validation fails and that the errors collection contains a
SquadNumber property error with a message containing `"greater than 0"`
- Follow the same assertion style (FluentAssertions) used throughout the file

**Task 3: Add
`ValidateAsync_FirstNameEmptyInUpdateRuleSet_ReturnsValidationError`**
- Add this new `[Fact]` `[Trait("Category", "Unit")]` test method in the file,
placed alongside the renamed
`ValidateAsync_SquadNumberBelongsToPlayerBeingUpdated_ReturnsNoErrors` test
since both exercise the Update rule set
- Use `PlayerFakes.MakeRequestModelForUpdate(10)` to get a valid update request
model, then set `FirstName = string.Empty`
- Validate using the `"Update"` rule set via the `CreateValidator()` factory
method
- Assert that validation fails and that the errors collection contains a
FirstName property error
- Follow the same assertion style (FluentAssertions) and attribute decorations
used throughout the file

Research

The project follows a standard .NET Web API structure with tests in test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/. The PlayerValidatorTests.cs file contains unit tests for PlayerRequestModelValidator, organized into comment-delimited sections (SquadNumber, FirstName, DateOfBirth, etc.). Tests follow a strict {MethodName}_{StateUnderTest}_{ExpectedBehavior} naming convention with 3 underscore-delimited segments. The test class uses PlayerFakes helper methods for test data, a CreateValidator() factory method, and FluentAssertions for assertions. All tests are decorated with [Fact] and [Trait("Category", "Unit")].

---

🚀 Next Steps

🤖 All AI agent prompts combined

This phase makes all changes exclusively in
`test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cs`. The
goal is to fix a naming convention violation and add two new test cases to close
coverage gaps — with no production code changes.

**Task 1: Rename test method**
- Locate the method named
`ValidateAsync_SquadNumber_BelongsToPlayerBeingUpdated_ReturnsNoErrors` and
rename it to
`ValidateAsync_SquadNumberBelongsToPlayerBeingUpdated_ReturnsNoErrors`
- Do not change the test body or any other code — this is a pure rename

**Task 2: Add `ValidateAsync_SquadNumberNegative_ReturnsValidationError`**
- Add this new `[Fact]` `[Trait("Category", "Unit")]` test method in the
SquadNumber section, near the existing
`ValidateAsync_SquadNumberNotGreaterThanZero_ReturnsValidationError` test
- Use `PlayerFakes.MakeRequestModelForCreate()` to get a request model and set
`SquadNumber = -5` (or any negative value)
- Validate using the `"Create"` rule set via the `CreateValidator()` factory
method
- Assert that validation fails and that the errors collection contains a
SquadNumber property error with a message containing `"greater than 0"`
- Follow the same assertion style (FluentAssertions) used throughout the file

**Task 3: Add
`ValidateAsync_FirstNameEmptyInUpdateRuleSet_ReturnsValidationError`**
- Add this new `[Fact]` `[Trait("Category", "Unit")]` test method in the file,
placed alongside the renamed
`ValidateAsync_SquadNumberBelongsToPlayerBeingUpdated_ReturnsNoErrors` test
since both exercise the Update rule set
- Use `PlayerFakes.MakeRequestModelForUpdate(10)` to get a valid update request
model, then set `FirstName = string.Empty`
- Validate using the `"Update"` rule set via the `CreateValidator()` factory
method
- Assert that validation fails and that the errors collection contains a
FirstName property error
- Follow the same assertion style (FluentAssertions) and attribute decorations
used throughout the file

💡 Iterate on the plan with: @coderabbitai <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!