test(solution): Improve Ploch.Data testing experience and code coverage across all projects#75
Open
test(solution): Improve Ploch.Data testing experience and code coverage across all projects#75
Conversation
Add new test projects for Data.Utilities and Data.Model, and add additional test files for Data.EFCore, Data.GenericRepository.EFCore, and UnitOfWork to improve overall code coverage toward the 80% target. New test projects: - Data.Utilities.Tests: DataColumnExtensionsTests (7 tests) - Data.Model.Tests: Category, Tag, Property, Image common types (38 tests) New test files in existing projects: - GetStaticPropertyValueTests for DbContextExtensions (5 tests) - QueryableRepositoryTests (8 tests) - ReadWriteRepositoryAsyncAdditionalTests (18 tests) - ReadWriteRepositoryDeleteByIdTests (5 tests) - UnitOfWorkAdditionalTests (5 tests) Total new tests: 86 Refs: #13
Contributor
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Code Review Agent Run #6cd825Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Review Summary by QodoAdd comprehensive test coverage across all projects (86 new tests)
WalkthroughsDescription• Add 86 new tests across 2 new test projects and 4 existing projects • Create Data.Utilities.Tests project with DataColumnExtensions coverage • Create Data.Model.Tests project for common types (Category, Tag, Property, Image) • Expand Data.EFCore.Tests with DbContextExtensions.GetStaticPropertyValue tests • Add comprehensive integration tests for GenericRepository and UnitOfWork • Update solution file to include new test projects Diagramflowchart LR
A["New Test Projects"] --> B["Data.Utilities.Tests<br/>7 tests"]
A --> C["Data.Model.Tests<br/>38 tests"]
D["Existing Test Projects"] --> E["Data.EFCore.Tests<br/>5 tests"]
D --> F["GenericRepository.EFCore<br/>36 tests"]
D --> G["UnitOfWork<br/>5 tests"]
B --> H["Solution Coverage<br/>86 new tests"]
C --> H
E --> H
F --> H
G --> H
File Changes1. tests/Data.Utilities.Tests/DataColumnExtensionsTests.cs
|
📝 WalkthroughWalkthroughAdds FluentAssertions equivalency helpers and a null/empty-collection equivalency step; extends EF Core integration-test helpers to expose a root DbContext and selectable scoped/root service resolution; reorders returned service-provider tuple; adds many new/updated unit and integration tests and multiple repository documentation/configuration files. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Integration Test
participant Scoped as Scoped ServiceProvider
participant UoW as UnitOfWork/Repository
participant RootProv as Root ServiceProvider (IDbContextFactory)
participant DB as Database
Test->>Scoped: CreateUnitOfWork(useScopedProvider=true)
Scoped->>UoW: Resolve repositories (scoped DbContext)
UoW->>DB: Persist changes via scoped DbContext
Test->>RootProv: CreateRootDbContext()
RootProv->>DB: New DbContext reads persisted state for verification
DB-->>Test: Return rehydrated entities
Note right of Test: Assertions use WithEntityEquivalencyOptions()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewⓘ The new review experience is currently in Beta. Learn more |
codeant-ai
Bot
added
the
size:XXL
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive suite of unit and integration tests across several projects, including Data.Model, Data.Utilities, and GenericRepository, while also adding the corresponding test projects to the solution file. Feedback was provided regarding fragile project references pointing outside the repository root, the need to enforce the IAsyncDisposable contract in UnitOfWork tests rather than using conditional checks, and the recommendation to enable TreatWarningsAsErrors to maintain code quality.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 4 medium 5 minor |
| CodeStyle | 39 minor |
🟢 Metrics 92 complexity · -24 duplication
Metric Results Complexity 92 Duplication -24
TIP This summary will be updated as you push new changes. Give us feedback
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3639541a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Contributor
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
tests/Data.EFCore.Tests/GetStaticPropertyValueTests.cs (1)
39-45: Consider tightening the type-mismatch message assertion.
WithMessage("*not of*type*")is a bit broad; includingPublicValueandInt32would make this test more regression-resistant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Data.EFCore.Tests/GetStaticPropertyValueTests.cs` around lines 39 - 45, Update the assertion in GetStaticPropertyValue_should_throw_when_property_type_does_not_match to assert a more specific error message: when invoking typeof(ClassWithStaticProperties).GetStaticPropertyValue<int>("PublicValue") the thrown InvalidOperationException message should include both the property name "PublicValue" and the expected type name "Int32" (or System.Int32) so replace WithMessage("*not of*type*") with a pattern that ensures "PublicValue" and "Int32" appear in the message to make the test regression-resistant.tests/Data.Model.Tests/CommonTypes/PropertyTests.cs (1)
68-109: Optional: splitIntPropertyTestsandStringPropertyTestsinto separate files.Keeping one test class per file would make discovery and future edits easier as coverage grows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Data.Model.Tests/CommonTypes/PropertyTests.cs` around lines 68 - 109, Split the combined test class file by creating two separate test files: one containing the IntPropertyTests class (moving the IntProperty_should_have_int_value and IntPropertyWithCustomId_should_support_custom_id_type tests that reference IntProperty and IHasValue<int>) and another containing the StringPropertyTests class (moving StringProperty_should_have_string_value and StringPropertyWithCustomId_should_support_custom_id_type that reference StringProperty and IHasValue<string>); preserve the original namespace and using directives in both new files, name each file to match its class (e.g., IntPropertyTests.cs and StringPropertyTests.cs), and verify test discovery still finds both classes.tests/Data.Utilities.Tests/DataColumnExtensionsTests.cs (1)
10-63: Optional: collapse repetitive one-property tests into a[Theory].The per-property copy tests are clear, but a data-driven pattern would reduce duplication and simplify future property additions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Data.Utilities.Tests/DataColumnExtensionsTests.cs` around lines 10 - 63, Replace the repetitive [Fact] methods in DataColumnExtensionsTests that each verify a single property copy with a single data-driven [Theory] (e.g., CopyProperties_should_copy_property) that takes parameters for the property name and expected value; in the test use reflection to set the property on the source DataColumn, call source.CopyProperties(target), and assert the target's property equals the expected value; this consolidates the individual tests like CopyProperties_should_copy_AllowDBNull, CopyProperties_should_copy_AutoIncrement, CopyProperties_should_copy_Caption, CopyProperties_should_copy_AutoIncrementSeed, and CopyProperties_should_copy_AutoIncrementStep into one parameterized test using InlineData or MemberData.tests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/QueryableRepositoryTests.cs (2)
83-99:onDbSettest currently allows false positives.With current dataset and assertions, this passes even if
onDbSetis ignored. Add an assertion that all returned rows satisfy the custom filter (Id <= 8), or choose a filter/page combo where count differs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/QueryableRepositoryTests.cs` around lines 83 - 99, The test GetPageQuery_with_onDbSet_should_apply_custom_query may pass even if the onDbSet callback is ignored; update the test that calls IQueryableRepository<TestEntity>.GetPageQuery(..., onDbSet: q => q.Where(e => e.Id <= 8)) to assert the filter is applied by verifying every returned TestEntity has Id <= 8 (or change the onDbSet/filter and pagination values so the expected count would differ if the callback were ignored); ensure the assertion(s) reference the result variable and the onDbSet predicate so the test fails when GetPageQuery ignores the provided custom query.
24-40: Paging test should assert page contents, not just size.
HaveCount(5)alone does not prove that page 2 is returned. Assert expected IDs/names for stronger regression protection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/QueryableRepositoryTests.cs` around lines 24 - 40, Update the test GetPageQuery_should_return_paged_queryable to assert the actual page contents rather than only the count: after calling queryableRepo.GetPageQuery(2, 5) and materializing result, verify that the sequence contains the expected entities (e.g., Ids 6–10 or Names "Entity06".."Entity10") so the test confirms page 2 is returned; locate this behavior around the CreateReadRepositoryAsync<TestEntity, int>(), GetPageQuery(...) and the result.Should() assertion and replace/augment the HaveCount(5) check with explicit assertions on Id or Name values.tests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/ReadWriteRepositoryAsyncAdditionalTests.cs (1)
187-199: Strengthen assertions to actually validateonDbSet/sort/query behavior.Both tests currently can pass even if parts of query composition are ignored (they mostly assert
NotBeNull/HaveCount(5)). Assert returned IDs/order to prove semantics.Example tightening
[Fact] public async Task FindFirstAsync_with_onDbSet_should_apply_custom_query() { @@ - var result = await repository.FindFirstAsync(e => e.Name == "Alpha", q => q.OrderBy(e => e.Name)); + var result = await repository.FindFirstAsync(e => e.Name.Contains("a"), + q => q.OrderByDescending(e => e.Name)); @@ - result!.Name.Should().Be("Alpha"); + result!.Name.Should().Be("Beta"); } @@ public async Task GetPageAsync_with_sort_and_query_should_return_filtered_sorted_results() { @@ var page = await readRepo.GetPageAsync(1, 5, sortBy: e => e.Name, query: e => e.Id > 3); page.Should().HaveCount(5); + page.Select(e => e.Id).Should().Equal(10, 4, 5, 6, 7); // Entity10, Entity4..Entity7 (string sort) + page.Should().OnlyContain(e => e.Id > 3); }Also applies to: 264-279
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/ReadWriteRepositoryAsyncAdditionalTests.cs` around lines 187 - 199, The test FindFirstAsync_with_onDbSet_should_apply_custom_query currently only asserts NotBeNull and Name, which doesn't prove q => q.OrderBy was applied; update this test (and the similar one around lines 264-279) to insert entities with distinct IDs and Names (e.g., "Alpha","Beta"), call repository.FindFirstAsync(e => e.Name == "Alpha", q => q.OrderBy(e => e.Name)) and assert the returned entity's Id and Name equal the expected values (e.g., Id == 1 and Name == "Alpha"), and for the other test assert the full ordered sequence of returned IDs/names to prove onDbSet/sort/query composition is respected (reference repository.FindFirstAsync and the test methods FindFirstAsync_with_onDbSet_should_apply_custom_query and the other test at 264-279).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@tests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/UnitOfWorkAdditionalTests.cs`:
- Around line 54-58: The test currently calls DisposeAsync only when unitOfWork
is IAsyncDisposable, which can silently pass if CreateUnitOfWork stops
implementing IAsyncDisposable; change the test to assert that unitOfWork is
assignable to IAsyncDisposable (e.g., using
unitOfWork.Should().BeAssignableTo<IAsyncDisposable>()) before casting and
invoking DisposeAsync, then perform the async dispose call on the cast instance
(DisposeAsync) and assert it does not throw; target the unitOfWork variable and
the CreateUnitOfWork call in the test so the assertion fails loudly if async
disposal support is removed.
---
Nitpick comments:
In `@tests/Data.EFCore.Tests/GetStaticPropertyValueTests.cs`:
- Around line 39-45: Update the assertion in
GetStaticPropertyValue_should_throw_when_property_type_does_not_match to assert
a more specific error message: when invoking
typeof(ClassWithStaticProperties).GetStaticPropertyValue<int>("PublicValue") the
thrown InvalidOperationException message should include both the property name
"PublicValue" and the expected type name "Int32" (or System.Int32) so replace
WithMessage("*not of*type*") with a pattern that ensures "PublicValue" and
"Int32" appear in the message to make the test regression-resistant.
In
`@tests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/QueryableRepositoryTests.cs`:
- Around line 83-99: The test
GetPageQuery_with_onDbSet_should_apply_custom_query may pass even if the onDbSet
callback is ignored; update the test that calls
IQueryableRepository<TestEntity>.GetPageQuery(..., onDbSet: q => q.Where(e =>
e.Id <= 8)) to assert the filter is applied by verifying every returned
TestEntity has Id <= 8 (or change the onDbSet/filter and pagination values so
the expected count would differ if the callback were ignored); ensure the
assertion(s) reference the result variable and the onDbSet predicate so the test
fails when GetPageQuery ignores the provided custom query.
- Around line 24-40: Update the test GetPageQuery_should_return_paged_queryable
to assert the actual page contents rather than only the count: after calling
queryableRepo.GetPageQuery(2, 5) and materializing result, verify that the
sequence contains the expected entities (e.g., Ids 6–10 or Names
"Entity06".."Entity10") so the test confirms page 2 is returned; locate this
behavior around the CreateReadRepositoryAsync<TestEntity, int>(),
GetPageQuery(...) and the result.Should() assertion and replace/augment the
HaveCount(5) check with explicit assertions on Id or Name values.
In
`@tests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/ReadWriteRepositoryAsyncAdditionalTests.cs`:
- Around line 187-199: The test
FindFirstAsync_with_onDbSet_should_apply_custom_query currently only asserts
NotBeNull and Name, which doesn't prove q => q.OrderBy was applied; update this
test (and the similar one around lines 264-279) to insert entities with distinct
IDs and Names (e.g., "Alpha","Beta"), call repository.FindFirstAsync(e => e.Name
== "Alpha", q => q.OrderBy(e => e.Name)) and assert the returned entity's Id and
Name equal the expected values (e.g., Id == 1 and Name == "Alpha"), and for the
other test assert the full ordered sequence of returned IDs/names to prove
onDbSet/sort/query composition is respected (reference repository.FindFirstAsync
and the test methods FindFirstAsync_with_onDbSet_should_apply_custom_query and
the other test at 264-279).
In `@tests/Data.Model.Tests/CommonTypes/PropertyTests.cs`:
- Around line 68-109: Split the combined test class file by creating two
separate test files: one containing the IntPropertyTests class (moving the
IntProperty_should_have_int_value and
IntPropertyWithCustomId_should_support_custom_id_type tests that reference
IntProperty and IHasValue<int>) and another containing the StringPropertyTests
class (moving StringProperty_should_have_string_value and
StringPropertyWithCustomId_should_support_custom_id_type that reference
StringProperty and IHasValue<string>); preserve the original namespace and using
directives in both new files, name each file to match its class (e.g.,
IntPropertyTests.cs and StringPropertyTests.cs), and verify test discovery still
finds both classes.
In `@tests/Data.Utilities.Tests/DataColumnExtensionsTests.cs`:
- Around line 10-63: Replace the repetitive [Fact] methods in
DataColumnExtensionsTests that each verify a single property copy with a single
data-driven [Theory] (e.g., CopyProperties_should_copy_property) that takes
parameters for the property name and expected value; in the test use reflection
to set the property on the source DataColumn, call
source.CopyProperties(target), and assert the target's property equals the
expected value; this consolidates the individual tests like
CopyProperties_should_copy_AllowDBNull,
CopyProperties_should_copy_AutoIncrement, CopyProperties_should_copy_Caption,
CopyProperties_should_copy_AutoIncrementSeed, and
CopyProperties_should_copy_AutoIncrementStep into one parameterized test using
InlineData or MemberData.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ced24c00-f205-4eaf-a14d-42f1d5469def
📒 Files selected for processing (13)
Ploch.Data.slnxtests/Data.EFCore.Tests/GetStaticPropertyValueTests.cstests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/QueryableRepositoryTests.cstests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/ReadWriteRepositoryAsyncAdditionalTests.cstests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/ReadWriteRepositoryDeleteByIdTests.cstests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/UnitOfWorkAdditionalTests.cstests/Data.Model.Tests/CommonTypes/CategoryTests.cstests/Data.Model.Tests/CommonTypes/ImageTests.cstests/Data.Model.Tests/CommonTypes/PropertyTests.cstests/Data.Model.Tests/CommonTypes/TagTests.cstests/Data.Model.Tests/Ploch.Data.Model.Tests.csprojtests/Data.Utilities.Tests/DataColumnExtensionsTests.cstests/Data.Utilities.Tests/Ploch.Data.Utilities.Tests.csproj
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
- Use `using` declarations for DataColumn instances to properly dispose IDisposable resources in DataColumnExtensionsTests - Strengthen weak paging test assertions: use page sizes larger than filtered result sets so the test would fail if the filter is not applied (addresses false-positive count assertions) Refs: #13
Changelist by BitoThis pull request implements the following key changes.
|
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
Code Review Agent Run #cfd3b8Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Remove `.editorconfig` and `build-dotnet-commands.ps1` as part of repository cleanup. These files are no longer needed and will help streamline the project structure.
Contributor
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XXL
Contributor
|
CodeAnt AI Incremental review completed. |
|
Code Review Agent Run #2f71afActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Establish comprehensive AI assistant rules covering commit message standards, documentation guidelines, domain model patterns, and QA testing processes to ensure consistent code quality and documentation across the solution.
Codacy reported 3 residual new issues after the .aiassistant/** / .github/agents/** / .github/copilot-instructions.md exclusions in 4c264a0. Local actionlint + yamllint pinpoints the remaining findings to the new 380-line copilot-pr-pipeline.yml workflow. That workflow is already tracked for rework in issue #79 (request-body schema mismatch with the Agent Tasks API + timeout-as-success bug), so excluding it from Codacy until the rework lands keeps the quality gate green without papering over real workflow infrastructure issues. The build-dotnet.yml and deploy-nuget-org.yml workflows remain analysed. Refs: #13
Session-specific lock written by Claude Code's task scheduler; not meant to be tracked. Added to .gitignore alongside the existing settings.local.json entry. Refs: #13
Two-fold cleanup based on the actual Codacy annotations fetched from the GitHub check-runs API: 1. Remove commented-out fallback code in DataIntegrationTest.CreateRootDbContext (S125). The GetRequiredService alternative was kept as a comment during the IDbContextFactory migration in 6714dc9 — no longer relevant. 2. Promote test/sample path exclusions from the sonarscharp engine block to the global exclude_paths in .codacy.yml. Codacy uses multiple engines for C# (sonarscharp + an internal analyzer) and the engine-specific list was only honoured by sonarscharp, letting findings from tests/Data.EFCore.Tests/GetStaticPropertyValueTests.cs leak through (S1118, S3257). The test/sample exclusions reflect the actual Codacy scope policy already documented in this repo. Refs: #13
Code Review Agent Run #717800Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Revised the ContextStream rules to enhance clarity and ensure consistent guidelines for session initialization, tool usage, and memory management. These changes aim to improve user understanding and adherence to best practices. Refs: #13
Included a new folder in the solution for the TestingSupport.FluentAssertions.Tests project to organize test files. Refs: #123
Restore the conditional 'if available / fall back to local tools' language in .cursorrules, CLAUDE.md, and GEMINI.md so ContextStream behaviour degrades gracefully when the MCP tools are not loaded. All new content additions from e78d705 (save lesson / save decision shortcuts, grounding guidance, past-sessions ladder, project-scope discipline) are preserved. Restore AGENTS.md to its pre-e78d705 content; the previous commit had inadvertently truncated it from 780 lines to 24, leaving the file with malformed leading content. Also add temp/ to .gitignore to keep scratch summary reports out of the working tree. Refs: #13
Drop the .claude/rules/pr-checks-completion-gate.md symlink that was auto-staged into 34e5af0. The link points at the workspace-level rule file (../../../.claude/rules/pr-checks-completion-gate.md), which does not resolve cross-platform — git stores the path as text and contributors without filesystem symlink support (e.g. Windows without Developer Mode) end up with a broken file. If a project-local copy of the rule is needed later, vendor the file or reference the workspace-level path explicitly from the rules that link to it instead of relying on a filesystem symlink. Refs: #13
…ule files Apply the same 'if ContextStream tools are available / fall back when unavailable' framing already used in .cursorrules, CLAUDE.md, and GEMINI.md. The mandatory wording previously implied that the rules in AGENTS.md applied unconditionally, which is misleading for environments without the ContextStream MCP tools loaded. - Add a top-level scoping note clarifying that the rules apply only when ContextStream tools are available - Soften 'NEVER fall back' / 'Use local tools ONLY if ContextStream returns 0 results' instructions to acknowledge the unavailable case - Add 'ContextStream tools are unavailable' to the 'When Local Tools Are OK' bullet list in the search protocol section Refs: #13
Contributor
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XXL
Contributor
|
CodeAnt AI Incremental review completed. |
There was a problem hiding this comment.
♻️ Duplicate comments (3)
.github/copilot-instructions.md (1)
520-520:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix typo: "documentationo" should be "documentation".
The sentence reads "...to include the new feature usage documentationo" which should be "documentation".
📝 Suggested fix
-- Always keep the documentation markdown files in `docs` folder in the repository root [docs/](../docs/) up to date. If new features are being added, then those docs need to be extended to include the new feature usage documentationo. If anything changes, then the docs need to be updated. Always provide examples in the docs when discussing a feature. +- Always keep the documentation markdown files in `docs` folder in the repository root [docs/](../docs/) up to date. If new features are being added, then those docs need to be extended to include the new feature usage documentation. If anything changes, then the docs need to be updated. Always provide examples in the docs when discussing a feature.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/copilot-instructions.md at line 520, Fix the typo in the documentation sentence by replacing "documentationo" with "documentation" in the markdown content under the docs guidance sentence (the string "...to include the new feature usage documentationo"); update the text so it reads "...to include the new feature usage documentation" and ensure the corrected phrase appears wherever that exact misspelling exists in the file (.github/copilot-instructions.md)..aiassistant/rules/data-provider-project.md (1)
184-223:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPowerShell script templates lack Push-Location/Pop-Location guards for location-independent execution.
All three PowerShell script templates (recreate-migrations.ps1, update-database.ps1, and recreate-migrations-update-database.ps1) are missing the
Push-Location $PSScriptRoot/try/finally Pop-Locationguards. These templates will be copied by developers creating new provider projects, and scripts without these guards will silently fail or behave incorrectly when invoked from directories other than the provider project root.♻️ Suggested fix for all script templates
Wrap each script's commands in location guards:
recreate-migrations.ps1:
-Remove-Item Migrations -Force -Confirm:$false -Recurse -dotnet ef migrations add Initial +Push-Location $PSScriptRoot +try { + Remove-Item Migrations -Force -Confirm:$false -Recurse + dotnet ef migrations add Initial +} finally { Pop-Location }update-database.ps1:
-dotnet ef database update +Push-Location $PSScriptRoot +try { + dotnet ef database update +} finally { Pop-Location }recreate-migrations-update-database.ps1 (SQLite):
-Remove-Item *.db -Force -Confirm:$false -ErrorAction SilentlyContinue -./recreate-migrations.ps1 -./update-database.ps1 +Push-Location $PSScriptRoot +try { + Remove-Item *.db -Force -Confirm:$false -ErrorAction SilentlyContinue + ./recreate-migrations.ps1 + ./update-database.ps1 +} finally { Pop-Location }Apply the same pattern to the SQL Server variant (lines 219-223).
Based on learnings: PowerShell EF Core helper scripts (e.g., update-database.ps1, recreate-migrations.ps1, recreate-migrations-update-database.ps1) under samples/SampleApp/src/Data.* provider directories use
Push-Location $PSScriptRoot/Pop-Locationguards to ensure location-independent execution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.aiassistant/rules/data-provider-project.md around lines 184 - 223, The PowerShell script templates (recreate-migrations.ps1, update-database.ps1, and recreate-migrations-update-database.ps1) lack location-guard scaffolding, so wrap each script's existing commands by calling Push-Location $PSScriptRoot at the top and ensure a try / finally that calls Pop-Location in the finally block so the script runs correctly regardless of the caller's current directory; apply this pattern to both the SQLite and SQL Server variants in recreate-migrations-update-database.ps1 as well..aiassistant/rules/data-project.md (1)
151-157:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winContradicts domain-model rule: DataAnnotations [Key], [Required], [MaxLength] should be in Fluent API, not on entities.
Lines 151-157 recommend placing
[Key],[Required], and[MaxLength]DataAnnotations on entities in the Model project, but this contradicts the established rule that domain entity classes must remain ORM-agnostic. Based on learnings, relational constraints (keys, required, max-length, etc.) should be expressed via Fluent API in the Data project's EntityTypeConfiguration classes, not through DataAnnotations in domain models.Move these three items from "Prefer Data Annotations on the entity" to the "Always configure in Fluent API" section (lines 142-149).
Based on learnings: domain entity classes (in Ploch.Data.Model and consumer domain models) must remain ORM-agnostic; relational constraints (keys, required, max-length, etc.) should be expressed via Fluent API in the Data project (e.g., EntityTypeConfiguration classes), not forced through DataAnnotations in domain models.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.aiassistant/rules/data-project.md around lines 151 - 157, Update the rules doc to remove the three DataAnnotation recommendations ([Key], [Required], [MaxLength]) from the "Prefer Data Annotations on the entity" section and add them to the "Always configure in Fluent API" section; specifically ensure the guidance now states those relational constraints belong in Fluent API (EntityTypeConfiguration classes) rather than on domain model classes to keep domain entities ORM-agnostic, and adjust any examples or bullets that reference DataAnnotations to reference Fluent API configuration instead.
🧹 Nitpick comments (1)
.aiassistant/rules/agent.md (1)
11-11: 💤 Low valueConsider generalizing the tool reference for broader agent compatibility.
The reference to Cursor's
fetch_rulestool is IDE-specific. If this specification is intended for use across multiple AI assistants or environments, consider either:
- Providing alternative approaches for other environments
- Using more generic language like "Fetch relevant rules using available tooling (e.g.,
fetch_rulesin Cursor)"If Cursor is the sole intended environment, the current phrasing is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.aiassistant/rules/agent.md at line 11, The sentence currently references Cursor's `fetch_rules` tool which is IDE-specific; update the line in .aiassistant/rules/agent.md to generalize the tooling reference by either listing alternatives or using a generic phrase such as "Fetch relevant rules using available tooling (e.g., `fetch_rules` in Cursor)" so other agents/environments can follow it—locate the occurrence of `fetch_rules` and the mention of Cursor and replace with the generalized wording or add alternative approaches for other environments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.aiassistant/rules/data-project.md:
- Around line 151-157: Update the rules doc to remove the three DataAnnotation
recommendations ([Key], [Required], [MaxLength]) from the "Prefer Data
Annotations on the entity" section and add them to the "Always configure in
Fluent API" section; specifically ensure the guidance now states those
relational constraints belong in Fluent API (EntityTypeConfiguration classes)
rather than on domain model classes to keep domain entities ORM-agnostic, and
adjust any examples or bullets that reference DataAnnotations to reference
Fluent API configuration instead.
In @.aiassistant/rules/data-provider-project.md:
- Around line 184-223: The PowerShell script templates (recreate-migrations.ps1,
update-database.ps1, and recreate-migrations-update-database.ps1) lack
location-guard scaffolding, so wrap each script's existing commands by calling
Push-Location $PSScriptRoot at the top and ensure a try / finally that calls
Pop-Location in the finally block so the script runs correctly regardless of the
caller's current directory; apply this pattern to both the SQLite and SQL Server
variants in recreate-migrations-update-database.ps1 as well.
In @.github/copilot-instructions.md:
- Line 520: Fix the typo in the documentation sentence by replacing
"documentationo" with "documentation" in the markdown content under the docs
guidance sentence (the string "...to include the new feature usage
documentationo"); update the text so it reads "...to include the new feature
usage documentation" and ensure the corrected phrase appears wherever that exact
misspelling exists in the file (.github/copilot-instructions.md).
---
Nitpick comments:
In @.aiassistant/rules/agent.md:
- Line 11: The sentence currently references Cursor's `fetch_rules` tool which
is IDE-specific; update the line in .aiassistant/rules/agent.md to generalize
the tooling reference by either listing alternatives or using a generic phrase
such as "Fetch relevant rules using available tooling (e.g., `fetch_rules` in
Cursor)" so other agents/environments can follow it—locate the occurrence of
`fetch_rules` and the mention of Cursor and replace with the generalized wording
or add alternative approaches for other environments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ff360cf-20b1-4f8d-a757-b6a4296413fb
📒 Files selected for processing (33)
.aiassistant/review_guidelines.md.aiassistant/rules/agent.md.aiassistant/rules/commits.md.aiassistant/rules/data-access.md.aiassistant/rules/data-project.md.aiassistant/rules/data-provider-project.md.aiassistant/rules/domain-model.md.aiassistant/rules/todo-tasks-execution.md.aiassistant/rules/writing-dotnet-tests.md.claude/rules/integration-testing.md.claude/settings.local.json.codacy.yml.cursorrules.github/agents/pr-remediation.agent.md.github/agents/pr-review-planner.agent.md.github/copilot-instructions.md.gitignore.idea/.idea.Ploch.Data/.idea/indexLayout.xmlAGENTS.mdCLAUDE.mdGEMINI.mdNuGet.ConfigPloch.Data.slnxTODO.mdsrc/Data.EFCore.IntegrationTesting.FluentAssertions/EntitiesEquivalencyOptionsExtensions.cssrc/Data.EFCore.IntegrationTesting.FluentAssertions/NullEmptyCollectionEquivalencyStep.cssrc/Data.EFCore.IntegrationTesting/DataIntegrationTest.cstests/Data.EFCore.SqLite.Tests/SqLiteConnectionOptionsTests.cstests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/ReadRepositoryTests.cstests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/ReadWriteRepositoryAsyncAdditionalTests.cstests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/ReadWriteRepositoryAsyncTests.cstests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/RepositoryHelper.cstests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/UnitOfWorkRepositoryAsyncSQLiteInMemoryTests.cs
💤 Files with no reviewable changes (2)
- .claude/settings.local.json
- .idea/.idea.Ploch.Data/.idea/indexLayout.xml
✅ Files skipped from review due to trivial changes (4)
- .aiassistant/rules/writing-dotnet-tests.md
- .aiassistant/rules/todo-tasks-execution.md
- NuGet.Config
- TODO.md
🚧 Files skipped from review as they are similar to previous changes (7)
- tests/Data.EFCore.SqLite.Tests/SqLiteConnectionOptionsTests.cs
- tests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/RepositoryHelper.cs
- .gitignore
- Ploch.Data.slnx
- src/Data.EFCore.IntegrationTesting/DataIntegrationTest.cs
- tests/Data.GenericRepository/Data.GenericRepository.EFCore.IntegrationTests/ReadWriteRepositoryAsyncAdditionalTests.cs
- src/Data.EFCore.IntegrationTesting.FluentAssertions/NullEmptyCollectionEquivalencyStep.cs
Resolve the 5 minor CodeRabbit findings remaining on PR #75: - review_guidelines.md: Fix subject-verb agreement ("there is no new warnings" -> "there are no new warnings") and tighten phrasing of the "no new warning" golden rule. - commits.md: Add 'text' language identifier to all six fenced code blocks (commit-message format + five examples) to satisfy MD040. - data-provider-project.md: Wrap each PowerShell migration script template in Push-Location \$PSScriptRoot / try { ... } finally { Pop-Location } so the templates encourage location-independent scripts, matching the existing scripts in samples/SampleApp/src/Data.*. - pr-review-planner.agent.md: Add an H1 heading after the front matter to satisfy MD041 (first-line-h1). - copilot-instructions.md: Fix typo "documentationo" -> "documentation". Refs: #13
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/agents/pr-review-planner.agent.md (1)
34-41: ⚡ Quick winClarify the plan-critic integration workflow.
Step 8 instructs to "incorporate the critique and produce the final plan," but doesn't specify how to handle the plan-critic verdict (
approvevsrevise). Based on the pipeline orchestrator context, the intent appears to be: incorporate the critique's feedback (required changes, optional improvements, residual risks) into the final plan document regardless of verdict, rather than looping back to revise and re-invoke plan-critic.Consider adding explicit guidance after line 41, such as:
"Incorporate all feedback from the plan-critic (required changes, optional improvements, and residual risk assessment) into the final remediation plan document. If the verdict is 'revise', ensure the required changes are addressed in the final plan's implementation steps."
This prevents potential confusion about whether to loop back or proceed forward in the pipeline.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/agents/pr-review-planner.agent.md around lines 34 - 41, Add an explicit sentence after step 8 clarifying how to handle the plan-critic verdict: state that all feedback from plan-critic (required changes, optional improvements, residual risks) must be incorporated into the final remediation plan document and that even if the verdict is "revise" the pipeline should not loop back but instead ensure required changes are explicitly addressed in the final plan's implementation steps; reference the "plan-critic" step and the "final plan" language so the guidance is unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.aiassistant/rules/data-provider-project.md:
- Line 251: Change the duplicate headings "### SQLite" and "### SQL Server" so
they are unique within the document (e.g., "### SQLite — Data Provider", "###
SQLite (Migrations)", "### SQL Server — Connection" or include the parent
section name); update the two occurrences referenced around the current block
(symbols: the literal headings "### SQLite" and "### SQL Server") to distinct
text that preserves meaning and avoids MD024 duplicate-heading warnings.
- Around line 11-24: In .aiassistant/rules/data-provider-project.md update the
two fenced code blocks to include explicit language tags: change the directory
listing block to start with ```text and change the gitignore example to start
with ```gitignore (the blocks containing the src/ Data.SQLite/ listing and the
*.db/*.db-shm/*.db-wal patterns); also apply the same addition of language
identifiers to the other occurrence mentioned (lines 253-257) so markdownlint
MD040 is satisfied.
In @.github/copilot-instructions.md:
- Around line 327-347: The duplicate "### Output Format Hints" heading should be
renamed to a distinct heading to resolve MD024; locate the second occurrence of
the heading in the Search Protocol section and change its text (for example to
"### Output Format Recommendations" or "### Output Format Tips") and update any
surrounding references if present so anchors are unique and navigation works
correctly.
---
Nitpick comments:
In @.github/agents/pr-review-planner.agent.md:
- Around line 34-41: Add an explicit sentence after step 8 clarifying how to
handle the plan-critic verdict: state that all feedback from plan-critic
(required changes, optional improvements, residual risks) must be incorporated
into the final remediation plan document and that even if the verdict is
"revise" the pipeline should not loop back but instead ensure required changes
are explicitly addressed in the final plan's implementation steps; reference the
"plan-critic" step and the "final plan" language so the guidance is unambiguous.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8348efd9-6044-4674-a6ba-f26f1ff02ada
📒 Files selected for processing (5)
.aiassistant/review_guidelines.md.aiassistant/rules/commits.md.aiassistant/rules/data-provider-project.md.github/agents/pr-review-planner.agent.md.github/copilot-instructions.md
✅ Files skipped from review due to trivial changes (1)
- .aiassistant/rules/commits.md
Resolve the new minor markdownlint findings raised against eb399ea: - data-provider-project.md (MD040): Add 'text' language tag to the Project Structure directory listing and 'gitignore' tag to the .gitignore example block. - data-provider-project.md (MD024): Disambiguate the duplicate '### SQLite' / '### SQL Server' headings under the .gitignore section by adding '(.gitignore)' suffix so each heading is unique. - copilot-instructions.md (MD024): Rename the duplicate '### Output Format Hints' heading in the SEARCH-FIRST section to '### Output Format Hints (Search-First Section)' to avoid a duplicate anchor with the earlier Search Protocol section heading. Refs: #13
StyleCop's SA1515 (single-line comment should be preceded by blank line) fired in two integration-test files. Add the missing blank lines to satisfy the rule. Whitespace-only — no behaviour change. - ReadRepositoryTests.cs (lines 134-135 and 162-163) - ReadWriteRepositoryAsyncTests.cs (lines 102-103 and 136-137) Refs: #13
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
The Install mono step in build-dotnet.yml and release.yml fetches the mono signing key from hkp://keyserver.ubuntu.com:80. When that keyserver is unresponsive (a recurring problem) the GPG client hangs for the default 30+ minute timeout instead of falling back, which blocks every PR build. Run 25202539009 hit this twice in a row before being cancelled. Replace the single-keyserver call with a small loop: - Try hkps://keys.openpgp.org first (more reliable HKP server). - Fall back to hkp://keyserver.ubuntu.com:80 (the original). - Final fallback to hkp://pgp.mit.edu. - Wrap each attempt in 'timeout 30' so an unresponsive server cannot hang the job — at most 90s total spent on key import. - Verify the key landed in the keyring before continuing, so a silent empty-keyring outcome triggers an explicit failure rather than a later opaque apt-update signature error. Same fix in both workflows. No behavioural change when the primary keyserver works. Refs: #13
Follow-up to d7a9c26. The first attempt to fetch the mono signing key in the new fallback loop hit hkps://keys.openpgp.org, which by policy strips user IDs from imported keys. GPG's recv-keys exited 0 (it processed the response), but the import was actually skipped: gpg: key A6A19B38D3D831EF: new key but contains no user ID - skipped Result: empty keyring, the loop accepted "success", and the post-loop list-keys check failed the build. Move the verification inside the loop and require at least one 'uid' line in --list-keys output. Re-order keyservers so the original keyserver.ubuntu.com is tried first (when it's responsive — that's the common case) and keys.openpgp.org is the last resort. Explicit exit 1 if nothing imports across all three. Refs: #13
f683357's keyserver loop hit the worst case in run 25203387217: keyserver.ubuntu.com and pgp.mit.edu both timed out (30s each), and keys.openpgp.org returned the key without user IDs (so GPG skipped it). Net result: 1m+ wasted plus build failure. Switch the primary path to a direct HTTPS download from mono's own distribution endpoint (https://download.mono-project.com/repo/xamarin.gpg) which is the same domain that hosts the apt repository itself. This: - Avoids GPG keyservers entirely on the happy path. - Uses curl with --max-time 30 — same time bound as before, no hang. - Handles both ASCII-armored and binary key formats (some mono mirrors serve one, some the other). - Falls back to the keyserver loop only if the HTTPS fetch fails or yields a keyring without UIDs. - Removes keys.openpgp.org from the fallback list (proven unusable). Same change applied to build-dotnet.yml and release.yml. Refs: #13
Code Review Agent Run #19848eActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
kploch
dismissed
bito-code-review[bot]’s stale review
Addressed in subsequent commits; Bito's later runs (#717800 on 2026-04-25, #19848e on 2026-05-01) report 0 actionable suggestions on HEAD cb75979. All 5 originally-flagged items in EntitiesEquivalencyOptionsExtensions.cs are fixed.
The .NET edition of dotnet-sonarscanner does not read sonar-project.properties — when present, post-processing fails with exit code 1. The Begin / End steps had `continue-on-error: true`, so the failure was silently swallowed: every PR build since 0ebee36 (2026-04-14) ran without uploading any analysis to SonarCloud, and the required `SonarCloud Code Analysis` GitHub status check stopped being posted on new commits. Migrate the analysis settings (coverage report paths, source and coverage exclusions) into inline `/d:` arguments on the Begin step, delete sonar-project.properties, and remove `continue-on-error: true` from the End step so future regressions surface immediately rather than rotting silently. Refs: #13
Contributor
|
Skipping CodeAnt AI review — this PR changes more than 100 files, which usually means a migration, codemod, or vendored drop. Line-level review on diffs this large produces duplicate findings on the same rewrite pattern and drowns out anything that actually matters. If you still want a review, comment |
|
This was referenced May 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Pull Request Description
Issue ticket number and link
Closes #13
Closes #12
Pull Request Changes Summary
💥 Breaking Changes
None.
🎯 New Features
Ploch.Data.EFCore.IntegrationTesting.FluentAssertions— provides theWithEntityEquivalencyOptions()extension method that consolidates three recurring EF Core integration-test concerns into a single call:WithoutStrictOrdering()— database result sets are unordered.IgnoringCyclicReferences()— EF Core eager-loading populates back-navigation cycles.BeCloseTo(1 ms)tolerance forDateTimeOffset— SQLite storesDateTimeOffsetas TEXT with ~100 µs precision; 1 ms is 10× the maximum observed rounding error.Data.Utilities.Tests—DataColumnExtensionsTestscovering allCopyPropertiesbehaviour (7 tests).Data.Model.Tests— Tests forCategory,Tag,Property,IntProperty,StringProperty, andImagecommon types including interface implementation verification, property get/set, hierarchical relationships, and generic type parameter variations (38 tests).Data.EFCore.Tests/GetStaticPropertyValueTests— Tests forDbContextExtensions.GetStaticPropertyValuecovering public/private static properties, null values, type mismatches, and missing properties (5 tests).Data.GenericRepository.EFCore.IntegrationTests/QueryableRepositoryTests— Tests forIQueryableRepository.EntitiesandGetPageQuerywith sorting, filtering,onDbSet, and argument validation (8 tests).Data.GenericRepository.EFCore.IntegrationTests/ReadWriteRepositoryAsyncAdditionalTests— Tests for async CRUD operations: delete by ID/entity, update, AddRange, GetById withonDbSet/keyValues, GetAll with filter/onDbSet, FindFirst, Count, GetPage (18 tests).Data.GenericRepository.EFCore.IntegrationTests/ReadWriteRepositoryDeleteByIdTests— Tests for sync repository: delete by ID, GetById withonDbSet, error cases (5 tests).Data.GenericRepository.EFCore.IntegrationTests/UnitOfWorkAdditionalTests— Tests for UnitOfWork: rollback, repository caching, dispose, commit return values (5 tests).🪲 Fixes
WithoutRecursing()with cyclic EF Core entity graphs.WithoutRecursing()limits recursion depth but does not prevent traversal into collection items, so cyclic back-navigations (Tag.BlogPosts,Category.Parent/Children) still caused false failures. Replaced with explicit back-navigation exclusion +IgnoringCyclicReferences()via the newWithEntityEquivalencyOptions()helper.GetPagetests — removed unnecessary top-levelTags/Categoriesexclusions where EF Core fixup makes cycles symmetric on both sides; replaced the three-step assertion pattern (main comparison + Tag equivalency + Category count/equivalency) with a singleContainEquivalentOfusing path-based back-navigation exclusions. The new assertion is both simpler and actually stronger — it verifies Tag/Category scalar content (Id, Name) instead of only counts.result!.Name.Should().Be("WithOnDbSet")assertion inReadWriteRepositoryDeleteByIdTests.GetById_with_onDbSet_should_return_entitywhere the expected name did not match the test-fixture blog name.📖 Docs
Ploch.Data.EFCore.IntegrationTesting.FluentAssertions/README.md— comprehensive documentation covering the problem the helper solves, API reference, usage patterns, back-navigation handling, SQLite precision details, and integration withDataIntegrationTest<TDbContext>.WithEntityEquivalencyOptions()extension method with<summary>,<remarks>,<example>, and<list>tags explaining all three concerns the helper addresses..claude/rules/files covering naming, summaries, QA, code quality, commits, PR descriptions, branch naming, testing standards, data-access/data-project/data-provider-project/domain-model/sample-apps rules. These live inmrploch-developmentand are imported byploch-data.🌿 Other
refactor(tests): Update service provider usage in integration tests — cleaner lifetime management and more consistentCreateRootDbContext()/IUnitOfWorkusage.chore(solution): Centralise build properties intoDirectory.Build.propsand extract SonarCloud configuration — removedTreatWarningsAsErrors,ImplicitUsings,Platforms, and other settings from 25 individual.csprojfiles into the rootDirectory.Build.props.chore: Remove stale root-level.editorconfigand unused build scripts (rules now governed by the workspace-level config inmrploch-development).Data.Model.TestsandData.Utilities.TeststoPloch.Data.slnx.TODO.mdentry queued for a follow-up: replace repository-based validation with directDbContextqueries (viaCreateRootDbContext()) across all integration tests — the current pattern uses the repository under test to verify its own writes, which can bypass true persistence through the EF tracking cache.Describe your changes
Add comprehensive test coverage for previously untested or under-tested projects in the
ploch-datasolution, and introduce a new FluentAssertions helper that dramatically simplifies entity-equivalency assertions in integration tests.Originally scoped as a pure test-coverage PR (86 new tests across 2 new test projects and 4 new test files), the scope was extended during review to:
WithEntityEquivalencyOptions()), eliminating ~60 lines of duplicated and fragile assertion code.DateTimeOffsettolerance was set at 100 ms, wide enough to hide real regressions. Investigated SQLite's actual precision behaviour (stores as TEXT with ~100 µs truncation; observed max ~78 µs), and tightened tolerance to 1 ms.Checklist before requesting a review
📐 Design Decisions
DateTimeOffsetas TEXT and truncates sub-microsecond ticks; the maximum observed rounding error is ~78 µs. 1 ms is ~10× the maximum error — tight enough to catch real bugs, loose enough to be stable. 100 ms was more than 1,000× the maximum error and could mask real timing regressions.IgnoringCyclicReferences()instead ofWithoutRecursing()?WithoutRecursing()limits depth but still enters collection items, so cyclic back-navigations (e.g.Tag.BlogPosts) cause false failures.IgnoringCyclicReferences()stops only when an actual cycle is detected, which is the correct semantics here.Tag.BlogPostshas 5 items while the in-memory tag has ~13. Top-level.Excluding(p => p.Tags)loses all tag verification; path-basedmember.Path.EndsWith(".BlogPosts")only excludes the asymmetric back-navigation, allowing full Tag/Category content (Id, Name) comparison in a single assertion..FluentAssertionsproject rather than putting the helper inData.EFCore.IntegrationTesting? Keeps the core integration-testing library free of a hard dependency on FluentAssertions — consumers who use a different assertion framework can use the base library without pulling FluentAssertions in.using? The helper is used in nearly every integration test file; a global using avoids visual noise and ensures consistent use.UnitOfWork.CommitAsync()in integration tests to ensure entities are persisted to the in-memory SQLite database before read assertions.IQueryableRepository<TEntity>cast to testEntitiesproperty andGetPageQuerydirectly.Testing
Data.Utilities0% → 100%,Data.Modelcommon types 0% → 100%,Data.EFCore.GetStaticPropertyValue0% → covered,QueryableRepository/ReadWriteRepositoryadditional paths covered.WithEntityEquivalencyOptions(), removal of unnecessary exclusions), the full integration test suite (85 tests) was re-run and continued to pass.Summary by CodeRabbit
Tests
Documentation
Bug Fixes
Chores
CodeAnt-AI Description
Improve EF Core integration testing and add broader repository coverage
What Changed
DateTimeOffsettolerance, and null-versus-empty collectionsImpact
✅ Clearer entity comparison failures✅ Fewer false test failures with EF Core and SQLite✅ Safer repository and paging behaviour checks🔄 Retrigger CodeAnt AI Review
Details
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by Bito
This pull request improves the testing experience and infrastructure reliability in the Ploch.Data solution by updating configuration files and test code to ensure deterministic behavior and better tool compatibility.
Detailed Changes