improve integration tests readability #112
Conversation
There was a problem hiding this comment.
Pull request overview
This PR significantly improves the readability and maintainability of integration tests by introducing a builder pattern for test data creation. The changes eliminate hundreds of lines of repetitive seeding code by extracting common setup logic into reusable builder classes.
Key changes:
- Introduced builder pattern: Created
ThreadTestDataBuilder,PostTestDataBuilder, andBanTestDataBuilderto encapsulate test data creation with a fluent API - Refactored test methods: Replaced inline entity construction with concise, declarative builder calls that reduce test method sizes by ~80-90%
- Improved naming consistency: Renamed
ISeedResulttoIAppScopeandSeedResulttoAppScopefor better semantic clarity
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ThreadRepositoryTests.cs | Replaced ~600 lines of inline entity setup with builder-based seed methods; tests are now more focused on assertions |
| PostRepositoryTests.cs | Extracted post search test data into SeedSearchPostsDataAsync using PostTestDataBuilder |
| BanRepositoryTests.cs | Replaced ban entity construction with BanTestDataBuilder for both exact and range ban tests |
| ThreadTestDataBuilder.cs | New builder providing fluent API for creating threads, posts with attachments, and complex scenarios like bump limits and sage posts |
| PostTestDataBuilder.cs | New builder for simpler post-focused test scenarios |
| BanTestDataBuilder.cs | New builder for ban-related test data with exact and range ban support |
| IAppScope.cs | Renamed from ISeedResult to better reflect its purpose as an application scope container |
| IAppFactoryScope.cs | New interface extending IAppScope to include AppFactory property |
| AppScope.cs | Renamed from SeedResult and now implements IAppFactoryScope |
| IAppFactorySeedResult.cs | Removed (functionality merged into IAppFactoryScope) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| [MustDisposeResource] | ||
| private async Task<ISeedResult> Seed(CancellationToken cancellationToken) | ||
| private async Task<IAppScope> CreateSeedResultAsync(CancellationToken cancellationToken) |
There was a problem hiding this comment.
Method naming is inconsistent across test files. This file uses CreateSeedResultAsync, but ThreadRepositoryTests.cs uses CreateAppScopeAsync for the same purpose. The methods should have the same name for consistency.
Suggestion: Rename to CreateAppScopeAsync to match ThreadRepositoryTests.cs and better reflect what it returns (an IAppScope).
| private async Task<IAppScope> CreateSeedResultAsync(CancellationToken cancellationToken) | |
| private async Task<IAppScope> CreateAppScopeAsync(CancellationToken cancellationToken) |
|
|
||
| [MustDisposeResource] | ||
| private async Task<ISeedResult> Seed(CancellationToken cancellationToken) | ||
| private async Task<IAppScope> CreateSeedResultAsync(CancellationToken cancellationToken) |
There was a problem hiding this comment.
Method naming is inconsistent across test files. This file uses CreateSeedResultAsync, but ThreadRepositoryTests.cs uses CreateAppScopeAsync for the same purpose. The methods should have the same name for consistency.
Suggestion: Rename to CreateAppScopeAsync to match ThreadRepositoryTests.cs and better reflect what it returns (an IAppScope).
| private async Task<IAppScope> CreateSeedResultAsync(CancellationToken cancellationToken) | |
| private async Task<IAppScope> CreateAppScopeAsync(CancellationToken cancellationToken) |
No description provided.