Skip to content

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

Merged
nanotaboada merged 2 commits intomasterfrom
refactor/rename-test-methods
Mar 8, 2026
Merged

refactor(tests): rename test methods to Microsoft naming standard#399
nanotaboada merged 2 commits intomasterfrom
refactor/rename-test-methods

Conversation

@nanotaboada
Copy link
Copy Markdown
Owner

@nanotaboada nanotaboada commented Mar 7, 2026

Closes #396

Summary by CodeRabbit

  • Tests

    • Restructured the unit test suite with standardized naming conventions to improve code clarity, consistency, and maintainability across all test classes.
    • Expanded validation test coverage by adding comprehensive edge case scenarios to ensure robust error handling.
  • Bug Fixes

    • Fixed incorrect HTTP response code assertion in the API's resource creation functionality.

- Apply {HttpMethod}_{Resource}_{Condition}_Returns{Outcome} to
  PlayerControllerTests (PascalCase per .NET conventions)
- Apply {MethodName}_{StateUnderTest}_{ExpectedBehavior} to
  PlayerServiceTests and PlayerValidatorTests
- Fix broken 201 assertion in Post_Players_NonExisting_Returns201Created
- Add UpdateAsync_PlayerNotFound and DeleteAsync_PlayerNotFound (service)
- Add DateOfBirthToday and DateOfBirthOnJanuary1st1900 boundary tests

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 7, 2026

Warning

Rate limit exceeded

@nanotaboada has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 8 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 65d372cf-0302-4a34-94a9-29a1cc96b1ba

📥 Commits

Reviewing files that changed from the base of the PR and between 18a5c59 and 71b10d7.

📒 Files selected for processing (3)
  • src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
  • test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs
  • test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cs

Walkthrough

The PR refactors test method naming across three unit test files to follow the Microsoft .NET standard, replacing verbose Given/When/Then patterns with concise {MethodName}{StateUnderTest}{ExpectedBehavior} conventions. Controller tests additionally use {METHOD}{Resource}{Condition}_Returns{Outcome}. New edge-case tests are added for UpdateAsync, DeleteAsync, and DateOfBirth boundary validation.

Changes

Cohort / File(s) Summary
Documentation Updates
.github/copilot-instructions.md, CHANGELOG.md
Updated test naming guidance with explicit patterns for Controller, Service, and Validator layers; added changelog entries for test refactoring and edge-case bug fixes.
Controller Test Refactoring
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs
Renamed 15 test methods from GivenXXX_WhenYYY_ThenZZZ to {METHOD}{Resource}{Condition}_Returns{Outcome} pattern (e.g., GivenGetAsync_WhenServiceRetrieveAsyncReturnsListOfPlayers_ThenResponseShouldBeEquivalentToListOfPlayersGet_Players_Existing_ReturnsPlayers); updated CreatedAtRoute type assertions and route names to reflect behavior changes.
Service Test Refactoring & Coverage
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerServiceTests.cs
Renamed 9 test methods to {MethodName}{StateUnderTest}{ExpectedBehavior} pattern; added 2 new tests for missing "not found" edge cases: UpdateAsync_PlayerNotFound_DoesNotUpdateRepository() and DeleteAsync_PlayerNotFound_DoesNotRemoveFromRepository().
Validator Test Refactoring & Coverage
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cs
Renamed 10 test methods to {MethodName}{StateUnderTest}{ExpectedBehavior} pattern; added 3 new DateOfBirth boundary validation tests: ValidateAsync_DateOfBirthToday_ReturnsValidationError(), ValidateAsync_DateOfBirthOnJanuary1st1900_ReturnsNoErrors(), and ValidateAsync_DateOfBirthBeforeYear1900_ReturnsValidationError().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Assessment against linked issues

Objective Addressed Explanation
Apply unified naming pattern derived from Microsoft standard, specialized by test layer [#396]
Refactor Controller tests to {METHOD}_{Resource}_{Condition}_Returns{Outcome} pattern [#396]
Refactor Service and Validator tests to {MethodName}_{StateUnderTest}_{ExpectedBehavior} pattern [#396]
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]

Possibly related PRs

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.84% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits format, is under 80 characters, and accurately describes the main refactoring effort: renaming test methods to align with Microsoft's .NET naming standard.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #396

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/rename-test-methods
  • 🛠️ sync documentation: Commit on current branch
  • 🛠️ sync documentation: Create PR
  • 🛠️ enforce http error handling: Commit on current branch
  • 🛠️ enforce http error handling: Create PR
  • 🛠️ idiomatic review: Commit on current branch
  • 🛠️ idiomatic review: Create PR
  • 🛠️ verify api contract: Commit on current branch
  • 🛠️ verify api contract: Create PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nanotaboada
Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 7, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs`:
- Around line 149-156: Replace the conditional if-check with an unconditional
type assertion: in PlayerControllerTests (the assertion block for PostAsync),
first assert result is of type CreatedAtRoute<PlayerResponseModel> (e.g.
result.Should().BeOfType<CreatedAtRoute<PlayerResponseModel>>() or
Assert.IsType<CreatedAtRoute<PlayerResponseModel>>(result)), then cast to
CreatedAtRoute<PlayerResponseModel> and assert StatusCode == 201, Value is
equivalent to the expected response, RouteName == "RetrieveBySquadNumber", and
that RouteValues contains the exact "squadNumber" entry with the expected value
(not just the key).

In `@test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cs`:
- Around line 213-225: The test
ValidateAsync_DateOfBirthToday_ReturnsValidationError is flaky because it reads
DateTime.UtcNow.Date while PlayerRequestModelValidator also uses
DateTime.UtcNow.Date; refactor PlayerRequestModelValidator to accept an injected
time source (e.g., TimeProvider or an IClock) and use that instead of
DateTime.UtcNow, update CreateValidator() to accept and pass a deterministic
TimeProvider to the validator, and change the test to construct a fixed
TimeProvider (or mock IClock) and set request.DateOfBirth relative to that fixed
time so both the test and PlayerRequestModelValidator use the same stable clock.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1d819aaa-5792-4ac0-a740-fe2ce9407e8c

📥 Commits

Reviewing files that changed from the base of the PR and between c727528 and 18a5c59.

📒 Files selected for processing (5)
  • .github/copilot-instructions.md
  • CHANGELOG.md
  • test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs
  • test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerServiceTests.cs
  • test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerValidatorTests.cs

Comment thread test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs Outdated
- Replace if-guarded 201 assertion with unconditional BeOfType and add
  RouteValues squadNumber value check (PlayerControllerTests)
- Inject TimeProvider into PlayerRequestModelValidator to eliminate
  DateTime.UtcNow coupling
- Add FakeTimeProvider in PlayerValidatorTests and pin DateOfBirthToday
  to a fixed clock shared with the validator

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Mar 7, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor test method naming to follow the .NET unit testing standard

1 participant