test(repositories): add integration tests for Repository<T> and PlayerRepository (#461)

[nanotaboada]

Summary

• Add test/.../Integration/PlayerRepositoryTests.cs with 9 integration tests covering the non-trivial behaviors of Repository<T> and PlayerRepository
• All tests run against in-memory SQLite via DatabaseFakes.MigrateAsync(), validating the full EF Core migration chain as a side effect
• Tests are tagged [Trait("Category", "Integration")] to distinguish from unit tests
• Both branches of RemoveAsync (entity found / not found) and SquadNumberExistsAsync (exists / does not exist) are exercised

Test coverage

Method	Scenario
GetAllAsync	Returns all 26 seeded players; verifies AsNoTracking() via empty ChangeTracker
FindByIdAsync	Existing ID returns player; unknown ID returns null
RemoveAsync	Existing entity is removed; unknown ID is a no-op with no exception
FindBySquadNumberAsync	Existing squad number returns player; unknown returns null
SquadNumberExistsAsync	Existing squad number returns true; unknown returns false

Test plan

• dotnet build --configuration Release passes with 0 warnings
• dotnet test --settings .runsettings passes — 50/50 tests (9 new)
• dotnet csharpier --check . passes

Closes #461

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Introduced comprehensive integration test suite with 9 tests for player repository functionality, including validation of player retrieval, database removal operations, and squad number lookups. Tests execute against a SQLite database to ensure robust data handling and system reliability.

[coderabbitai]

Warning

Rate limit exceeded

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

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 1 seconds.

⌛ 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.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 572b7320-f44a-4e67-9caf-b8ec66a58be9

📥 Commits

Reviewing files that changed from the base of the PR and between 03a95aa and e463018.

📒 Files selected for processing (2)

• CHANGELOG.md
• test/Dotnet.Samples.AspNetCore.WebApi.Tests/Integration/PlayerRepositoryTests.cs

Walkthrough

A new integration test file is added to validate Repository<T> and PlayerRepository methods against an in-memory SQLite database. The test class uses IAsyncLifetime for setup and teardown, seeding data via migrations, and covers nine scenarios including happy paths and edge cases (missing entities, null guards, existence checks).

Changes

Cohort / File(s)	Summary
Changelog CHANGELOG.md	Added entry documenting new integration test file for Repository and PlayerRepository coverage.
Integration Tests test/.../Integration/PlayerRepositoryTests.cs	New test class with 9 async [Fact] tests covering GetAllAsync, FindByIdAsync, RemoveAsync, FindBySquadNumberAsync, and SquadNumberExistsAsync using EF Core in-memory SQLite database initialized via DatabaseFakes.MigrateAsync().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Assessment against linked issues

Objective	Addressed	Explanation
Create test/.../Integration/PlayerRepositoryTests.cs with coverage for Repository<T> and PlayerRepository methods [#461]	✅
Test both branches of RemoveAsync (entity found and not found) and SquadNumberExistsAsync (exists and does not exist) [#461]	✅
Use DatabaseFakes.MigrateAsync() for setup and apply [Trait("Category", "Integration")] to all tests [#461]	✅
Update CHANGELOG.md [#461]	✅

Possibly related PRs

• PR #460: Introduces DatabaseFakes.MigrateAsync() and in-memory SQLite infrastructure that these integration tests directly depend on.
• PR #189: Implements the Repository<T> and PlayerRepository methods that these tests exercise and validate.

🚥 Pre-merge checks | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	⚠️ Warning	The pull request title exceeds the 80-character limit (91 characters) specified in requirements, despite using proper Conventional Commits format and being descriptive.	Reduce title length to 80 characters or fewer while maintaining the Conventional Commits format and descriptive nature. Consider: 'test(repositories): add integration tests for Repository' or similar variation.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/repository-integration-tests

• 🛠️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

test/Dotnet.Samples.AspNetCore.WebApi.Tests/Integration/PlayerRepositoryTests.cs (1)

121-129: Reduce magic numbers in squad-number tests.

Using fixed literals (23, 999) makes these tests more brittle to seed-data evolution. Prefer PlayerFakes for known existing values and derive an unknown value from current seeded data.

♻️ Suggested refactor

 public async Task FindBySquadNumberAsync_ExistingSquadNumber_ReturnsPlayer()
 {
+    var existingSquadNumber = PlayerFakes.MakeFromStarting11(1).SquadNumber;
+
     // Act
-    var player = await _repository.FindBySquadNumberAsync(23);
+    var player = await _repository.FindBySquadNumberAsync(existingSquadNumber);

     // Assert
     player.Should().NotBeNull();
-    player!.SquadNumber.Should().Be(23);
+    player!.SquadNumber.Should().Be(existingSquadNumber);
 }

 public async Task FindBySquadNumberAsync_UnknownSquadNumber_ReturnsNull()
 {
+    var unknownSquadNumber = (await _repository.GetAllAsync()).Max(p => p.SquadNumber) + 1;
+
     // Act
-    var player = await _repository.FindBySquadNumberAsync(999);
+    var player = await _repository.FindBySquadNumberAsync(unknownSquadNumber);

     // Assert
     player.Should().BeNull();
 }

 public async Task SquadNumberExistsAsync_ExistingSquadNumber_ReturnsTrue()
 {
+    var existingSquadNumber = PlayerFakes.MakeFromStarting11(1).SquadNumber;
+
     // Act
-    var exists = await _repository.SquadNumberExistsAsync(23);
+    var exists = await _repository.SquadNumberExistsAsync(existingSquadNumber);

     // Assert
     exists.Should().BeTrue();
 }

 public async Task SquadNumberExistsAsync_UnknownSquadNumber_ReturnsFalse()
 {
+    var unknownSquadNumber = (await _repository.GetAllAsync()).Max(p => p.SquadNumber) + 1;
+
     // Act
-    var exists = await _repository.SquadNumberExistsAsync(999);
+    var exists = await _repository.SquadNumberExistsAsync(unknownSquadNumber);

     // Assert
     exists.Should().BeFalse();
 }

As per coding guidelines: Always prefer `PlayerFakes` factory methods (`MakeNew()`, `MakeRequestModelForCreate()`, `MakeRequestModelForUpdate(n)`, `MakeFromStarting11(n)`) over constructing test data inline.

Also applies to: 133-140, 148-155, 159-166

🤖 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/Integration/PlayerRepositoryTests.cs`:
- Around line 106-113: The test RemoveAsync_UnknownId_NoExceptionThrown only
checks for no exception; update it to also verify the repository state is
unchanged: read the items (or count) from _repository before calling
RemoveAsync(Guid.NewGuid()), call the method, then assert the post-call
items/count equals the pre-call value; also, if RemoveAsync returns a
boolean/indication, assert it returns false (no-op). Locate this logic in the
PlayerRepositoryTests test method RemoveAsync_UnknownId_NoExceptionThrown and
use the repository's existing List/Count/Find methods to perform the
before/after comparison.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f5df0c04-ad0d-4bca-abc6-00591b207fe9

📥 Commits

Reviewing files that changed from the base of the PR and between 9631511 and 03a95aa.

📒 Files selected for processing (2)

• CHANGELOG.md
• test/Dotnet.Samples.AspNetCore.WebApi.Tests/Integration/PlayerRepositoryTests.cs

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud