-
Notifications
You must be signed in to change notification settings - Fork 0
Improve tests coverage #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Remove check-secrets job that was failing dependabot PRs - Use default values (|| 'test123', || 'postgres', etc.) for all POSTGRES secrets - Allows workflows to run without configured GitHub secrets - Test credentials are only for CI/CD, not production Fixes dependabot PRs failing with 'Required secrets are missing' error
- Update roadmap with Sprint 1 accomplishments (days 1-6 completed) - Move test coverage improvement to Sprint 2 (dedicated sprint) - Create detailed Sprint 2 tasks document with: - Bruno API collections for 5 modules (30 mins) - Tools projects update (15 mins) - Fix skipped tests (45 mins) - Unit tests coverage 80%+ (2-3 hours) - Integration tests (1-2 hours) - E2E critical scenarios (1 hour) - Adjust sprint planning: Sprint 2 now Dec 3-16 (coverage + collections + tools) - Update milestones and timeline - CI/CD workflow fix documented Sprint 1 achievements: - Geographic restriction middleware with IBGE API ✅ - 4 Module APIs implemented (Documents, ServiceCatalogs, SearchProviders, Locations) ✅ - 28 tests reactivated (auth refactor + race condition fixes) ✅ - Skipped tests reduced 26% → 11.5% ✅ - Integration events Providers → SearchProviders ✅ - Schema standardization (search_providers) ✅ - CI/CD secrets validation removed ✅ Next: Begin Sprint 2 work on improve-tests-coverage branch
…modules
Documents module (5 endpoints):
- UploadDocument.bru - POST /api/v1/documents (multipart upload)
- GetDocument.bru - GET /api/v1/documents/{id}
- GetProviderDocuments.bru - GET /api/v1/documents/provider/{providerId}
- VerifyDocument.bru - POST /api/v1/documents/{id}/verify (admin)
- RejectDocument.bru - POST /api/v1/documents/{id}/reject (admin)
SearchProviders module (3 endpoints):
- SearchProviders.bru - POST /api/v1/search (anonymous, geospatial search)
- IndexProvider.bru - POST /api/v1/search/providers/{id}/index (admin)
- RemoveProvider.bru - DELETE /api/v1/search/providers/{id} (admin)
Features:
- Complete REST API documentation in .bru format
- Authorization policies documented (SelfOrAdmin, AdminOnly, AllowAnonymous)
- Request/response examples with realistic data
- Integration with shared Keycloak authentication
- Error codes and troubleshooting sections
- Ranking logic documented (tier → rating → distance)
Progress: 2/3 modules created (Documents ✅, SearchProviders ✅)
Remaining: ServiceCatalogs, Locations (next commit)
Related to Sprint 2 task #5: Create Bruno API collections
…s modules
ServiceCatalogs module (3 endpoints):
- CreateCategory.bru - POST /api/v1/catalogs/categories (admin)
- ListCategories.bru - GET /api/v1/catalogs/categories (public)
- CreateService.bru - POST /api/v1/catalogs/services (admin)
Locations module (2 endpoints):
- GetAddressFromCep.bru - GET /api/v1/locations/cep/{cep} (anonymous)
- ValidateCity.bru - POST /api/v1/locations/validate-city (anonymous)
Features:
- READMEs with endpoint documentation
- CEP lookup with 3-provider fallback chain (ViaCEP → BrasilAPI → OpenCEP)
- IBGE API integration for city validation
- Authorization policies documented
- Compact format for maintainability
Progress: 4/4 modules completed ✅
- Documents ✅ (5 endpoints)
- SearchProviders ✅ (3 endpoints)
- ServiceCatalogs ✅ (3 endpoints)
- Locations ✅ (2 endpoints)
Total: 13 new .bru files + 4 READMEs
Sprint 2 Task #5 completed: Create Bruno API collections for 5 modules
- Remove explicit version numbers from PackageReferences - Versions now managed centrally via Directory.Packages.props - Already using .NET 10 packages (EF Core 10 RC, Npgsql 10 RC) - Build and restore tested successfully Sprint 2 Task #6 completed: Update tools/ projects
Added Infrastructure project references: - Documents.Infrastructure (DocumentsDbContext) - SearchProviders.Infrastructure (SearchProvidersDbContext) - ServiceCatalogs.Infrastructure (ServiceCatalogsDbContext) Fixes: - Fixed AddDbContext call to use reflection with MakeGenericMethod - Properly invoke AddDbContext<TContext> for each discovered DbContext type - Maintains schema-per-module isolation (__EFMigrationsHistory per schema) Now supports all 5 module DbContexts: - UsersDbContext (users schema) - ProvidersDbContext (providers schema) - DocumentsDbContext (documents schema) - SearchProvidersDbContext (search_providers schema) - ServiceCatalogsDbContext (service_catalogs schema) Build: ✅ Succeeded with 1 warning (SonarLint S3885 - Assembly.LoadFrom) Sprint 2 Task #6 fully completed: Update tools/ projects
Archive Sprint 1 docs: - Moved sprint-1-checklist.md to docs/archive/sprint-1/ - Moved sprint-1-final-summary.md to docs/archive/sprint-1/ - Keep docs/ clean with only active sprint documentation - Preserve Sprint 1 history for reference (565 + 187 lines archived) Update API collections documentation: - Added all 6 modules to generated collections list - Users ✅ - Providers ✅ - Documents ✅ (new) - SearchProviders ✅ (new) - ServiceCatalogs ✅ (new) - Locations ✅ (new) - Updated structure examples with representative endpoints - Generator is dynamic (auto-detects from OpenAPI tags) Rationale: - Keep active documentation clean and focused - Archive completed sprint docs for historical reference - Reflect all current modules in tooling documentation Files changed: - docs/archive/sprint-1/ (new directory with 2 files) - tools/api-collections/README.md (updated module list)
… violations)
Root cause analysis:
- Tests were running in parallel and creating entities with same names
- Unique constraint on category/service names caused 23505 errors
- Test assertions comparing hardcoded names vs actual Guid-suffixed names
Solutions applied:
1. **Test Data Isolation**:
- Added Guid suffix to entity names in CreateServiceCategoryAsync()
- Added Guid suffix to entity names in CreateServiceAsync()
- Format: 'TestName_{guid}' ensures uniqueness across parallel tests
2. **Parallelization Control**:
- Added DisableParallelization=true to CollectionDefinition
- Created xunit.runner.json with maxParallelThreads=1
- Prevents race conditions in shared database schema
3. **Test Assertions**:
- Updated all .Name.Should().Be() assertions
- Changed from hardcoded strings to actual entity.Name
- Stored entity references for comparison in GetAllAsync tests
Files changed:
- ServiceCatalogsIntegrationTestBase.cs (helper methods with Guid)
- GlobalTestConfiguration.cs (DisableParallelization)
- xunit.runner.json (new file)
- ServiceCategoryRepositoryIntegrationTests.cs (6 tests fixed)
- ServiceRepositoryIntegrationTests.cs (4 tests fixed)
- ServiceCatalogsModuleApiIntegrationTests.cs (2 tests fixed)
- MeAjudaAi.Modules.ServiceCatalogs.Tests.csproj (copy xunit.runner.json)
Test results:
- Before: 18 failing, 14 passing
- After: 0 failing, 32 passing ✅
Impact:
- Eliminates flaky test failures in CI/CD
- Improves test isolation and reliability
- Pattern can be applied to other modules if needed
WalkthroughMulti-module update adding many Bruno API client specs, new document/search/location/service-catalog endpoints, extensive unit/integration/E2E tests, shared TestContainers (Azurite + PostGIS), migration/index adjustments, coverage tooling and scripts, CI/compose updates, documentation reorganization, and several handler/mapping and visibility changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Documents API
participant Auth as Keycloak / Token
participant DB as Database
participant Blob as Azure Blob (Azurite)
participant Jobs as Background Job Processor
Client->>API: POST /api/v1/documents (multipart/form)
API->>Auth: validate bearer token
Auth-->>API: userId, roles
API->>Blob: generate unique blob name & SAS URL (or forward file)
Blob-->>API: SAS URL / confirmation
API->>DB: create Document record (status: Uploaded)
DB-->>API: persisted document (id)
API->>Jobs: enqueue verification job (documentId)
Jobs-->>Blob: fetch blob for verification (async)
Jobs->>API: call verification endpoint or use internal service
Jobs->>DB: update Document status (PendingVerification -> Verified/Rejected)
DB-->>Jobs: confirmation
API-->>Client: 201 Created (documentId, status)
(High-level flow for upload → background verification: Client, API, Auth, Blob storage, DB, Background Job Processor.) Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/pr-validation.yml (2)
62-63: Fix misleading check-secrets reference in workflow comment.Line 63 claims secrets are "enforced by check-secrets", but this job no longer exists (removed per AI summary). The comment creates a false sense of security.
Apply this diff to fix the misleading message:
- name: Check Database Configuration - run: echo "✅ GitHub secrets configured (enforced by check-secrets)" + run: | + echo "ℹ️ Database configuration check" + echo "PostgreSQL will use provided secrets or fallback to test defaults"
18-20: STRICT_COVERAGE flag is a known blocker intentionally added in this PR and must be resolved before merging.The
STRICT_COVERAGE: falseenvironment variable (lines 18-20) was added in commit 9f55b63 as a deliberate temporary measure. According to the project's code coverage guide, this flag bypasses pipeline failure when coverage falls below the 70% minimum threshold.Current status: The PR fixes specific ServiceCatalogs integration test failures but does not improve overall test coverage percentages. The flag remains set to
false, allowing the pipeline to pass despite potential coverage < 70%.Action required:
- Run the test suite and capture current coverage metrics
- Add or update tests for all new/modified code until coverage meets 70% minimum (85% recommended per learnings)
- Remove the
STRICT_COVERAGE: falseflag and set totrueto enforce quality gates- Verify the PR passes with strict coverage enforcement enabled
The TODO is blocking and must be resolved before this PR can merge to main.
tools/MigrationTool/Program.cs (1)
372-375: Inconsistent fallback password.The fallback connection string uses
Password=postgreswhile the_connectionStringsdictionary at line 24-29 usesPassword=test123. This inconsistency could cause confusion when the fallback is used for a module not in the dictionary.// Fallback: generate connection string var dbName = $"meajudaai_{moduleName.ToLowerInvariant()}"; - return $"Host=localhost;Port=5432;Database={dbName};Username=postgres;Password=postgres"; + return $"Host=localhost;Port=5432;Database={dbName};Username=postgres;Password=test123";
🧹 Nitpick comments (11)
src/Modules/Documents/API/API.Client/README.md (2)
7-18: Specify language identifier for markdown code block.The first fenced code block should include a language identifier for proper syntax highlighting.
-``` +```yaml API.Client/ ├── collection.bru.example # Template de configuração (copie para collection.bru) ├── collection.bru # Configuração local (não versionado - criar local) ├── README.md # Documentação completa └── DocumentAdmin/ ├── UploadDocument.bru # POST /api/v1/documents ├── GetDocument.bru # GET /api/v1/documents/{id} ├── GetProviderDocuments.bru # GET /api/v1/documents/provider/{providerId} ├── VerifyDocument.bru # POST /api/v1/documents/{id}/verify └── RejectDocument.bru # POST /api/v1/documents/{id}/reject -``` +```
78-83: Specify language identifier for markdown code block.The second fenced code block should include a language identifier for proper syntax highlighting.
-``` +```yaml baseUrl: http://localhost:5000 accessToken: [AUTO-SET by shared setup] providerId: [CONFIGURE_AQUI] documentId: [CONFIGURE_AQUI após upload] -``` +```docs/sprint-2-tasks.md (2)
80-80: Minor: Localization - Replace "template" with "modelo".Line 80 uses the English word "template" which should be "modelo" in Portuguese documentation.
Apply this change:
-**Referência**: Usar tools/api-collections/Users/ como template +**Referência**: Usar tools/api-collections/Users/ como modelo
159-159: Minor: Grammar - Add missing article "os".Line 159 has a grammar issue in Portuguese. "todos campos" should be "todos os campos".
Apply this change:
- - [ ] Test: CreateProvider com todos campos válidos + - [ ] Test: CreateProvider com todos os campos válidosdocs/roadmap.md (1)
833-840: Clarify TODO items and coverage improvement deferral.Lines 833-840 indicate that coverage improvement tasks have been moved to Sprint 2. The notation could be slightly clearer:
Current:
#### 🆕 Coverage Improvement: MOVIDO PARA SPRINT 2 ✅ - ⏳ **TODO**: Aumentar coverage 35.11% → 80%+ (+200 unit tests) - ⏳ **TODO**: E2E test para provider indexing flowThis is clear enough, but consider using consistent formatting. The
**TODO**prefix with⏳is slightly redundant. Either use:
[ ] Tarefa (checkbox format), or⏳ Tarefa (status icon)But not both in the same section.
src/Modules/ServiceCatalogs/API/API.Client/README.md (1)
1-13: Optionally surface query parameters in the endpoints tableIf
GET /api/v1/catalogs/categoriessupportsactiveOnly(e.g.,/api/v1/catalogs/categories?activeOnly=true), consider either showing the parameter in the path or adding a short note/section about available filters so consumers don’t miss it.
Based on AI summary, this endpoint already supports that filter.src/Modules/ServiceCatalogs/Tests/Infrastructure/ServiceCatalogsIntegrationTestBase.cs (1)
68-70: Unique test entity names are solid; consider a shared helper and length guardAppending
Guid.NewGuid():Nto the base name is a simple and effective way to avoid collisions across tests, especially with shared databases. As a small improvement, you could:
- Factor this into a private helper like
GenerateUniqueName(string baseName)used by both methods.- If
ServiceCategory.Name/Service.Namehave a max length, trim the base name before appending the GUID to avoid future validation failures in tests.Also applies to: 89-92
src/Modules/Locations/API/API.Client/LocationQuery/GetAddressFromCep.bru (1)
1-35: Consider parameterizing the CEP path segment instead of hard-coding a valueTo make this Bru request reusable for arbitrary CEPs, you could use a variable in the URL, e.g.:
-get { - url: {{baseUrl}}/api/v1/locations/cep/36880000 +get { + url: {{baseUrl}}/api/v1/locations/cep/{{cep}}and define a default
cepvariable in the collection/environment. The rest of the definition (fallback chain and response example) looks good.src/Modules/SearchProviders/API/API.Client/README.md (2)
7-15: Add language specifier to fenced code block.The code block showing the collection structure should specify a language for better rendering and syntax highlighting.
Apply this diff:
-``` +```text API.Client/ ├── README.md # Documentação completa └── SearchAdmin/
38-44: Add language specifier to fenced code block.The variables block should specify a language for better rendering.
Apply this diff:
-``` +```text baseUrl: http://localhost:5000 accessToken: [AUTO-SET by shared setup]src/Modules/ServiceCatalogs/API/API.Client/CatalogAdmin/CreateService.bru (1)
30-40: Consider adding response structure for consistency.Other endpoint definitions in this PR (e.g., UploadDocument.bru, RejectDocument.bru) include expected response examples. Adding a similar response block here would improve consistency and developer experience.
docs { # Create Service Cria novo serviço em categoria (admin). ## Validações - Categoria deve existir e estar ativa - Nome único por categoria ## Status: 201 Created + + ## Resposta Esperada + ```json + { + "success": true, + "data": { + "id": "uuid", + "categoryId": "uuid", + "name": "Instalação de Tomadas", + "description": "Instalação de tomadas residenciais e comerciais", + "displayOrder": 1, + "isActive": true + } + } + ``` }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
.github/workflows/pr-validation.yml(5 hunks)docs/roadmap.md(6 hunks)docs/sprint-2-tasks.md(1 hunks)src/Modules/Documents/API/API.Client/DocumentAdmin/GetDocument.bru(1 hunks)src/Modules/Documents/API/API.Client/DocumentAdmin/GetProviderDocuments.bru(1 hunks)src/Modules/Documents/API/API.Client/DocumentAdmin/RejectDocument.bru(1 hunks)src/Modules/Documents/API/API.Client/DocumentAdmin/UploadDocument.bru(1 hunks)src/Modules/Documents/API/API.Client/DocumentAdmin/VerifyDocument.bru(1 hunks)src/Modules/Documents/API/API.Client/README.md(1 hunks)src/Modules/Locations/API/API.Client/LocationQuery/GetAddressFromCep.bru(1 hunks)src/Modules/Locations/API/API.Client/LocationQuery/ValidateCity.bru(1 hunks)src/Modules/Locations/API/API.Client/README.md(1 hunks)src/Modules/SearchProviders/API/API.Client/README.md(1 hunks)src/Modules/SearchProviders/API/API.Client/SearchAdmin/IndexProvider.bru(1 hunks)src/Modules/SearchProviders/API/API.Client/SearchAdmin/RemoveProvider.bru(1 hunks)src/Modules/SearchProviders/API/API.Client/SearchAdmin/SearchProviders.bru(1 hunks)src/Modules/ServiceCatalogs/API/API.Client/CatalogAdmin/CreateCategory.bru(1 hunks)src/Modules/ServiceCatalogs/API/API.Client/CatalogAdmin/CreateService.bru(1 hunks)src/Modules/ServiceCatalogs/API/API.Client/CatalogAdmin/ListCategories.bru(1 hunks)src/Modules/ServiceCatalogs/API/API.Client/README.md(1 hunks)src/Modules/ServiceCatalogs/Tests/GlobalTestConfiguration.cs(1 hunks)src/Modules/ServiceCatalogs/Tests/Infrastructure/ServiceCatalogsIntegrationTestBase.cs(2 hunks)src/Modules/ServiceCatalogs/Tests/Integration/ServiceCatalogsModuleApiIntegrationTests.cs(2 hunks)src/Modules/ServiceCatalogs/Tests/Integration/ServiceCategoryRepositoryIntegrationTests.cs(3 hunks)src/Modules/ServiceCatalogs/Tests/Integration/ServiceRepositoryIntegrationTests.cs(3 hunks)src/Modules/ServiceCatalogs/Tests/MeAjudaAi.Modules.ServiceCatalogs.Tests.csproj(1 hunks)src/Modules/ServiceCatalogs/Tests/xunit.runner.json(1 hunks)tools/MigrationTool/MigrationTool.csproj(1 hunks)tools/MigrationTool/Program.cs(1 hunks)tools/MigrationTool/packages.lock.json(1 hunks)tools/api-collections/README.md(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cs,csx}
📄 CodeRabbit inference engine (AGENT.md)
Use UUID v7 (time-ordered) instead of UUID v4 for entity IDs in .NET 9 projects
Files:
src/Modules/ServiceCatalogs/Tests/Integration/ServiceCatalogsModuleApiIntegrationTests.cssrc/Modules/ServiceCatalogs/Tests/Integration/ServiceCategoryRepositoryIntegrationTests.cssrc/Modules/ServiceCatalogs/Tests/Integration/ServiceRepositoryIntegrationTests.cstools/MigrationTool/Program.cssrc/Modules/ServiceCatalogs/Tests/Infrastructure/ServiceCatalogsIntegrationTestBase.cssrc/Modules/ServiceCatalogs/Tests/GlobalTestConfiguration.cs
**/*.cs
📄 CodeRabbit inference engine (AGENT.md)
**/*.cs: All public APIs should have XML documentation comments
Code style and formatting must adhere to .editorconfig rules
Files:
src/Modules/ServiceCatalogs/Tests/Integration/ServiceCatalogsModuleApiIntegrationTests.cssrc/Modules/ServiceCatalogs/Tests/Integration/ServiceCategoryRepositoryIntegrationTests.cssrc/Modules/ServiceCatalogs/Tests/Integration/ServiceRepositoryIntegrationTests.cstools/MigrationTool/Program.cssrc/Modules/ServiceCatalogs/Tests/Infrastructure/ServiceCatalogsIntegrationTestBase.cssrc/Modules/ServiceCatalogs/Tests/GlobalTestConfiguration.cs
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Add or update tests for all new functionality before submitting a PR
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/*.Tests/**/*.cs : Test method naming follows pattern: MethodName_Scenario_ExpectedResult
Applied to files:
src/Modules/ServiceCatalogs/Tests/Integration/ServiceCatalogsModuleApiIntegrationTests.cssrc/Modules/ServiceCatalogs/Tests/Integration/ServiceCategoryRepositoryIntegrationTests.cs
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Infrastructure/Persistence/**/*.cs : Use Entity Framework Core with dedicated DbContext per module, apply schema isolation via HasDefaultSchema()
Applied to files:
tools/MigrationTool/Program.cstools/MigrationTool/MigrationTool.csprojsrc/Modules/ServiceCatalogs/Tests/Infrastructure/ServiceCatalogsIntegrationTestBase.cs
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Infrastructure/Persistence/**DbContext.cs : Apply all EF Core configurations from assembly using modelBuilder.ApplyConfigurationsFromAssembly() in DbContext.OnModelCreating()
Applied to files:
tools/MigrationTool/Program.cstools/MigrationTool/MigrationTool.csproj
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Infrastructure/Persistence/**DbContext.cs : Database per module using schema isolation: Users module uses meajudaai_users schema, Providers uses meajudaai_providers schema
Applied to files:
tools/MigrationTool/Program.cstools/MigrationTool/MigrationTool.csproj
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/*.{cs,csx} : Use UUID v7 (time-ordered) instead of UUID v4 for entity IDs in .NET 9 projects
Applied to files:
tools/MigrationTool/MigrationTool.csproj
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Infrastructure/Persistence/**DbContext.cs : Each module must have a dedicated PostgreSQL schema named meajudaai_{module_name} in lowercase
Applied to files:
tools/MigrationTool/MigrationTool.csproj
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/*.Tests/**/*.cs : Use xUnit v3 for all test projects
Applied to files:
src/Modules/ServiceCatalogs/Tests/MeAjudaAi.Modules.ServiceCatalogs.Tests.csprojsrc/Modules/ServiceCatalogs/Tests/xunit.runner.json
🪛 LanguageTool
src/Modules/Documents/API/API.Client/README.md
[typographical] ~28-~28: Símbolo sem par: “[” aparentemente está ausente
Context: ... coleção ### 1. Pré-requisitos - Bruno instalado - ...
(UNPAIRED_BRACKETS)
docs/sprint-2-tasks.md
[locale-violation] ~11-~11: “packages” é um estrangeirismo. É preferível dizer “embalagens” ou “pacotes”.
Context: ...*: 35.11% (caiu após migration devido a packages.lock.json + generated code) - Tests...
(PT_BARBARISMS_REPLACE_PACKAGES)
[locale-violation] ~80-~80: “template” é um estrangeirismo. É preferível dizer “modelo”.
Context: ... Usar tools/api-collections/Users/ como template --- ## 🔧 Tarefa 2: Atualizar Tools P...
(PT_BARBARISMS_REPLACE_TEMPLATE)
[grammar] ~159-~159: Se ‘campos’ é um substantivo, ‘todos’ no plural exige um segundo artigo.
Context: ...rules) - [ ] Test: CreateProvider com todos campos válidos - [ ] Test: UpdateBusinessPro...
(TODOS_FOLLOWED_BY_NOUN_PLURAL)
[locale-violation] ~533-~533: “Targets” é um estrangeirismo. É preferível dizer “objetivos” ou “alvos”.
Context: ...70%+* (endpoints, middleware) Test Targets: - ✅ Skipped tests: 11 → 0-2 ...
(PT_BARBARISMS_REPLACE_TARGETS)
[grammar] ~540-~540: Possível erro de concordância de número.
Context: ...módulos = 6 módulos completos - ✅ Total endpoints: ~35 endpoints documentados *Tools...
(GENERAL_NUMBER_AGREEMENT_ERRORS)
src/Modules/Locations/API/API.Client/README.md
[uncategorized] ~3-~3: Encontrada possível ausência de vírgula.
Context: ...ção Bruno para serviços de localização (CEP lookup, validação geográfica). ## Endp...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
docs/roadmap.md
[inconsistency] ~18-~18: Nesta situação o mês é redundante.
Context: ...DIAS 1-6 CONCLUÍDOS, FINALIZANDO) - ⏳ 3 Dez - 16 Dez: Sprint 2 - Test Coverage 80% + API C...
(MONTH_REDUNDANCY)
[inconsistency] ~67-~67: Nesta situação o mês é redundante.
Context: ...NCLUÍDOS | | Sprint 2 | 2 semanas | 3 Dez - 16 Dez | Test Coverage 80% + API Collections +...
(MONTH_REDUNDANCY)
[inconsistency] ~68-~68: Nesta situação o mês é redundante.
Context: ...lanejado | | Sprint 3 | 2 semanas | 17 Dez - 31 Dez | Blazor Admin Portal (Web) | ⏳ Planeja...
(MONTH_REDUNDANCY)
[typographical] ~760-~760: Símbolo sem par: “]” aparentemente está ausente
Context: ...-coverage` (criada) Documentação: [docs/skipped-tests-analysis.md](./testin...
(UNPAIRED_BRACKETS)
[uncategorized] ~835-~835: Se é uma abreviatura, falta um ponto. Se for uma expressão, coloque entre aspas.
Context: ...0%+ (+200 unit tests) - ⏳ TODO: E2E test para provider indexing flow - ⏳ *TODO...
(ABREVIATIONS_PUNCTUATION)
[locale-violation] ~897-~897: “packages” é um estrangeirismo. É preferível dizer “embalagens” ou “pacotes”.
Context: ...l: 35.11% (caiu após migration devido a packages.lock.json + generated code) - Skipped t...
(PT_BARBARISMS_REPLACE_PACKAGES)
🪛 markdownlint-cli2 (0.18.1)
src/Modules/Documents/API/API.Client/README.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
src/Modules/SearchProviders/API/API.Client/README.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
38-38: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Security Scan
- GitHub Check: Code Quality Checks
🔇 Additional comments (29)
src/Modules/Documents/API/API.Client/README.md (1)
1-109: Documentation is comprehensive and well-structured.The README provides clear guidance on collection structure, setup, endpoints, authorization policies, supported document types, and troubleshooting. The Portuguese documentation with bilingual elements aligns well with the project's context.
However, since the PR objective is "Improve tests coverage," verify that this documentation is accompanied by actual test cases or test infrastructure improvements. Based on the learnings, all new functionality should include tests before submission.
Are corresponding unit tests, integration tests, or API client test cases included in this PR for the newly documented endpoints?
docs/sprint-2-tasks.md (1)
1-560: Planning document is comprehensive but implementation PR should include actual tests.This PR introduces a well-structured Sprint 2 planning document covering 6 major tasks (API collections, tools updates, test fixes, unit/integration/E2E tests) with clear examples and success criteria. However, the document itself contains only checkboxes and planning—no actual test implementations.
Observation: Per your learnings, tests for new functionality should be added before submitting a PR. Consider whether:
- This planning PR is intentional (submitted to gather feedback before implementation), or
- A follow-up implementation PR should immediately follow with the actual test code
Scope Assessment: The target of +200 unit tests and 80%+ coverage within a 2-week sprint is ambitious but appears realistic given the detailed task breakdown. Ensure the development team has capacity and dependencies are clear.
Please verify:
- Are follow-up PRs planned to implement these tasks?
- Do the skipped tests (Tasks 3) have clear resolution paths (PostGIS setup, Azurite config)?
- Are the .bru API collection templates sufficient for team implementation?
docs/roadmap.md (3)
17-20: Excellent: Sprint timeline is clearly updated and integrated.The roadmap now clearly reflects:
- ✅ Sprint 0 (Migration .NET 10): CONCLUÍDO
- 🔄 Sprint 1 (Geographic Restriction): DIAS 1-6 CONCLUÍDOS, EM FINALIZAÇÂO
- ⏳ Sprint 2 (Test Coverage): PLANEJADO
- ⏳ Sprints 3-6: PLANEJADO
The updated timeline provides clear visibility into project progress and upcoming milestones. The integration with
docs/sprint-2-tasks.mdis well-structured.
66-71: Roadmap sprint table is well-organized and consistent.The sprint table now includes detailed timeline, objectives, and status for all sprints through MVP launch (31 Mar 2025). The structure is clear and provides excellent visibility for stakeholders.
Minor observation: The table includes both duration (e.g., "2 semanas") and date ranges which is redundant but not problematic—static analysis flagged this but it's actually good for clarity.
756-883: Sprint 1 recap and Sprint 2 introduction are comprehensive.The detailed summary of Sprint 1 (lines 756-883) provides excellent context:
- Clear status breakdown (DIAS 1-6 CONCLUÍDOS)
- Specific achievements (geographic restriction, module APIs, skipped tests reduced)
- Integration details (Providers ↔ Documents, Providers ↔ SearchProviders)
- Bugs fixed (AUTH race condition, IBGE fallback)
Sprint 2 introduction (lines 885-954) properly sets context for the new planning document, with clear objectives and connections to
docs/sprint-2-tasks.md.One concern: The scope of work described in Sprint 1 (lines 756-883) appears substantial. Ensure:
- Completion validation is clear (code review pending as noted)
- Dependencies for Sprint 2 are resolved before sprint start
Please confirm Sprint 1 completion and code review status before Sprint 2 kickoff.
src/Modules/ServiceCatalogs/Tests/MeAjudaAi.Modules.ServiceCatalogs.Tests.csproj (1)
56-60: xUnit runner config correctly wired into the test projectCopying
xunit.runner.jsonto the output viaNone Update+PreserveNewestis the right pattern and will ensure the runner settings are honored when executing this test assembly.src/Modules/Locations/API/API.Client/README.md (1)
1-19: Locations API Client README looks clear and aligned with the endpointsThe endpoints table and provider/IBGE notes are concise and match the described Locations API surface; no changes needed here.
src/Modules/ServiceCatalogs/Tests/Integration/ServiceCatalogsModuleApiIntegrationTests.cs (1)
31-31: Assertions againstcategory.Name/service.Namecorrectly track dynamic test dataUsing the actual
Namefrom the created entities keeps these integration tests aligned with the unique-name strategy and avoids brittle string literals.Also applies to: 99-99
src/Modules/ServiceCatalogs/Tests/Integration/ServiceRepositoryIntegrationTests.cs (1)
33-33: Repository integration tests now correctly assert against created servicesSwitching to
service.Namein assertions and inExistsWithNameAsyncmakes these tests resilient to the unique-name generation in the shared test infrastructure and removes hidden coupling to hard-coded literals.Also applies to: 55-57, 64-66, 115-117
src/Modules/ServiceCatalogs/Tests/xunit.runner.json (1)
1-8: xUnit runner configuration matches the sequential test strategyUsing the current runner schema with
methodDisplay: "method"and fully disabling parallelization (single thread, no assembly/collection parallelization) aligns well with the integration tests that share database state in this module.src/Modules/Locations/API/API.Client/LocationQuery/ValidateCity.bru (1)
1-45: LGTM! Well-structured API client definition.The ValidateCity endpoint definition is clear, complete, and well-documented. The example data and Portuguese documentation appropriately describe the city validation functionality.
src/Modules/Documents/API/API.Client/DocumentAdmin/VerifyDocument.bru (1)
1-76: LGTM! Comprehensive API definition with clear documentation.The VerifyDocument endpoint is well-defined with thorough documentation covering authorization, effects, domain events, and provider impact. The Portuguese documentation clearly explains the verification workflow and its consequences.
src/Modules/ServiceCatalogs/API/API.Client/CatalogAdmin/CreateCategory.bru (1)
1-40: LGTM! Clean and well-documented API definition.The CreateCategory endpoint definition is clear and follows consistent patterns. The documentation appropriately describes the admin operation and body parameters.
src/Modules/Documents/API/API.Client/DocumentAdmin/GetProviderDocuments.bru (1)
1-74: LGTM! Well-documented endpoint with helpful API references.The GetProviderDocuments endpoint is clearly defined with appropriate authorization policy (SelfOrAdmin). The documentation helpfully references the status counts helper API for obtaining document statistics.
src/Modules/Documents/API/API.Client/DocumentAdmin/GetDocument.bru (1)
1-60: LGTM! Clear and consistent API definition.The GetDocument endpoint follows the same well-structured pattern as other Documents API endpoints, with clear authorization policy and comprehensive response documentation.
src/Modules/ServiceCatalogs/Tests/GlobalTestConfiguration.cs (1)
7-10: Good defensive approach to prevent test conflicts.Disabling parallelization for ServiceCatalogs integration tests is a reasonable approach to avoid duplicate key constraint violations during category/service creation. The clear comment documents the rationale for future maintainers.
Note: This will increase test execution time, but given the integration test nature (real database operations), the reliability benefit outweighs the performance cost.
src/Modules/ServiceCatalogs/Tests/Integration/ServiceCategoryRepositoryIntegrationTests.cs (3)
20-36: Excellent improvement to test reliability.Changing the assertion from a hard-coded string to
category.Namemakes the test more robust and resilient to changes in the test infrastructure's naming strategy. This aligns well with the GUID-based unique naming introduced in the test base class.
51-67: Good refactoring for dynamic name handling.Storing the created categories in variables and using their actual names in assertions (
category1.Name, etc.) is a solid improvement. This approach correctly handles the GUID-based unique naming in the test infrastructure and makes assertions accurate and maintainable.
87-98: Consistent pattern application.The update to use
category.Namein the existence check is consistent with the improvements in other test methods and correctly handles dynamically generated names.src/Modules/SearchProviders/API/API.Client/SearchAdmin/RemoveProvider.bru (1)
1-29: LGTM!The DELETE endpoint definition is well-structured with appropriate bearer authentication, clear AdminOnly authorization documentation, and correct status codes for a delete operation.
src/Modules/Documents/API/API.Client/DocumentAdmin/UploadDocument.bru (1)
1-76: LGTM!Comprehensive API definition for document upload with proper multipart form handling, bearer authentication, and thorough documentation covering parameters, status transitions, and response structure.
src/Modules/ServiceCatalogs/API/API.Client/CatalogAdmin/ListCategories.bru (1)
1-22: LGTM!Simple and correct public endpoint definition for listing categories with appropriate
auth: nonesetting and clear documentation.src/Modules/SearchProviders/API/API.Client/SearchAdmin/SearchProviders.bru (1)
1-55: LGTM!Well-structured public search endpoint with comprehensive request body example and thorough documentation covering parameters, ranking criteria, and status codes.
tools/api-collections/README.md (1)
53-143: LGTM!Documentation updates correctly reflect the new API collections and module endpoints introduced in this PR, maintaining consistent formatting with existing documentation.
src/Modules/SearchProviders/API/API.Client/SearchAdmin/IndexProvider.bru (1)
1-33: LGTM!Well-documented admin endpoint for manual provider indexing with clear explanation of when to use it (re-indexing vs automatic indexation via integration events).
src/Modules/Documents/API/API.Client/DocumentAdmin/RejectDocument.bru (1)
1-82: LGTM!Excellent API definition with comprehensive documentation covering business logic implications (domain events, provider status impact), common rejection reasons, and clear request/response examples.
tools/MigrationTool/MigrationTool.csproj (2)
11-15: LGTM - Central Package Management pattern.Removing explicit version constraints from
PackageReferenceentries is consistent with Central Package Management viaDirectory.Packages.props. This ensures version consistency across the solution.
22-24: LGTM - New module references added.The new
ProjectReferenceentries for Documents, SearchProviders, and ServiceCatalogs infrastructure modules align with the PR's objective of expanding test coverage across modules. The paths follow the established pattern.tools/MigrationTool/packages.lock.json (1)
1-1394: Lock file added for reproducible builds.Adding
packages.lock.jsonis a good practice for ensuring deterministic package restoration across environments. The comprehensive dependency graph is correctly captured.
- Changed image from postgres:15-alpine to postgis/postgis:15-alpine - Removed Skip attributes from 6 SearchProviders E2E tests - PostGIS extension now available for geospatial queries - Tests use ST_DWithin/ST_Distance for location-based search
…all environments Consistency improvements: - Updated TestContainers to use postgis/postgis:16-3.4 (same as CI/CD) - Updated aspire-ci-cd.yml workflow (was using postgres:15 without PostGIS) - Updated docker-compose files: - environments/development.yml (main postgres) - environments/testing.yml (postgres-test) - standalone/postgres-only.yml (both postgres services) Rationale: - ci-cd.yml and pr-validation.yml already use postgis/postgis:16-3.4 - SearchProviders module requires PostGIS for geospatial queries - Using same image version ensures consistent behavior across: - Local development - Integration tests - E2E tests - CI/CD pipelines Benefits: - PostGIS E2E tests now pass (6 tests fixed) - No environment-specific bugs due to version mismatch - Simpler troubleshooting (single source of truth) Technical notes: - PostGIS extension is created automatically by SearchProviders migration - Image size slightly larger but necessary for geospatial features - All other PostgreSQL instances (Keycloak DB, etc.) remain postgres:16
Implemented TestContainers.Azurite support: - Added Testcontainers.Azurite package (v4.7.0) to Directory.Packages.props - Added package reference to Shared.Tests and E2E.Tests projects - Updated SharedTestContainers to include Azurite container - Updated TestContainerTestBase to configure Azurite for E2E tests Changes in SharedTestContainers: - Added AzuriteContainer field and property - Initialize Azurite with mcr.microsoft.com/azure-storage/azurite:latest - Start Azurite in parallel with PostgreSQL and Redis (Task.WhenAll) - Stop Azurite in DisposeAsync cleanup Changes in TestContainerTestBase: - Added Azurite container initialization - Configure Azure:Storage:ConnectionString from Azurite - Proper cleanup in DisposeAsync Fixes: - Removed Skip attribute from DocumentsVerificationE2ETests - Documents module now uses Azurite blob storage in tests - Proper Docker networking (no localhost mismatch) Benefits: - Documents upload/verification tests now work in E2E - Consistent blob storage behavior across local and CI/CD - No external Azure Storage dependency for tests Next: Build will complete once file locks are released
… in DisposeAsync - Add ConfigurableTestAuthenticationHandler.ClearConfiguration() call in TestContainerTestBase.DisposeAsync() - Remove Skip attributes from 2 E2E tests previously failing due to authentication state pollution - Root cause: AsyncLocal context was not being cleared between tests when thread pool reused threads - Tests fixed: - ModuleIntegrationTests.CreateAndUpdateUser_ShouldMaintainConsistency - CrossModuleCommunicationE2ETests.ModuleToModuleCommunication_ShouldWorkForDifferentConsumers (4 Theory cases) Result: 5 skipped → 0 skipped (8 total tests now passing)
…dStates configuration - Remove RJ from AllowedStates to prevent state-level bypass - Update AllowedCities to use 'City|State' format for precise matching - Root cause: middleware fallback validation allowed entire RJ state when IBGE failed - Expected behavior: only specific cities should be allowed, not entire states Configuration changes in ApiTestBase: - AllowedStates: [MG, RJ, ES] → [MG, ES] (remove RJ state-level access) - AllowedCities format: 'Muriaé' → 'Muriaé|MG' (add state specificity) - This ensures Rio de Janeiro (not in list) is blocked even when in RJ state Remove Skip from GeographicRestriction_WhenIbgeUnavailableAndCityNotAllowed_ShouldDenyAccess Result: 1 skipped → 0 skipped (4 IBGE tests passing)
- Move skipped-tests-analysis.md to docs/archive/sprint-1/ - Move skipped-tests-tracker.md to docs/archive/sprint-1/ - Create comprehensive skipped-tests-resolution-summary.md documenting: - All 31 tests fixed across 6 commits - Detailed problem/solution for each category - Configuration changes and technical details - Impact summary and key learnings - Remaining acceptable skips (9 intentional) Sprint 2 test quality improvement complete: - 30 tests fixed (18 ServiceCatalogs + 6 PostGIS + 1 Azurite + 5 Race + 1 IBGE) - 100% of targeted skipped tests resolved - PostgreSQL standardized across entire project - New test patterns documented for future reference
MigrationTool improvements: - Add EFCore.NamingConventions package reference - Apply UseSnakeCaseNamingConvention() to match other modules' table/column naming - Replace fragile GetMethod reflection with robust enumeration-based lookup - Add explicit null check and throw descriptive exception if AddDbContext method not found - Wrap Invoke in try/catch to surface TargetInvocationException with clear context - Fix inconsistent fallback password (postgres → test123 to match _connectionStrings) Workflow improvements (pr-validation.yml): - Remove misleading 'enforced by check-secrets' comment (job no longer exists) - Add Validate Secrets Configuration step with repository/fork detection - Warn when POSTGRES_PASSWORD secret missing but allow test defaults - Improve transparency about secret configuration vs safe defaults Documentation fixes: - Fix roadmap.md link text to match actual target path (testing/skipped-tests-analysis.md) Addresses code review feedback from coderabbitai analysis
API Client README improvements:
- Add language identifiers (yaml/text) to all fenced code blocks for proper syntax highlighting
- Documents API.Client: yaml blocks for structure and variables
- SearchProviders API.Client: text blocks for structure and variables
Bruno collection improvements:
- ServiceCatalogs CreateService.bru: add expected response structure example for consistency
- Locations GetAddressFromCep.bru: parameterize CEP with {{cep}} variable for reusability
Portuguese localization and grammar:
- sprint-2-tasks.md: replace 'template' with 'modelo' (line 80)
- sprint-2-tasks.md: add missing article 'os' in 'todos os campos' (line 159)
Roadmap cleanup:
- Remove redundant **TODO** prefix from coverage improvement tasks (⏳ status icon is sufficient)
Addresses all documentation feedback from coderabbitai review
…ared helper ServiceCatalogsIntegrationTestBase improvements: - Extract GenerateUniqueName() helper method to reduce duplication - Add max length guard (default 200 chars) to prevent validation failures - Truncate base name if needed before appending GUID (32 chars + underscore) - Improve code maintainability with centralized unique name generation logic Benefits: - DRY principle: single source of truth for unique name generation - Safety: automatic truncation prevents exceeding database column constraints - Flexibility: configurable maxTotalLength parameter for different entity types - Maintainability: easier to modify unique name strategy in one place All 141 ServiceCatalogs tests passing ✅ Addresses code review suggestion from coderabbitai
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (2)
docs/roadmap.md (1)
760-760:⚠️ Fix the documentation link display text on line 760.The display text
[testing/skipped-tests-analysis.md]should match the URL path. The URL correctly points to./testing/skipped-tests-analysis.md, so update the display text to match:-**Documentação**: [testing/skipped-tests-analysis.md](./testing/skipped-tests-analysis.md) +**Documentação**: [docs/testing/skipped-tests-analysis.md](./testing/skipped-tests-analysis.md)This issue was noted in a previous review and is still unresolved. This is the path where the comprehensive skipped tests analysis lives (432 lines), not the shorter file at
docs/skipped-tests-analysis.md(265 lines)..github/workflows/pr-validation.yml (1)
32-34:⚠️ Secret fallback pattern persists without strict production validation.The placeholders (
'test123','postgres','meajudaai_test') remain inline across multiple steps. While the new "Validate Secrets Configuration" step (lines 62–80) adds logging, it does not enforce strict failure in production CI contexts (main repo, pull_request events).Concern (from previous review): This allows misconfigured CI on the main repository to silently use hardcoded defaults instead of failing explicitly. The new validation step logs warnings but continues execution, defeating the intent of strict secrets validation.
Recommendation:
- Exit with error code 1 when running on main repo (
github.repository == "frigini/MeAjudaAi") and critical secrets are missing (Lines 71–75).- Reserve lenient defaults only for forks or workflow_dispatch contexts.
Example fix for the validation step:
if [ "${{ github.repository }}" = "frigini/MeAjudaAi" ] && [ "${{ github.event_name }}" = "pull_request" ]; then if [ -z "${{ secrets.POSTGRES_PASSWORD }}" ]; then echo "❌ POSTGRES_PASSWORD secret required for production CI on main repo" exit 1 # ← Currently missing fi fiAlso applies to: 109-110, 144-146, 158-160, 164-166
🧹 Nitpick comments (7)
src/Modules/Documents/API/API.Client/README.md (1)
78-83: Specify language identifier for code block.The fenced code block lacks a language specifier, which affects syntax highlighting and markdown compliance.
Apply this diff to add a language identifier:
-``` +```yaml baseUrl: http://localhost:5000 accessToken: [AUTO-SET by shared setup] providerId: [CONFIGURE_AQUI] documentId: [CONFIGURE_AQUI após upload] -``` +```tools/MigrationTool/Program.cs (1)
120-130: MissingUseNetTopologySuite()may cause issues for PostGIS-enabled modules.The
SearchProvidersmodule configuresUseNetTopologySuite()for PostGIS support (seesrc/Modules/SearchProviders/Infrastructure/Extensions.cslines 42-43). When the migration tool builds the model forSearchProvidersDbContext, it may fail if geometry types are referenced in the model configuration.Consider conditionally adding NetTopologySuite for modules that require it:
options.UseNpgsql(connectionString, npgsqlOptions => { npgsqlOptions.MigrationsHistoryTable("__EFMigrationsHistory", contextInfo.SchemaName); + // Enable NetTopologySuite for modules using PostGIS (e.g., SearchProviders) + if (contextInfo.ModuleName == "SearchProviders") + { + npgsqlOptions.UseNetTopologySuite(); + } }) .UseSnakeCaseNamingConvention();This also requires adding
Npgsql.EntityFrameworkCore.PostgreSQL.NetTopologySuitepackage reference to the project.#!/bin/bash # Verify if SearchProviders model uses geometry types that require NetTopologySuite echo "=== Checking for geometry type usage in SearchProviders ===" rg -n "UseNetTopologySuite|NetTopologySuite|Geometry|Point|LineString|Polygon" src/Modules/SearchProviders --type cs -C 2src/Modules/Locations/API/API.Client/LocationQuery/GetAddressFromCep.bru (1)
13-35: Consider documenting CEP format requirements.The documentation shows a response example with a hyphenated CEP (
"36880-000"), but doesn't specify whether the{{cep}}request parameter should include the hyphen or not. Adding a brief note about accepted CEP formats would help API consumers avoid format-related errors.Optional improvement to the docs section:
docs { # Get Address from CEP Busca endereço completo via CEP brasileiro. + + ## Request + - CEP format: accepts both `01310-100` and `01310100` ## Fallback Chaintests/MeAjudaAi.Integration.Tests/Modules/Locations/IbgeUnavailabilityTests.cs (1)
82-117: Re‑enabling this IBGE unavailability test makes sense; consider a precondition assert for clearer failuresUnskipping this
[Fact]is aligned with the investigation comment and improves coverage. To keep CI failures actionable, you might add an explicit check at the top of the test (or inApiTestBase) that the GeographicRestriction feature flag and test environment are configured as expected, and fail with a clear message if not. This would turn config drift into an immediately understandable failure instead of a seemingly incorrect business‑logic regression..github/workflows/pr-validation.yml (1)
65-65: Fix YAML linting violations (trailing spaces and line length).Static analysis reported:
- Line 65: trailing spaces
- Line 67: line too long (122 > 120 characters)
- Line 69: trailing spaces
- Line 79: trailing spaces
These are minor formatting issues but should be cleaned up before merge to maintain consistent linting standards.
Suggested fixes:
# Line 67 is 122 chars; shorten or split to ≤120: - if [ "${{ github.repository }}" = "frigini/MeAjudaAi" ] && \ [ "${{ github.event_name }}" = "pull_request" ]; then # Remove trailing spaces on lines 65, 69, 79Also applies to: 67-67, 69-69, 79-79
tests/MeAjudaAi.E2E.Tests/Integration/ModuleIntegrationTests.cs (1)
49-104: Test covers the end‑to‑end flow, but you could assert updated data when possible.The flow (create → optional update → get) is reasonable and defensive about existing data. If you want stronger coverage for “ShouldMaintainConsistency”, consider—only when
createResponseandgetResponseareOK—asserting that the returned user payload reflects the updated name/email, not just the status codes. This would tighten the contract without impacting the current resilience to conflicts.tests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cs (1)
3-3: Azurite integration looks good; consider pinning the image tag and double‑check endpoint usage.Nice reuse of the shared container pattern for Azurite and parallel startup/teardown. Two small follow‑ups to keep things robust:
- Avoid
"mcr.microsoft.com/azure-storage/azurite:latest"in tests; pin a specific tag (the one used in CI) to prevent surprises whenlatestchanges.- Ensure tests that consume Azurite use the container’s mapped endpoints/connection string as exposed by
AzuriteContainer(and that the default port bindings fromAzuriteBuildermatch what the tests expect).Functionally this is fine; these are just to harden reproducibility and avoid subtle test env drift.
Also applies to: 16-16, 33-43, 78-82, 88-93, 105-109, 153-162
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
.github/workflows/aspire-ci-cd.yml(1 hunks).github/workflows/pr-validation.yml(6 hunks)Directory.Packages.props(1 hunks)docs/archive/sprint-1/skipped-tests-resolution-summary.md(1 hunks)docs/roadmap.md(6 hunks)docs/sprint-2-tasks.md(1 hunks)infrastructure/compose/environments/development.yml(1 hunks)infrastructure/compose/environments/testing.yml(1 hunks)infrastructure/compose/standalone/postgres-only.yml(2 hunks)src/Modules/Documents/API/API.Client/README.md(1 hunks)src/Modules/Documents/Tests/packages.lock.json(2 hunks)src/Modules/Locations/API/API.Client/LocationQuery/GetAddressFromCep.bru(1 hunks)src/Modules/Providers/Tests/packages.lock.json(2 hunks)src/Modules/SearchProviders/API/API.Client/README.md(1 hunks)src/Modules/SearchProviders/Tests/packages.lock.json(2 hunks)src/Modules/ServiceCatalogs/API/API.Client/CatalogAdmin/CreateService.bru(1 hunks)src/Modules/ServiceCatalogs/Tests/Infrastructure/ServiceCatalogsIntegrationTestBase.cs(3 hunks)src/Modules/ServiceCatalogs/Tests/packages.lock.json(2 hunks)src/Modules/Users/Tests/packages.lock.json(2 hunks)tests/MeAjudaAi.E2E.Tests/Base/TestContainerTestBase.cs(6 hunks)tests/MeAjudaAi.E2E.Tests/CrossModuleCommunicationE2ETests.cs(1 hunks)tests/MeAjudaAi.E2E.Tests/Integration/ModuleIntegrationTests.cs(1 hunks)tests/MeAjudaAi.E2E.Tests/Integration/SearchProvidersEndpointTests.cs(6 hunks)tests/MeAjudaAi.E2E.Tests/MeAjudaAi.E2E.Tests.csproj(1 hunks)tests/MeAjudaAi.E2E.Tests/Modules/DocumentsVerificationE2ETests.cs(1 hunks)tests/MeAjudaAi.E2E.Tests/packages.lock.json(2 hunks)tests/MeAjudaAi.Integration.Tests/Base/ApiTestBase.cs(1 hunks)tests/MeAjudaAi.Integration.Tests/Modules/Locations/IbgeUnavailabilityTests.cs(1 hunks)tests/MeAjudaAi.Integration.Tests/packages.lock.json(2 hunks)tests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cs(6 hunks)tests/MeAjudaAi.Shared.Tests/MeAjudaAi.Shared.Tests.csproj(1 hunks)tests/MeAjudaAi.Shared.Tests/packages.lock.json(1 hunks)tools/MigrationTool/MigrationTool.csproj(1 hunks)tools/MigrationTool/Program.cs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{cs,csx}
📄 CodeRabbit inference engine (AGENT.md)
Use UUID v7 (time-ordered) instead of UUID v4 for entity IDs in .NET 9 projects
Files:
tests/MeAjudaAi.E2E.Tests/Integration/SearchProvidersEndpointTests.cstests/MeAjudaAi.Integration.Tests/Modules/Locations/IbgeUnavailabilityTests.cstests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cstools/MigrationTool/Program.cstests/MeAjudaAi.Integration.Tests/Base/ApiTestBase.cstests/MeAjudaAi.E2E.Tests/Integration/ModuleIntegrationTests.cstests/MeAjudaAi.E2E.Tests/Modules/DocumentsVerificationE2ETests.cstests/MeAjudaAi.E2E.Tests/CrossModuleCommunicationE2ETests.cssrc/Modules/ServiceCatalogs/Tests/Infrastructure/ServiceCatalogsIntegrationTestBase.cstests/MeAjudaAi.E2E.Tests/Base/TestContainerTestBase.cs
**/*.cs
📄 CodeRabbit inference engine (AGENT.md)
**/*.cs: All public APIs should have XML documentation comments
Code style and formatting must adhere to .editorconfig rules
Files:
tests/MeAjudaAi.E2E.Tests/Integration/SearchProvidersEndpointTests.cstests/MeAjudaAi.Integration.Tests/Modules/Locations/IbgeUnavailabilityTests.cstests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cstools/MigrationTool/Program.cstests/MeAjudaAi.Integration.Tests/Base/ApiTestBase.cstests/MeAjudaAi.E2E.Tests/Integration/ModuleIntegrationTests.cstests/MeAjudaAi.E2E.Tests/Modules/DocumentsVerificationE2ETests.cstests/MeAjudaAi.E2E.Tests/CrossModuleCommunicationE2ETests.cssrc/Modules/ServiceCatalogs/Tests/Infrastructure/ServiceCatalogsIntegrationTestBase.cstests/MeAjudaAi.E2E.Tests/Base/TestContainerTestBase.cs
**/*.Tests/**/*.cs
📄 CodeRabbit inference engine (AGENT.md)
**/*.Tests/**/*.cs: Use xUnit v3 for all test projects
Structure unit tests using Arrange-Act-Assert pattern: Arrange (test data setup), Act (execute operation), Assert (verify outcome)
Test method naming follows pattern: MethodName_Scenario_ExpectedResult
Files:
tests/MeAjudaAi.E2E.Tests/Integration/SearchProvidersEndpointTests.cstests/MeAjudaAi.Integration.Tests/Modules/Locations/IbgeUnavailabilityTests.cstests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cstests/MeAjudaAi.Integration.Tests/Base/ApiTestBase.cstests/MeAjudaAi.E2E.Tests/Integration/ModuleIntegrationTests.cstests/MeAjudaAi.E2E.Tests/Modules/DocumentsVerificationE2ETests.cstests/MeAjudaAi.E2E.Tests/CrossModuleCommunicationE2ETests.cstests/MeAjudaAi.E2E.Tests/Base/TestContainerTestBase.cs
Directory.Packages.props
📄 CodeRabbit inference engine (AGENT.md)
All package versions must be defined centrally in Directory.Packages.props
Files:
Directory.Packages.props
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Add or update tests for all new functionality before submitting a PR
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/*.Tests/**/*.cs : Use xUnit v3 for all test projects
Applied to files:
tests/MeAjudaAi.E2E.Tests/MeAjudaAi.E2E.Tests.csproj
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Infrastructure/Persistence/**DbContext.cs : Database per module using schema isolation: Users module uses meajudaai_users schema, Providers uses meajudaai_providers schema
Applied to files:
tests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cstools/MigrationTool/Program.cstools/MigrationTool/MigrationTool.csproj
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Infrastructure/Persistence/**/*.cs : Use Entity Framework Core with dedicated DbContext per module, apply schema isolation via HasDefaultSchema()
Applied to files:
tools/MigrationTool/Program.cstools/MigrationTool/MigrationTool.csprojtests/MeAjudaAi.E2E.Tests/Base/TestContainerTestBase.cs
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Infrastructure/Persistence/**DbContext.cs : Apply all EF Core configurations from assembly using modelBuilder.ApplyConfigurationsFromAssembly() in DbContext.OnModelCreating()
Applied to files:
tools/MigrationTool/Program.cstools/MigrationTool/MigrationTool.csproj
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Infrastructure/Persistence/**DbContext.cs : Each module must have a dedicated PostgreSQL schema named meajudaai_{module_name} in lowercase
Applied to files:
tools/MigrationTool/Program.cstools/MigrationTool/MigrationTool.csproj
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Domain/Events/**/*.cs : Name Domain Events following the pattern [Entity][Action]DomainEvent (e.g., UserRegisteredDomainEvent, ProviderDeletedDomainEvent)
Applied to files:
tests/MeAjudaAi.E2E.Tests/Integration/ModuleIntegrationTests.cs
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/*.Tests/**/*.cs : Test method naming follows pattern: MethodName_Scenario_ExpectedResult
Applied to files:
tests/MeAjudaAi.E2E.Tests/Modules/DocumentsVerificationE2ETests.cs
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/*.{cs,csx} : Use UUID v7 (time-ordered) instead of UUID v4 for entity IDs in .NET 9 projects
Applied to files:
tools/MigrationTool/MigrationTool.csproj
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to Directory.Packages.props : All package versions must be defined centrally in Directory.Packages.props
Applied to files:
Directory.Packages.props
🧬 Code graph analysis (4)
tests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cs (2)
tests/MeAjudaAi.E2E.Tests/Base/TestContainerTestBase.cs (12)
Task(227-254)Task(256-282)Task(293-298)Task(307-312)Task(321-325)Task(330-334)Task(339-343)Task(348-353)Task(358-363)Task(396-397)Task(406-407)Task(482-494)tests/MeAjudaAi.Shared.Tests/Base/IntegrationTestBase.cs (4)
Task(31-31)Task(74-91)Task(114-114)Task(119-119)
tools/MigrationTool/Program.cs (2)
src/Modules/Documents/Infrastructure/Extensions.cs (5)
IServiceCollection(21-29)IServiceCollection(31-81)IServiceCollection(83-113)IServiceCollection(115-121)IServiceCollection(126-132)src/Modules/SearchProviders/Infrastructure/Extensions.cs (2)
IServiceCollection(22-66)IServiceCollection(71-77)
tests/MeAjudaAi.E2E.Tests/Modules/DocumentsVerificationE2ETests.cs (1)
tests/MeAjudaAi.E2E.Tests/CrossModuleCommunicationE2ETests.cs (2)
Fact(114-144)Fact(146-175)
tests/MeAjudaAi.E2E.Tests/Base/TestContainerTestBase.cs (1)
tests/MeAjudaAi.Shared.Tests/Auth/ConfigurableTestAuthenticationHandler.cs (2)
ConfigurableTestAuthenticationHandler(14-271)ClearConfiguration(220-229)
🪛 GitHub Check: YAML Syntax Check
.github/workflows/pr-validation.yml
[warning] 79-79:
79:1 [trailing-spaces] trailing spaces
[warning] 69-69:
69:1 [trailing-spaces] trailing spaces
[warning] 67-67:
67:121 [line-length] line too long (122 > 120 characters)
[warning] 65-65:
65:1 [trailing-spaces] trailing spaces
🪛 LanguageTool
docs/roadmap.md
[inconsistency] ~18-~18: Nesta situação o mês é redundante.
Context: ...DIAS 1-6 CONCLUÍDOS, FINALIZANDO) - ⏳ 3 Dez - 16 Dez: Sprint 2 - Test Coverage 80% + API C...
(MONTH_REDUNDANCY)
[inconsistency] ~67-~67: Nesta situação o mês é redundante.
Context: ...NCLUÍDOS | | Sprint 2 | 2 semanas | 3 Dez - 16 Dez | Test Coverage 80% + API Collections +...
(MONTH_REDUNDANCY)
[inconsistency] ~68-~68: Nesta situação o mês é redundante.
Context: ...lanejado | | Sprint 3 | 2 semanas | 17 Dez - 31 Dez | Blazor Admin Portal (Web) | ⏳ Planeja...
(MONTH_REDUNDANCY)
[typographical] ~760-~760: Símbolo sem par: “]” aparentemente está ausente
Context: ...-coverage` (criada) Documentação: [testing/skipped-tests-analysis.md](./tes...
(UNPAIRED_BRACKETS)
[uncategorized] ~835-~835: Se é uma abreviatura, falta um ponto. Se for uma expressão, coloque entre aspas.
Context: ...35.11% → 80%+ (+200 unit tests) - ⏳ E2E test para provider indexing flow - ⏳ Criar ....
(ABREVIATIONS_PUNCTUATION)
[locale-violation] ~897-~897: “packages” é um estrangeirismo. É preferível dizer “embalagens” ou “pacotes”.
Context: ...l: 35.11% (caiu após migration devido a packages.lock.json + generated code) - Skipped t...
(PT_BARBARISMS_REPLACE_PACKAGES)
docs/sprint-2-tasks.md
[locale-violation] ~11-~11: “packages” é um estrangeirismo. É preferível dizer “embalagens” ou “pacotes”.
Context: ...*: 35.11% (caiu após migration devido a packages.lock.json + generated code) - Tests...
(PT_BARBARISMS_REPLACE_PACKAGES)
[locale-violation] ~533-~533: “Targets” é um estrangeirismo. É preferível dizer “objetivos” ou “alvos”.
Context: ...70%+* (endpoints, middleware) Test Targets: - ✅ Skipped tests: 11 → 0-2 ...
(PT_BARBARISMS_REPLACE_TARGETS)
[grammar] ~540-~540: Possível erro de concordância de número.
Context: ...módulos = 6 módulos completos - ✅ Total endpoints: ~35 endpoints documentados *Tools...
(GENERAL_NUMBER_AGREEMENT_ERRORS)
src/Modules/Documents/API/API.Client/README.md
[typographical] ~28-~28: Símbolo sem par: “[” aparentemente está ausente
Context: ... coleção ### 1. Pré-requisitos - Bruno instalado - ...
(UNPAIRED_BRACKETS)
🪛 markdownlint-cli2 (0.18.1)
src/Modules/Documents/API/API.Client/README.md
78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Security Scan
- GitHub Check: build-and-test
- GitHub Check: code-analysis
🔇 Additional comments (47)
src/Modules/Documents/API/API.Client/README.md (1)
1-109: Excellent documentation supporting API client and test coverage.This README provides clear, comprehensive documentation for the Documents API Client collection. It includes well-organized sections on prerequisites, setup, available endpoints with authorization policies, supported document types, status states, and practical troubleshooting guidance. The Portuguese language aligns with the codebase, and the structure effectively guides developers through using the collection. This documentation supports the PR objective of improving test coverage by exposing and documenting the API client surface.
docs/archive/sprint-1/skipped-tests-resolution-summary.md (3)
1-1: Verify folder structure: document discusses Sprint 2 but is located in sprint-1 archive.The file path is
docs/archive/sprint-1/but the content consistently references "Sprint 2". Either the folder should be renamed tosprint-2/or verify this is intentional (e.g., historical archive of Sprint 2 work planned during Sprint 1).Also applies to: 3-4
1-199: Excellent documentation of test fixes and learnings.The file is well-structured, comprehensive, and provides clear problem-solution-result patterns for each fix. The key learnings section (lines 172–178) distills valuable patterns. The detailed commit breakdown and impact summary make this a solid reference document for test infrastructure improvements.
11-11: Standardize documentation language to English.Line 11 contains Portuguese text in an otherwise English-language document. Ensure consistency with project documentation conventions.
-Durante o Sprint 2, todos os testes skipped identificados em `skipped-tests-tracker.md` foram corrigidos. A investigação também descobriu 18 testes failing adicionais no módulo ServiceCatalogs que foram resolvidos. +During Sprint 2, all skipped tests identified in `skipped-tests-tracker.md` were fixed. The investigation also discovered 18 additional failing tests in the ServiceCatalogs module that were resolved.⛔ Skipped due to learnings
Learnt from: frigini Repo: frigini/MeAjudaAi PR: 29 File: tests/MeAjudaAi.Shared.Tests/Middleware/GeographicRestrictionMiddlewareTests.cs:178-178 Timestamp: 2025-11-25T01:05:52.367Z Learning: In the MeAjudaAi codebase, the standard for comments is Portuguese (due to the Brazilian team), and English is only required for logs. Do not suggest changing Portuguese comments to English.src/Modules/SearchProviders/API/API.Client/README.md (2)
1-49: API Client documentation is clear and well-structured.The README provides good coverage of the collection structure, endpoints, ranking logic, and configuration. The documentation is well-organized and will help users understand the API client collection.
22-28: Documentation describes endpoints that do not exist in the implementation.The README documents 4 endpoints with specific authorization levels, but verification reveals only 1 endpoint is actually implemented:
- Documented: 4 endpoints (2 POST search endpoints, 1 POST indexing endpoint, 1 DELETE endpoint)
- Actual: 1 endpoint (GET
/api/v1/search/providers)The documented endpoints for indexing (
POST /api/v1/search/providers/{id}/indexwith AdminOnly) and deletion (DELETE /api/v1/search/providers/{id}with AdminOnly) do not exist in the codebase. Additionally, the actual endpoint uses GET, not POST as documented. No authorization attributes ([AllowAnonymous],[Authorize], or[AdminOnly]) are applied to the implemented endpoint.Update the README to reflect the single GET endpoint that actually exists, or implement the documented admin endpoints.
Likely an incorrect or invalid review comment.
src/Modules/ServiceCatalogs/Tests/Infrastructure/ServiceCatalogsIntegrationTestBase.cs (2)
68-91: LGTM! Good test isolation improvement.The unique name generation for both categories and services effectively prevents naming conflicts during parallel test execution. This aligns with the PR's objective to improve test coverage and reliability.
100-119: Well-implemented helper method.The
GenerateUniqueNamemethod is correctly implemented with:
- Proper XML documentation
- Sound truncation logic to prevent length violations (reserves 33 chars for separator + GUID)
- Consistent format using the "N" GUID format (32 chars, no hyphens)
Note: The use of
Guid.NewGuid()(UUID v4) here is appropriate since it's for generating unique test names, not entity IDs. Entity IDs are managed by the domain layer'sCreatemethods.src/Modules/ServiceCatalogs/API/API.Client/CatalogAdmin/CreateService.bru (1)
1-55: LGTM! Well-structured API specification.This Bruno API specification file is complete and well-documented. It properly defines the service creation endpoint with:
- Authentication configuration
- Request body structure
- Validation requirements
- Expected response format
Note: C# coding guidelines don't apply to this file as it's an API specification format, not executable C# code.
docs/sprint-2-tasks.md (6)
1-80: ✅ Sprint 2 context and objectives are clearly defined.The planning document appropriately establishes baseline metrics and aligns with the roadmap. Context about coverage decline and skipped tests is accurate.
19-104: ✅ API collections and tools tasks are well-specified with clear deliverables.Reference to Users module as template and concrete version targets for .NET 10 tools are helpful. Seeder requirements (10 categories + 50 services) are appropriately scoped.
107-148:⚠️ Skipped tests count may be incomplete in this section.The document lists 5 PostGIS + 2 Azurite + 2 race condition tests (9 total), but the Sprint 1 conclusion in
docs/roadmap.mdreferences 11 skipped tests. Clarify which tests are the remaining 2 skipped tests or verify the count discrepancy.
150-280: ✅ Unit test specifications follow DDD layering and are appropriately detailed.Coverage targets (90%+ domain, 85%+ application) are well-calibrated for critical layers. Test cases cover both happy paths and business rule violations. Maintaining Locations' existing 52 tests shows good test hygiene awareness.
282-520: ✅ Integration and E2E test specifications are comprehensive with realistic code examples.C# examples follow project conventions (AAA pattern, FluentAssertions, async/await). Geographic restriction E2E test appropriately validates MVP-critical functionality. Multi-module integration testing properly demonstrates cross-boundary concerns.
524-558: ✅ Coverage targets and Definition of Done are well-structured and measurable.Tiered coverage targets (90%+ domain → 70%+ API) are proportional to criticality. Definition of Done includes verification command for coverage validation. Reference to
docs/testing/skipped-tests-analysis.mdis consistent with roadmap documentation structure.docs/roadmap.md (4)
17-37: ✅ Sprint 1 status and Sprint 2 planning are accurately updated.Progression from Sprint 0 → Sprint 1 (DIAS 1-6 CONCLUÍDOS) → Sprint 2 planning is logical. Decision to defer coverage improvement to Sprint 2 for focused code review is sound and well-documented.
756-771: ✅ Sprint 1 achievements and closure are clearly documented.Achievement summary appropriately quantifies improvements (skipped tests: 20→11). Integration events and schema fixes are properly highlighted. Branch reference
feature/module-integrationcorrectly tracks implementation work.
798-880: ✅ Sprint 2 planning is comprehensive and well-integrated with sprint-2-tasks.md.Status progression and rationale for deferring coverage work to Sprint 2 is clear. Cross-reference to the detailed task document helps readers find complete specifications. Target dates (3-16 Dez) align with sprint table and match sprint-2-tasks.md header.
885-956: ✅ Consistent coordination between roadmap.md and sprint-2-tasks.md.Sprint dates (3-16 Dez), branch references (
improve-tests-coverage), and core objectives are synchronized across both documents. The roadmap maintains appropriate abstraction level while sprint-2-tasks.md provides detailed specifications. No contradictions or conflicts detected.tools/MigrationTool/MigrationTool.csproj (2)
23-25: LGTM!The new project references for Documents, SearchProviders, and ServiceCatalogs infrastructure modules are correctly added, enabling the migration tool to discover and apply migrations for all modules.
11-16: Central Package Management is properly configured—no action required.The repository correctly implements Central Package Management through
Directory.Packages.propswithManagePackageVersionsCentrally=true. All unversionedPackageReferenceentries inMigrationTool.csproj(lines 11–16) are defined in the central file with appropriate versions. Build will succeed.tools/MigrationTool/Program.cs (2)
92-141: Improvements address prior review feedback.The reflection-based registration is now robust with proper error handling:
- Explicit method lookup with LINQ filtering
- Clear exception when method is not found
TargetInvocationExceptionhandling with descriptive error messagesUseSnakeCaseNamingConvention()added to match other modules
401-403: LGTM!The fallback connection string credentials are now consistent with the
_connectionStringsdictionary defined at lines 22-29.src/Modules/Locations/API/API.Client/LocationQuery/GetAddressFromCep.bru (1)
1-5: LGTM!The meta configuration follows standard Bruno API client format.
tests/MeAjudaAi.Integration.Tests/Base/ApiTestBase.cs (1)
118-125: The geographic restriction configuration is intentional and correctly designed.The RJ state inconsistency in the test configuration is by design. The
AllowedStateslist contains only MG and ES, whileItaperuna|RJis included inAllowedCities. This tests that the validation system:
- Controls access at the city level (via
AllowedCities), not the state level- Properly validates specific cities regardless of whether their state appears in
AllowedStates- Ensures the fallback validation (simple city/state matching) works correctly without accidentally allowing all cities in a state
The comments confirm this intent: "ensures fallback validation properly blocks non-allowed cities in same state." This validates that adding one RJ city doesn't grant access to all RJ cities. All integration and unit tests throughout the codebase (GeographicRestrictionMiddlewareTests, IbgeUnavailabilityTests, etc.) confirm this behavior works as expected with Itaperuna|RJ allowed while RJ is excluded from the states list.
Directory.Packages.props (1)
183-186: Central Testcontainers.Azurite version is correctly added and alignedAdding
Testcontainers.Azuritehere with version4.7.0is consistent with the existing Testcontainers packages and with the central‑version guideline; this should keep the various test projects in sync.If there were any previous ad‑hoc
Testcontainers.Azuritereferences, please confirm they now rely on this central version (e.g.,rg -n "Testcontainers.Azurite" -Sand ensure no explicit<PackageReference Version="...">remains).src/Modules/Users/Tests/packages.lock.json (1)
1462-1464: Lockfile update for Testcontainers.Azurite matches central versionThe new
Testcontainers.Azuriteentries (project dependency and CentralTransitive section) line up withDirectory.Packages.propsand the existing Testcontainers 4.7.0 stack. This looks like a valid, tool‑generated lock update.To avoid hand‑edited lockfiles, please confirm this was produced by
dotnet restore --use-lock-fileon the same SDK used in CI.Also applies to: 2261-2268
src/Modules/Providers/Tests/packages.lock.json (1)
1462-1464: Consistent Testcontainers.Azurite lockfile entry for Providers testsProviders test lockfile now also includes
Testcontainers.Azuriteat[4.7.0, )plus the corresponding CentralTransitive entry, matching the central package definition and the Users tests.As with the other lockfiles, just ensure this file was regenerated via
dotnet restorerather than manual edits so CI and local environments stay in sync.Also applies to: 2261-2268
infrastructure/compose/standalone/postgres-only.yml (1)
15-25: PostGIS image standardization verified and consistent across all compose environmentsBoth services in
postgres-only.yml(lines 15 and 34–52) are correctly aligned onpostgis/postgis:16-3.4. Verification confirms:
- All compose files in
infrastructure/compose/use postgis/postgis:16-3.4- All GitHub workflows reference postgis/postgis:16-3.4
- No lingering vanilla
postgres:image tags remain in infrastructure or workflows- Migration to PostGIS is complete and intentional across the codebase
The change maintains consistency with the broader repository standardization on PostGIS.
.github/workflows/aspire-ci-cd.yml (1)
29-40: PostGIS image is consistently used across all workflows with spatial tests already in placeThe change to
postgis/postgis:16-3.4is already deployed consistently across all three CI workflows (pr-validation, aspire-ci-cd, and ci-cd). Integration tests explicitly configure PostGIS and NetTopologySuite support via Testcontainers, with dedicated spatial query tests in SearchProviders module verifying geography columns and location-based filtering. No action needed.tests/MeAjudaAi.Integration.Tests/packages.lock.json (1)
2944-2946: ✅ Testcontainers.Azurite dependency correctly added to lockfile.Clean addition with proper dependency chain (Testcontainers.Azurite → Testcontainers 4.7.0). Consistent with other Testcontainers entries in the lock file and aligns with PR objective to integrate Azurite for test infrastructure.
.github/workflows/pr-validation.yml (3)
44-49: ✅ Azurite container correctly integrated into CI workflow.Service definition is properly placed alongside PostgreSQL and provides the standard Azure Storage emulator credentials. Connection strings (lines 173–174) use documented test defaults and are clearly marked.
317-387: ✅ Hangfire compatibility test coverage appropriately scoped.New "CRITICAL - Hangfire Npgsql 10.x Compatibility Tests" job (lines 317–387) is well-structured:
- Clear filtering to run only
Category=HangfireIntegrationtests- Explicit failure scenario with actionable mitigation steps (DOWNGRADE, WAIT, ALTERNATIVE BACKEND)
- Deployment blocker with comprehensive context and documentation reference
This addresses a real risk (Hangfire.PostgreSql 1.20.12 compiled against Npgsql 6.x + new Npgsql 10.x with breaking changes) and provides team guidance on unblocking.
62-80: Based on my verification of the codebase, I can now provide the rewritten review comment:
This review comment is based on an incorrect assumption about the workflow's purpose.
The
pr-validation.ymlworkflow is explicitly designed for pull request validation only (triggers:pull_requestonmaster,develop, andworkflow_dispatch). It is not a gating mechanism for production deployments.The actual CI/CD pipeline in
ci-cd.ymlhandles deployment decisions and runs onpushto branches—a separate workflow with independent triggers. Thepr-validation.ymlandci-cd.ymlworkflows are decoupled;pr-validation.ymldoes not gate or block deployments.The current implementation using lenient fallback defaults (
${{ secrets.POSTGRES_PASSWORD || 'test123' }}) is intentional and appropriate because:
- These are ephemeral test containers, not production infrastructure
- Lenient defaults enable contributor PRs from forks (which lack repository secrets access)
- The validation step logs diagnostics without blocking, allowing rapid iteration
Enforcing strict validation with
exit 1would break all fork contributions and PRs until secrets are manually configured, conflicting with the workflow's stated purpose of enabling pull request validation.No changes are needed.
infrastructure/compose/environments/development.yml (1)
19-19: ✅ PostGIS image correctly updated for geospatial support.Switching to
postgis/postgis:16-3.4adds spatial query capabilities needed for Locations module and geospatial features. Version alignment (16-3.4) is consistent and should be verified across all environments.Verification note: Ensure database initialization scripts in
../../database(line 29) and any migrations properly initialize PostGIS extensions (e.g.,CREATE EXTENSION IF NOT EXISTS postgis;).infrastructure/compose/environments/testing.yml (1)
23-23: ✅ Testing environment aligned with PostGIS image.Image updated consistently with development environment. PostGIS enablement in tests is critical for validating geospatial queries in the Locations module and related features.
tests/MeAjudaAi.E2E.Tests/MeAjudaAi.E2E.Tests.csproj (1)
19-19: ✅ Testcontainers.Azurite dependency correctly added to E2E tests.Package reference is properly positioned and consistent with lockfile updates. xUnit v3 is already in use (line 22), satisfying the stated learning: "Use xUnit v3 for all test projects."
Verification needed: Confirm that SharedTestContainers or a similar base test fixture initializes Azurite container in E2E tests (not visible in provided files but mentioned in summary).
tests/MeAjudaAi.Shared.Tests/packages.lock.json (1)
147-155: Azurite Testcontainers dependency looks consistent with existing Testcontainers stackThe new
Testcontainers.Azuritedirect dependency is version-aligned withTestcontainers/Testcontainers.PostgreSql(4.7.0); the lock entry looks internally consistent with the rest of the graph. Just keep Directory.Packages.props in sync with this version if you bump it later.tests/MeAjudaAi.Shared.Tests/MeAjudaAi.Shared.Tests.csproj (1)
42-56: Adding Testcontainers.Azurite to shared tests is aligned with the new storage test infraIncluding
Testcontainers.Azuritealongside PostgreSql/Respawn in the shared test project matches the new container setup and should enable blob‑related tests cleanly. Nothing to change here; just ensure the centrally managed version stays consistent with other test projects.tests/MeAjudaAi.E2E.Tests/Modules/DocumentsVerificationE2ETests.cs (1)
16-135: Re‑enabling the main document verification E2E flow looks goodUnskipping
RequestDocumentVerification_Should_UpdateStatusis valuable: it now exercises provider creation, document upload,/verify, and/statusin one end‑to‑end path, with flexible assertions on both status code and verification state. The name and structure follow the test guidelines; I don’t see issues here. Based on learnings, this nicely fills the gap aroundPOST /{documentId}/verify.tests/MeAjudaAi.E2E.Tests/CrossModuleCommunicationE2ETests.cs (1)
55-112: Enabling the cross‑module Theory and adding ReportingModule improves coverageRunning
ModuleToModuleCommunication_ShouldWorkForDifferentConsumersfor all four module names (including the newReportingModule) gives good breadth on Users API usage patterns across consumers. The arrange/act/assert flow is clear and the method name matches your test naming convention; I don’t see any behavioral issues here.tests/MeAjudaAi.E2E.Tests/Integration/SearchProvidersEndpointTests.cs (1)
17-42: Re‑enabling search providers E2E tests gives solid coverage of filter combinationsUnskipping this set of facts (coordinates, minRating, serviceIds, subscriptionTiers, pagination, and “all filters”) substantially improves coverage of the search endpoint. Each test:
- Uses clear Arrange/Act/Assert structure,
- Has a name following
Method_Scenario_ExpectedResult,- Correctly treats
OKvsNotFoundas acceptable for data‑dependent cases.No issues from my side here. Based on learnings, this aligns well with your test naming and coverage goals.
Also applies to: 108-136, 155-171, 173-187, 189-214, 233-261
tests/MeAjudaAi.E2E.Tests/Base/TestContainerTestBase.cs (1)
18-21: Azurite Testcontainer wiring and auth cleanup in the E2E test base look correctThe new Azurite container is managed consistently with Postgres/Redis (build,
WithCleanUp(true),StartAsync, andStopAsync), and injecting its connection string viaAzure:Storage:ConnectionStringfits the existing in‑memory configuration model. ClearingConfigurableTestAuthenticationHandlerinDisposeAsyncis also a good safeguard against auth state leakage between tests.Only thing to double‑check is that
Azure:Storage:ConnectionStringis exactly the key your storage clients bind to in the app configuration so the E2E tests actually hit Azurite.Also applies to: 30-33, 60-69, 81-91, 209-225
src/Modules/SearchProviders/Tests/packages.lock.json (1)
1444-1465: SearchProviders tests lockfile Azurite entries are version‑aligned and consistentAdding
Testcontainers.Azuritetomeajudaai.shared.testsand as a CentralTransitive entry (all at 4.7.0 with Testcontainers 4.7.0) matches the rest of your container stack and the other test lockfiles. I don’t see any inconsistency here; just keep this in sync with the central package definition if you upgrade later.Also applies to: 2261-2269
tests/MeAjudaAi.E2E.Tests/packages.lock.json (1)
96-104: Perfect! I've verified all the concerns from the original review comment. Let me provide the final rewritten comment:
E2E and shared test Azurite dependencies are properly declared and consistent.
Verification confirms:
tests/MeAjudaAi.E2E.Tests/MeAjudaAi.E2E.Tests.csproj(line 19): declares<PackageReference Include="Testcontainers.Azurite" />tests/MeAjudaAi.Shared.Tests/MeAjudaAi.Shared.Tests.csproj(line 43): declares<PackageReference Include="Testcontainers.Azurite" />packages.lock.jsonconsistently records version 4.7.0 for both projects (lines 96-104 and ~2017)- Lockfile format is standard NuGet v2 restore output, not manually edited.
src/Modules/Documents/Tests/packages.lock.json (1)
1462-1464: Azurite dependency wiring is correctly configured and lockfile is tool-generated.Verification confirms the setup is sound:
MeAjudaAi.Shared.Tests.csproj(line 43) declaresTestcontainers.Azuriteas a direct dependencyMeAjudaAi.Modules.Documents.Tests.csproj(line 44) depends onMeAjudaAi.Shared.Tests, pulling Azurite transitively- Lock file entries at lines 1462–1464 (transitive) and 2261–2268 (CentralTransitive with resolved metadata) match the tool-generated format from
dotnet restore- Consistent line numbers and structure across all module test lock files confirm automated generation
No manual edits or dependency configuration issues detected.
src/Modules/ServiceCatalogs/Tests/packages.lock.json (1)
1462-1462: Testcontainers.Azurite dependency addition is well-integrated.The new Testcontainers.Azurite (4.7.0) dependency is correctly added to the shared test infrastructure and maintains version consistency with other Testcontainers packages in use. The transitive dependency on Testcontainers 4.7.0 is already present in the lock file, ensuring clean dependency resolution.
Also applies to: 2261-2269
src/Modules/Locations/API/API.Client/LocationQuery/GetAddressFromCep.bru
Outdated
Show resolved
Hide resolved
Workflow fixes: - Remove emoji characters from bash script causing EOF parsing errors - Replace unicode symbols with ASCII text for shell compatibility - Fix: 'unexpected EOF while looking for matching quote' error Code formatting (dotnet format): - Fix whitespace formatting in 15 files (WHITESPACE errors) - Fix file encoding in 5 migration files (CHARSET errors) - Fix imports ordering in 2 files (IMPORTS errors) - Address all 58 formatting violations from CI Files auto-formatted by dotnet format: - E2E tests: TestContainerTestBase, ApiVersioningTests, UsersModuleTests - Providers: ActivateProviderCommandHandler, ProviderService, event handlers - SearchProviders: ProviderActivatedIntegrationEventHandler - Shared tests: ConfigurableTestAuthenticationHandler, SharedTestContainers - ServiceCatalogs: ServiceCatalogsIntegrationTestBase - Migration files: charset fixes for UTF-8 BOM Resolves CI pipeline failures in format verification step
- Fixed broken emoji icons in README sections - Updated project structure to include all 6 modules: * Users (authentication & user management) * Providers (service providers & verification) * Documents (AI document processing) * ServiceCatalogs (services & categories catalog) * SearchProviders (geospatial search with PostGIS) * Locations (IBGE API integration) - Enhanced module descriptions with technologies and features - Configured coverlet for business module coverage focus - Added coverage.runsettings for XPlat Code Coverage - Excluded infrastructure from coverage calculation: * Keycloak, RabbitMQ, ServiceBus, Hangfire * Logging, Monitoring, HealthChecks - Target: 80%+ line coverage in business modules
- docs: fix test count in skipped-tests-resolution-summary (30→31, Theory 5→6) - fix(locations): change GetAddressFromCep auth from bearer to none (AllowAnonymous) - docs(searchproviders): remove non-existent SearchByRadius.bru from README - docs(roadmap): fix documentation link display text - docs(documents): add yaml language identifier to code block - feat(migrations): add NetTopologySuite support for PostGIS modules - fix(tests): pin Azurite container image to 3.33.0 (avoid latest drift) - style(ci): fix YAML linting (trailing spaces, line length <120)
- ActivateProviderCommandHandler: 8 test scenarios (100% coverage) - Happy path with document validations - Provider not found - Missing required documents - Missing verified documents - Pending documents blocking activation - Rejected documents blocking activation - Documents API error handling - Repository exception handling - RejectProviderCommandHandler: 5 test scenarios (100% coverage) - Happy path rejection - Provider not found - Empty reason validation (Theory with 3 cases) - Empty rejectedBy validation (Theory with 3 cases) - Repository exception handling - SuspendProviderCommandHandler: 4 test scenarios (partial coverage) - Provider not found - Empty reason validation (Theory with 3 cases) - Empty suspendedBy validation (Theory with 3 cases) - Repository exception handling - Note: Happy path test skipped (requires investigation) Test Results: 25 passing, 1 skipped Coverage Impact: Commands tested now at 100% (from 0%)
…e to 56.9% Coverage Improvements: - Providers.Application: 46.4% → 56.9% (+10.5 percentage points) - Command Handlers: 3 handlers with 100% coverage (25 tests passing, 1 skipped) - Validators: 6 validators fully tested with 76 test cases New Test Files (6 validators, 76 tests total): 1. CreateProviderRequestValidatorTests.cs (26 tests) - UserId validation (empty, invalid GUID) - Name validation (empty, length constraints) - Provider type validation - BusinessProfile validation (required, description length) - ContactInfo validation (email format) - PrimaryAddress validation (all required fields) 2. RejectProviderCommandValidatorTests.cs (10 tests) - ProviderId validation - RejectedBy validation (empty, length) - Reason validation (empty, min/max length) 3. SuspendProviderCommandValidatorTests.cs (10 tests) - ProviderId validation - SuspendedBy validation (empty, length) - Reason validation (empty, min/max length) 4. AddDocumentRequestValidatorTests.cs (12 tests) - Number validation (empty, length, special characters) - DocumentType validation - Valid number formats testing 5. UpdateProviderProfileRequestValidatorTests.cs (8 tests) - Name validation - BusinessProfile validation - Description validation 6. UpdateVerificationStatusRequestValidatorTests.cs (5 tests) - Status validation - Notes validation (max length) Test Framework: - FluentValidation.TestHelper for declarative validation testing - Theory tests with InlineData for edge cases - Comprehensive null/empty/whitespace coverage Test Results: - Total: 385 tests in Providers module - Passing: 384 tests - Skipped: 1 test (SuspendProvider happy path) - Coverage: Validators show 25% (FluentValidation internal code not executed by TestHelper) Note on Validator Coverage: The 25% validator coverage is expected behavior - FluentValidation.TestHelper validates rules declaratively without executing internal FluentValidation code. All validation logic is thoroughly tested via TestValidateAsync assertions. Refs: #coverage-improvement
Remove duplicate migration files that were causing compilation errors: - Providers: 20251127122139_InitialCreate (duplicate) - Users: 20251126175104_SyncCurrentModel (duplicate Designer.cs) The correct migrations remain in the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pr-validation.yml (1)
18-20: Based on my verification, the original review comment is accurate and the concern is valid.STRICT_COVERAGE must be enabled and coverage thresholds met before merging to main.
The
STRICT_COVERAGE: falsesetting on line 20 is explicitly marked as temporary with a TODO to remove it before merging. I confirmed that:
- STRICT_COVERAGE actively enforces coverage: The workflow uses it at lines 785 and 810 to conditionally fail the pipeline when coverage falls below 70%
- Current setting bypasses protections: With
STRICT_COVERAGE: false, the workflow allows PRs to pass regardless of coverage percentage- Test infrastructure exists: The codebase has comprehensive test projects (Architecture, E2E, Integration, Shared, and module-specific tests), but coverage collection is currently incomplete (only the Users module is included in coverage reporting)
- This blocks merge criteria: Per learnings, minimum 70% test coverage is required; this PR disables that enforcement
Before merging to main, you must:
- Set
STRICT_COVERAGE: trueon line 20- Ensure all new functionality has corresponding tests
- Verify overall coverage meets the 70% minimum threshold (85% recommended per learnings)
🧹 Nitpick comments (14)
README.md (1)
200-210: Add missing punctuation in module descriptions for readability.The Users and ServiceCatalogs sections are missing commas between feature blocks. This impacts readability and consistency with Portuguese grammar conventions.
Apply this diff to improve punctuation:
- **Features**: Registro, autenticação, perfis, RBAC (cliente, prestador, admin) - **Tecnologias**: Keycloak OAuth2/OIDC, PostgreSQL, Event-Driven + **Features**: Registro, autenticação, perfis, RBAC (cliente, prestador, admin), + **Tecnologias**: Keycloak OAuth2/OIDC, PostgreSQL, Event-Driven- **Testes**: 141 testes (100% passing), cobertura 26% Domain, 50% Infrastructure - **Otimização**: Testes paralelos desabilitados para evitar conflitos de chave única + **Testes**: 141 testes (100% passing), cobertura 26% Domain, 50% Infrastructure, + **Otimização**: Testes paralelos desabilitados para evitar conflitos de chave únicasrc/Modules/Providers/Application/Handlers/Commands/ActivateProviderCommandHandler.cs (1)
50-105: Consider extracting repeated document validation pattern.The document validation logic follows the same pattern four times: call API →
Matchto convert result → check failure → log warning → return. This could be consolidated into a helper method to reduce duplication.Example extraction:
private async Task<Result> ValidateDocumentConditionAsync( Func<Guid, CancellationToken, Task<Result<bool>>> apiCall, Guid providerId, bool expectedValue, string failureMessage, CancellationToken cancellationToken) { var result = await apiCall(providerId, cancellationToken); var validation = result.Match( value => value == expectedValue ? Result.Success() : Result.Failure(failureMessage), error => Result.Failure($"Failed to validate documents: {error.Message}")); if (validation.IsFailure) { logger.LogWarning("Provider {ProviderId} cannot be activated: {Error}", providerId, validation.Error); } return validation; }This would allow each validation to be a single line call, improving readability and maintainability.
src/Modules/Providers/Tests/Unit/Application/Handlers/Commands/ActivateProviderCommandHandlerTests.cs (2)
104-168: Consider extracting shared document API mock setup.The tests for different failure scenarios repeat the mock setup pattern. A helper method could reduce duplication:
private void SetupDocumentMocksUpTo(Guid providerId, bool? required = null, bool? verified = null, bool? pending = null, bool? rejected = null) { if (required.HasValue) _documentsModuleApiMock .Setup(x => x.HasRequiredDocumentsAsync(providerId, It.IsAny<CancellationToken>())) .ReturnsAsync(Result<bool>.Success(required.Value)); // ... similar for other methods }This would make each test more concise and the intent clearer.
283-299: Consider adding test for UpdateAsync failure.The current test covers
GetByIdAsyncthrowing an exception. A complementary test forUpdateAsyncthrowing would verify the exception handler covers the persistence step as well.[Fact] public async Task HandleAsync_WhenUpdateAsyncThrowsException_ShouldReturnFailure() { // Arrange var providerId = Guid.NewGuid(); var provider = new ProviderBuilder().WithId(providerId).Build(); provider.CompleteBasicInfo("[email protected]"); var command = new ActivateProviderCommand(providerId, "[email protected]"); _providerRepositoryMock .Setup(r => r.GetByIdAsync(It.IsAny<ProviderId>(), It.IsAny<CancellationToken>())) .ReturnsAsync(provider); // Setup all document validations to pass _documentsModuleApiMock.Setup(x => x.HasRequiredDocumentsAsync(providerId, It.IsAny<CancellationToken>())) .ReturnsAsync(Result<bool>.Success(true)); _documentsModuleApiMock.Setup(x => x.HasVerifiedDocumentsAsync(providerId, It.IsAny<CancellationToken>())) .ReturnsAsync(Result<bool>.Success(true)); _documentsModuleApiMock.Setup(x => x.HasPendingDocumentsAsync(providerId, It.IsAny<CancellationToken>())) .ReturnsAsync(Result<bool>.Success(false)); _documentsModuleApiMock.Setup(x => x.HasRejectedDocumentsAsync(providerId, It.IsAny<CancellationToken>())) .ReturnsAsync(Result<bool>.Success(false)); _providerRepositoryMock .Setup(r => r.UpdateAsync(It.IsAny<Provider>(), It.IsAny<CancellationToken>())) .ThrowsAsync(new Exception("Database error")); // Act var result = await _handler.HandleAsync(command, CancellationToken.None); // Assert result.IsFailure.Should().BeTrue(); result.Error.Message.Should().Be("Failed to activate provider"); }Based on learnings, add tests for all new functionality.
src/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.cs (1)
134-139: New unique index duplicates the primary key index on (provider_id, service_id)
provider_servicesalready has a composite primary key on(provider_id, service_id), which enforces uniqueness and creates an index on those columns. Addingix_provider_services_provider_serviceas another unique index on the exact same column set is functionally redundant and will add extra write/storage overhead unless you specifically need this second index by name.If there is no external dependency on this index name, consider dropping it and relying on the PK index instead:
- migrationBuilder.CreateIndex( - name: "ix_provider_services_provider_service", - schema: "meajudaai_providers", - table: "provider_services", - columns: new[] { "provider_id", "service_id" }, - unique: true); -src/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.Designer.cs (1)
132-134: ProviderService composite index here also duplicates the primary keyThis
HasIndex("ProviderId", "ServiceId").IsUnique()generates the same unique index as the composite PK on(ProviderId, ServiceId), so in PostgreSQL you’ll end up with two indexes over the same columns (PK + this unique index). That’s typically unnecessary overhead unless you explicitly need this separately named index.If you decide to remove the redundant index from the migration, remember to also drop this mapping from the model snapshot and the corresponding
HasIndexconfiguration in the DbContext before re‑scaffolding:- b.HasIndex("ProviderId", "ServiceId") - .IsUnique() - .HasDatabaseName("ix_provider_services_provider_service"); -tests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cs (2)
101-113: Consider adding Azurite health validation for consistency.The
ValidateContainerHealthAsyncmethod only validates PostgreSQL readiness. While Testcontainers typically handles wait strategies internally, consider adding similar validation for Azurite to ensure consistency, especially if blob storage tests are sensitive to startup timing.// Verifica se o container PostgreSQL está realmente pronto await ValidateContainerHealthAsync(); + + // Verifica se o container Azurite está pronto + await ValidateAzuriteHealthAsync();
168-184: Consider adding Azurite blob cleanup for test isolation.The cleanup methods only handle PostgreSQL schemas. If integration tests upload blobs to Azurite, residual data could cause test interference. Consider adding an Azurite cleanup method:
+ /// <summary> + /// Limpa containers de blob storage no Azurite + /// </summary> + public static async Task CleanupAzuriteAsync() + { + if (!_isInitialized || _azuriteContainer == null) return; + + // Azurite pode ser resetado via API ou recriando containers + // Alternativa: usar BlobServiceClient para deletar containers + await _azuriteContainer.StopAsync(); + await _azuriteContainer.StartAsync(); + }Alternatively, use the Azure.Storage.Blobs SDK to enumerate and delete blob containers between tests for finer control.
src/Modules/Providers/Tests/Unit/Application/Validators/UpdateVerificationStatusRequestValidatorTests.cs (1)
34-49: Consider adding empty string test for Notes.The test covers
nullnotes, but you may want to add a test for empty string ("") to confirm whether the validator treats it as valid (likenull) or invalid.[Fact] public async Task Validate_WithEmptyNotes_ShouldNotHaveValidationErrors() { // Arrange var request = new UpdateVerificationStatusRequest { Status = EVerificationStatus.Verified, Notes = "" }; // Act var result = await _validator.TestValidateAsync(request); // Assert result.ShouldNotHaveValidationErrorFor(x => x.Notes); }src/Modules/Providers/Tests/Unit/Application/Validators/UpdateProviderProfileRequestValidatorTests.cs (1)
89-124: Consider extending nested validation coverage.The tests cover
NameandBusinessProfile.Description, butCreateProviderRequestValidatorTestsalso validates nested fields likeContactInfo.EmailandPrimaryAddress. IfUpdateProviderProfileRequestValidatorenforces the same nested validations, consider adding corresponding tests for parity.src/Modules/Providers/Tests/Unit/Application/Validators/AddDocumentRequestValidatorTests.cs (1)
114-141: Consider using parameterized tests for valid number formats.The
foreachloop works but makes it harder to identify which specific format failed if the test breaks. Using[Theory]with[InlineData]provides clearer test output.- [Fact] - public async Task Validate_WithValidNumberFormats_ShouldNotHaveValidationErrors() + [Theory] + [InlineData("ABC123")] + [InlineData("123-456")] + [InlineData("AB.CD.123")] + [InlineData("A1B2C3")] + [InlineData("123.456.789-00")] + public async Task Validate_WithValidNumberFormats_ShouldNotHaveValidationErrors(string number) { - // Arrange - var validNumbers = new[] - { - "ABC123", - "123-456", - "AB.CD.123", - "A1B2C3", - "123.456.789-00" - }; - - foreach (var number in validNumbers) + // Arrange + var request = new AddDocumentRequest { - var request = new AddDocumentRequest - { - Number = number, - DocumentType = EDocumentType.CPF - }; - - // Act - var result = await _validator.TestValidateAsync(request); + Number = number, + DocumentType = EDocumentType.CPF + }; - // Assert - result.ShouldNotHaveValidationErrorFor(x => x.Number); - } + // Act + var result = await _validator.TestValidateAsync(request); + + // Assert + result.ShouldNotHaveValidationErrorFor(x => x.Number); }tools/MigrationTool/Program.cs (1)
407-409: Verify fallback DB naming pattern and alignment with app/CI configurationThe fallback connection string now uses a per-module database name (
meajudaai_{module.ToLowerInvariant()}) with test credentials, while the predefined entries above still point to a sharedDatabase=meajudaai. This asymmetry is probably fine if newer modules (e.g., ones not in_connectionStrings) are intentionally given their own databases, but it’s easy for this to drift from what appsettings/CI containers use.Please double-check that:
- For modules without an explicit entry, the application and test infrastructure also target
meajudaai_{module}; or- If the intent is a single shared DB with per-module schemas, the fallback should match that pattern instead.
If you want extra safety, you could also log when the fallback is used so mismatches are obvious during local runs.
src/Modules/Providers/Tests/Unit/Application/Handlers/Commands/SuspendProviderCommandHandlerTests.cs (2)
73-96: Not-found scenario test is solid; optionally verify repository fetch countThe
HandleAsync_WhenProviderNotFound_ShouldReturnFailuretest correctly asserts failure, checks the error message, and ensuresUpdateAsyncis never called. That fully covers the not-found branch of the handler.If you want to lock in the interaction contract a bit more, you could also verify that
GetByIdAsyncis called exactly once:// Assert result.IsFailure.Should().BeTrue(); result.Error.Message.Should().Be("Provider not found"); _providerRepositoryMock.Verify( r => r.UpdateAsync(It.IsAny<Provider>(), It.IsAny<CancellationToken>()), Times.Never); + + _providerRepositoryMock.Verify( + r => r.GetByIdAsync(It.IsAny<ProviderId>(), It.IsAny<CancellationToken>()), + Times.Once);Not mandatory, but it tightens the test around how the handler interacts with the repository.
146-165: Exception path test is correct; minor option to assert no update after failure
HandleAsync_WhenRepositoryThrowsException_ShouldReturnFailureaccurately simulates a repository failure viaThrowsAsyncand verifies that the handler responds with a generic"Failed to suspend provider"error, matching the implementation.As a small enhancement, you might also assert that
UpdateAsyncis never invoked whenGetByIdAsyncthrows, to guard against future refactors that might try to continue after certain exceptions:// Assert result.IsFailure.Should().BeTrue(); result.Error.Message.Should().Be("Failed to suspend provider"); + + _providerRepositoryMock.Verify( + r => r.UpdateAsync(It.IsAny<Provider>(), It.IsAny<CancellationToken>()), + Times.Never);Optional, but it makes the failure behavior even more explicit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
.github/workflows/pr-validation.yml(6 hunks)README.md(4 hunks)config/coverage.runsettings(1 hunks)config/coverlet.json(1 hunks)docs/archive/sprint-1/skipped-tests-resolution-summary.md(1 hunks)docs/roadmap.md(6 hunks)src/Modules/Documents/API/API.Client/README.md(1 hunks)src/Modules/Documents/Infrastructure/Migrations/20251126174809_InitialCreate.cs(1 hunks)src/Modules/Locations/API/API.Client/LocationQuery/GetAddressFromCep.bru(1 hunks)src/Modules/Locations/Infrastructure/Services/IbgeService.cs(1 hunks)src/Modules/Locations/Tests/Unit/Infrastructure/Services/IbgeServiceTests.cs(1 hunks)src/Modules/Providers/Application/Handlers/Commands/ActivateProviderCommandHandler.cs(4 hunks)src/Modules/Providers/Domain/Entities/ProviderService.cs(1 hunks)src/Modules/Providers/Infrastructure/Events/Handlers/ProviderVerificationStatusUpdatedDomainEventHandler.cs(1 hunks)src/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.Designer.cs(2 hunks)src/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.cs(1 hunks)src/Modules/Providers/Tests/Integration/ProvidersIntegrationTestBase.cs(3 hunks)src/Modules/Providers/Tests/Unit/Application/Handlers/Commands/ActivateProviderCommandHandlerTests.cs(1 hunks)src/Modules/Providers/Tests/Unit/Application/Handlers/Commands/RejectProviderCommandHandlerTests.cs(1 hunks)src/Modules/Providers/Tests/Unit/Application/Handlers/Commands/SuspendProviderCommandHandlerTests.cs(1 hunks)src/Modules/Providers/Tests/Unit/Application/Validators/AddDocumentRequestValidatorTests.cs(1 hunks)src/Modules/Providers/Tests/Unit/Application/Validators/CreateProviderRequestValidatorTests.cs(1 hunks)src/Modules/Providers/Tests/Unit/Application/Validators/RejectProviderCommandValidatorTests.cs(1 hunks)src/Modules/Providers/Tests/Unit/Application/Validators/SuspendProviderCommandValidatorTests.cs(1 hunks)src/Modules/Providers/Tests/Unit/Application/Validators/UpdateProviderProfileRequestValidatorTests.cs(1 hunks)src/Modules/Providers/Tests/Unit/Application/Validators/UpdateVerificationStatusRequestValidatorTests.cs(1 hunks)src/Modules/SearchProviders/API/API.Client/README.md(1 hunks)src/Modules/SearchProviders/Infrastructure/Events/Handlers/ProviderActivatedIntegrationEventHandler.cs(2 hunks)src/Modules/SearchProviders/Infrastructure/Persistence/Migrations/20251126181150_InitialCreate.cs(1 hunks)src/Modules/ServiceCatalogs/Infrastructure/Migrations/20251127142645_InitialCreate.cs(1 hunks)src/Modules/ServiceCatalogs/Tests/Infrastructure/ServiceCatalogsIntegrationTestBase.cs(3 hunks)src/Modules/Users/Infrastructure/Migrations/20251127125806_InitialCreate.cs(1 hunks)tests/MeAjudaAi.E2E.Tests/Base/TestContainerTestBase.cs(7 hunks)tests/MeAjudaAi.E2E.Tests/Integration/ApiVersioningTests.cs(1 hunks)tests/MeAjudaAi.E2E.Tests/Integration/UsersModuleTests.cs(3 hunks)tests/MeAjudaAi.Shared.Tests/Auth/ConfigurableTestAuthenticationHandler.cs(5 hunks)tests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cs(6 hunks)tools/MigrationTool/Program.cs(2 hunks)
✅ Files skipped from review due to trivial changes (9)
- src/Modules/SearchProviders/Infrastructure/Persistence/Migrations/20251126181150_InitialCreate.cs
- src/Modules/Locations/Infrastructure/Services/IbgeService.cs
- config/coverage.runsettings
- src/Modules/Locations/Tests/Unit/Infrastructure/Services/IbgeServiceTests.cs
- src/Modules/Users/Infrastructure/Migrations/20251127125806_InitialCreate.cs
- src/Modules/Documents/Infrastructure/Migrations/20251126174809_InitialCreate.cs
- src/Modules/SearchProviders/Infrastructure/Events/Handlers/ProviderActivatedIntegrationEventHandler.cs
- tests/MeAjudaAi.E2E.Tests/Integration/UsersModuleTests.cs
- src/Modules/Providers/Infrastructure/Events/Handlers/ProviderVerificationStatusUpdatedDomainEventHandler.cs
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Modules/ServiceCatalogs/Tests/Infrastructure/ServiceCatalogsIntegrationTestBase.cs
- tests/MeAjudaAi.E2E.Tests/Base/TestContainerTestBase.cs
- src/Modules/Locations/API/API.Client/LocationQuery/GetAddressFromCep.bru
- src/Modules/SearchProviders/API/API.Client/README.md
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{cs,csx}
📄 CodeRabbit inference engine (AGENT.md)
Use UUID v7 (time-ordered) instead of UUID v4 for entity IDs in .NET 9 projects
Files:
src/Modules/Providers/Tests/Unit/Application/Validators/UpdateProviderProfileRequestValidatorTests.cssrc/Modules/Providers/Domain/Entities/ProviderService.cssrc/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.cssrc/Modules/Providers/Tests/Unit/Application/Handlers/Commands/SuspendProviderCommandHandlerTests.cssrc/Modules/ServiceCatalogs/Infrastructure/Migrations/20251127142645_InitialCreate.cssrc/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.Designer.cssrc/Modules/Providers/Tests/Unit/Application/Validators/CreateProviderRequestValidatorTests.cstests/MeAjudaAi.E2E.Tests/Integration/ApiVersioningTests.cstools/MigrationTool/Program.cssrc/Modules/Providers/Tests/Unit/Application/Handlers/Commands/ActivateProviderCommandHandlerTests.cssrc/Modules/Providers/Tests/Unit/Application/Handlers/Commands/RejectProviderCommandHandlerTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/AddDocumentRequestValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/SuspendProviderCommandValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/RejectProviderCommandValidatorTests.cssrc/Modules/Providers/Tests/Integration/ProvidersIntegrationTestBase.cssrc/Modules/Providers/Application/Handlers/Commands/ActivateProviderCommandHandler.cstests/MeAjudaAi.Shared.Tests/Auth/ConfigurableTestAuthenticationHandler.cssrc/Modules/Providers/Tests/Unit/Application/Validators/UpdateVerificationStatusRequestValidatorTests.cstests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cs
**/Application/**/*.cs
📄 CodeRabbit inference engine (AGENT.md)
**/Application/**/*.cs: Use Result pattern for operations that can fail instead of throwing exceptions
Use MediatR for CQRS command and query handling in the Application layer
Files:
src/Modules/Providers/Tests/Unit/Application/Validators/UpdateProviderProfileRequestValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Handlers/Commands/SuspendProviderCommandHandlerTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/CreateProviderRequestValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Handlers/Commands/ActivateProviderCommandHandlerTests.cssrc/Modules/Providers/Tests/Unit/Application/Handlers/Commands/RejectProviderCommandHandlerTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/AddDocumentRequestValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/SuspendProviderCommandValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/RejectProviderCommandValidatorTests.cssrc/Modules/Providers/Application/Handlers/Commands/ActivateProviderCommandHandler.cssrc/Modules/Providers/Tests/Unit/Application/Validators/UpdateVerificationStatusRequestValidatorTests.cs
**/*.cs
📄 CodeRabbit inference engine (AGENT.md)
**/*.cs: All public APIs should have XML documentation comments
Code style and formatting must adhere to .editorconfig rules
Files:
src/Modules/Providers/Tests/Unit/Application/Validators/UpdateProviderProfileRequestValidatorTests.cssrc/Modules/Providers/Domain/Entities/ProviderService.cssrc/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.cssrc/Modules/Providers/Tests/Unit/Application/Handlers/Commands/SuspendProviderCommandHandlerTests.cssrc/Modules/ServiceCatalogs/Infrastructure/Migrations/20251127142645_InitialCreate.cssrc/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.Designer.cssrc/Modules/Providers/Tests/Unit/Application/Validators/CreateProviderRequestValidatorTests.cstests/MeAjudaAi.E2E.Tests/Integration/ApiVersioningTests.cstools/MigrationTool/Program.cssrc/Modules/Providers/Tests/Unit/Application/Handlers/Commands/ActivateProviderCommandHandlerTests.cssrc/Modules/Providers/Tests/Unit/Application/Handlers/Commands/RejectProviderCommandHandlerTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/AddDocumentRequestValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/SuspendProviderCommandValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/RejectProviderCommandValidatorTests.cssrc/Modules/Providers/Tests/Integration/ProvidersIntegrationTestBase.cssrc/Modules/Providers/Application/Handlers/Commands/ActivateProviderCommandHandler.cstests/MeAjudaAi.Shared.Tests/Auth/ConfigurableTestAuthenticationHandler.cssrc/Modules/Providers/Tests/Unit/Application/Validators/UpdateVerificationStatusRequestValidatorTests.cstests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cs
**/Domain/Entities/**/*.cs
📄 CodeRabbit inference engine (AGENT.md)
Publish domain events in aggregate roots when state changes occur, handle them via INotificationHandler
Files:
src/Modules/Providers/Domain/Entities/ProviderService.cs
**/*.Tests/**/*.cs
📄 CodeRabbit inference engine (AGENT.md)
**/*.Tests/**/*.cs: Use xUnit v3 for all test projects
Structure unit tests using Arrange-Act-Assert pattern: Arrange (test data setup), Act (execute operation), Assert (verify outcome)
Test method naming follows pattern: MethodName_Scenario_ExpectedResult
Files:
tests/MeAjudaAi.E2E.Tests/Integration/ApiVersioningTests.cstests/MeAjudaAi.Shared.Tests/Auth/ConfigurableTestAuthenticationHandler.cstests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cs
**/Application/**/*Handler.cs
📄 CodeRabbit inference engine (AGENT.md)
Name command/query handlers following the pattern [Command/Query]Handler (e.g., RegisterUserCommandHandler)
Files:
src/Modules/Providers/Application/Handlers/Commands/ActivateProviderCommandHandler.cs
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Add or update tests for all new functionality before submitting a PR
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Application/**/*Validator.cs : Use FluentValidation for input validation of Commands and Queries
Applied to files:
src/Modules/Providers/Tests/Unit/Application/Validators/UpdateProviderProfileRequestValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/CreateProviderRequestValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/AddDocumentRequestValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/SuspendProviderCommandValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/RejectProviderCommandValidatorTests.cssrc/Modules/Providers/Tests/Unit/Application/Validators/UpdateVerificationStatusRequestValidatorTests.cs
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Infrastructure/Persistence/**DbContext.cs : Database per module using schema isolation: Users module uses meajudaai_users schema, Providers uses meajudaai_providers schema
Applied to files:
src/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.cssrc/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.Designer.cstools/MigrationTool/Program.cssrc/Modules/Providers/Tests/Integration/ProvidersIntegrationTestBase.cstests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.csREADME.md
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Infrastructure/Persistence/**DbContext.cs : Each module must have a dedicated PostgreSQL schema named meajudaai_{module_name} in lowercase
Applied to files:
src/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.cssrc/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.Designer.cstools/MigrationTool/Program.csREADME.md
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Infrastructure/Persistence/**/*.cs : Use Entity Framework Core with dedicated DbContext per module, apply schema isolation via HasDefaultSchema()
Applied to files:
src/Modules/ServiceCatalogs/Infrastructure/Migrations/20251127142645_InitialCreate.cssrc/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.Designer.cstools/MigrationTool/Program.cssrc/Modules/Providers/Tests/Integration/ProvidersIntegrationTestBase.cs
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Infrastructure/Persistence/**DbContext.cs : Apply all EF Core configurations from assembly using modelBuilder.ApplyConfigurationsFromAssembly() in DbContext.OnModelCreating()
Applied to files:
src/Modules/ServiceCatalogs/Infrastructure/Migrations/20251127142645_InitialCreate.cstools/MigrationTool/Program.cs
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/API/**/*.cs : Use Minimal APIs for HTTP endpoint definitions in the API layer
Applied to files:
tests/MeAjudaAi.E2E.Tests/Integration/ApiVersioningTests.cs
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to {Shared/Contracts/Modules/**/*.cs,**/Application/Services/**ModuleApi.cs} : Module APIs must be defined as interfaces in Shared/Contracts/Modules/{ModuleName}/ with [ModuleApi] attribute on implementation
Applied to files:
tests/MeAjudaAi.E2E.Tests/Integration/ApiVersioningTests.csREADME.md
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Applies to **/Application/**/*Handler.cs : Name command/query handlers following the pattern [Command/Query]Handler (e.g., RegisterUserCommandHandler)
Applied to files:
src/Modules/Providers/Tests/Unit/Application/Handlers/Commands/RejectProviderCommandHandlerTests.cs
📚 Learning: 2025-11-25T16:00:52.763Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.763Z
Learning: Modular Monolith: Each module in src/Modules/ should have Application, Domain, Infrastructure, API, and Tests folders
Applied to files:
README.md
🧬 Code graph analysis (10)
src/Modules/Providers/Tests/Unit/Application/Validators/UpdateProviderProfileRequestValidatorTests.cs (2)
src/Modules/Providers/Application/Mappers/ProviderMapper.cs (1)
ContactInfoDto(61-68)src/Modules/Users/Domain/ValueObjects/Email.cs (1)
src/Modules/Providers/Tests/Unit/Application/Handlers/Commands/SuspendProviderCommandHandlerTests.cs (1)
src/Modules/Providers/Application/Handlers/Commands/SuspendProviderCommandHandler.cs (1)
SuspendProviderCommandHandler(19-69)
src/Modules/Providers/Tests/Unit/Application/Validators/CreateProviderRequestValidatorTests.cs (5)
src/Shared/Constants/ApiEndpoints.cs (1)
Providers(35-51)src/Modules/Providers/Tests/Unit/Application/Validators/UpdateProviderProfileRequestValidatorTests.cs (4)
Fact(17-28)Theory(30-44)Theory(89-106)BusinessProfileDto(135-157)src/Modules/Providers/Application/Mappers/ProviderMapper.cs (1)
ContactInfoDto(61-68)src/Modules/Users/Domain/ValueObjects/Email.cs (1)
src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/GeographicRestrictionMiddleware.cs (1)
City(101-141)
src/Modules/Providers/Tests/Unit/Application/Handlers/Commands/ActivateProviderCommandHandlerTests.cs (2)
src/Modules/Providers/Application/Handlers/Commands/ActivateProviderCommandHandler.cs (1)
ActivateProviderCommandHandler(23-122)src/Modules/Providers/Domain/Entities/Provider.cs (1)
CompleteBasicInfo(437-443)
src/Modules/Providers/Tests/Unit/Application/Handlers/Commands/RejectProviderCommandHandlerTests.cs (1)
src/Modules/Providers/Application/Handlers/Commands/RejectProviderCommandHandler.cs (1)
RejectProviderCommandHandler(19-69)
src/Modules/Providers/Tests/Unit/Application/Validators/SuspendProviderCommandValidatorTests.cs (2)
src/Modules/Providers/Tests/Unit/Application/Validators/CreateProviderRequestValidatorTests.cs (18)
Fact(18-29)Fact(47-59)Fact(77-89)Fact(91-104)Fact(106-117)Fact(119-131)Fact(133-149)Fact(151-166)Fact(190-208)Fact(210-225)Theory(31-45)Theory(61-75)Theory(168-188)Theory(227-247)Theory(249-269)Theory(271-291)Theory(293-313)Theory(315-335)src/Modules/Providers/Tests/Unit/Application/Validators/RejectProviderCommandValidatorTests.cs (3)
Fact(16-31)Theory(51-70)Theory(91-110)
src/Modules/Providers/Tests/Unit/Application/Validators/RejectProviderCommandValidatorTests.cs (1)
src/Modules/Providers/Tests/Unit/Application/Validators/SuspendProviderCommandValidatorTests.cs (3)
Fact(16-31)Theory(51-70)Theory(91-110)
src/Modules/Providers/Application/Handlers/Commands/ActivateProviderCommandHandler.cs (1)
src/Shared/Functional/Result.cs (11)
Match(61-70)Result(5-34)Result(20-20)Result(21-21)Result(23-23)Result(24-24)Result(25-25)Result(36-87)Result(49-49)Result(50-50)Result(51-51)
tests/MeAjudaAi.Shared.Tests/Auth/ConfigurableTestAuthenticationHandler.cs (4)
tests/MeAjudaAi.Shared.Tests/Auth/TestAuthenticationHandlers.cs (3)
GetTestUserId(18-18)GetTestUserName(19-19)GetTestUserEmail(20-20)tests/MeAjudaAi.Shared.Tests/Auth/AspireTestAuthenticationHandler.cs (3)
GetTestUserId(33-33)GetTestUserName(34-34)GetTestUserEmail(35-35)src/Modules/Users/Domain/ValueObjects/Email.cs (1)
src/Shared/Constants/ValidationMessages.cs (1)
Length(38-47)
src/Modules/Providers/Tests/Unit/Application/Validators/UpdateVerificationStatusRequestValidatorTests.cs (2)
src/Modules/Providers/Tests/Unit/Application/Validators/AddDocumentRequestValidatorTests.cs (5)
Fact(17-32)Fact(55-71)Fact(73-90)Fact(114-141)Fact(143-158)src/Modules/Providers/Tests/Unit/Application/Validators/RejectProviderCommandValidatorTests.cs (1)
Fact(16-31)
🪛 LanguageTool
docs/roadmap.md
[inconsistency] ~18-~18: Nesta situação o mês é redundante.
Context: ...DIAS 1-6 CONCLUÍDOS, FINALIZANDO) - ⏳ 3 Dez - 16 Dez: Sprint 2 - Test Coverage 80% + API C...
(MONTH_REDUNDANCY)
[inconsistency] ~67-~67: Nesta situação o mês é redundante.
Context: ...NCLUÍDOS | | Sprint 2 | 2 semanas | 3 Dez - 16 Dez | Test Coverage 80% + API Collections +...
(MONTH_REDUNDANCY)
[inconsistency] ~68-~68: Nesta situação o mês é redundante.
Context: ...lanejado | | Sprint 3 | 2 semanas | 17 Dez - 31 Dez | Blazor Admin Portal (Web) | ⏳ Planeja...
(MONTH_REDUNDANCY)
[typographical] ~760-~760: Símbolo sem par: “]” aparentemente está ausente
Context: ...-coverage` (criada) Documentação: [docs/testing/skipped-tests-analysis.md](...
(UNPAIRED_BRACKETS)
[uncategorized] ~835-~835: Se é uma abreviatura, falta um ponto. Se for uma expressão, coloque entre aspas.
Context: ...35.11% → 80%+ (+200 unit tests) - ⏳ E2E test para provider indexing flow - ⏳ Criar ....
(ABREVIATIONS_PUNCTUATION)
[locale-violation] ~897-~897: “packages” é um estrangeirismo. É preferível dizer “embalagens” ou “pacotes”.
Context: ...l: 35.11% (caiu após migration devido a packages.lock.json + generated code) - Skipped t...
(PT_BARBARISMS_REPLACE_PACKAGES)
README.md
[uncategorized] ~203-~203: Encontrada possível ausência de vírgula.
Context: ...e, prestador, admin) - Tecnologias: Keycloak OAuth2/OIDC, PostgreSQL, Event-Driven -...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~221-~221: Encontrada possível ausência de vírgula.
Context: ... Testes: 141 testes (100% passing), cobertura 26% Domain, 50% Infrastructure - **Otim...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[locale-violation] ~228-~228: “Performance” é um estrangeirismo. É preferível dizer “desempenho”, “atuação”, “apresentação”, “espetáculo” ou “interpretação”.
Context: ...tgreSQL 16 com extensão PostGIS 3.4 - Performance: Índices GiST para consultas geoespac...
(PT_BARBARISMS_REPLACE_PERFORMANCE)
src/Modules/Documents/API/API.Client/README.md
[typographical] ~28-~28: Símbolo sem par: “[” aparentemente está ausente
Context: ... coleção ### 1. Pré-requisitos - Bruno instalado - ...
(UNPAIRED_BRACKETS)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Security Scan
- GitHub Check: build-and-test
- GitHub Check: code-analysis
🔇 Additional comments (37)
config/coverlet.json (2)
13-14: Clarify coverage strategy for infrastructure and security-critical code.This configuration excludes significant infrastructure components (authentication, messaging, health checks, monitoring) from coverage metrics. While the "business-focus" tag clarifies intent, excluding authentication-related classes (
-*Keycloak*,-*KeycloakPermissionResolver*) and message handling infrastructure raises concerns about hidden gaps in security and reliability code.Please clarify:
- Is there a separate coverage strategy/configuration for infrastructure components, or are they intentionally not tracked?
- Are security-critical components like authentication and authorization covered by other metrics or testing strategies?
- How does this exclusion align with the PR objective of "improving test coverage"—are you adding new tests for infrastructure, or just excluding it from metrics?
This helps ensure the team doesn't inadvertently reduce visibility into critical infrastructure quality.
Also applies to: 18-35
50-51: Verify threshold values are appropriate for the "business-focus" scope.The thresholds (80% line, 70% branch, 75% method) are reasonable baselines, but verify they're attainable and appropriate for only the business logic subset of code. If these thresholds were set based on historical coverage of the full codebase, they may need adjustment for the narrower scope.
src/Modules/ServiceCatalogs/Infrastructure/Migrations/20251127142645_InitialCreate.cs (1)
1-1: Encoding cleanup improves code hygiene.The removal of the invisible character is a good housekeeping change. The migration file follows EF Core conventions and properly implements schema isolation.
src/Modules/Documents/API/API.Client/README.md (1)
1-109: Comprehensive API documentation with clear structure and guidelines.The README effectively documents the Documents API Client collection with clear sections for setup, prerequisites, endpoints, authorization policies, and troubleshooting. The structure, variable definitions, and error handling guidance are well-organized and should help developers use these endpoints correctly.
README.md (3)
224-234: Verify test coverage metrics and consider terminology adjustment.Line 221 specifies 141 tests with 26% Domain and 50% Infrastructure coverage for ServiceCatalogs. Line 228 uses "Performance" which, in Portuguese documentation, is typically expressed as "Desempenho" for consistency with project conventions.
Consider:
- Confirming that the 141 test count and coverage percentages reflect the current state of the test suite.
- Replacing "Performance" with "Desempenho" to maintain Portuguese terminology consistency throughout the README.
176-189: Module structure reorganization effectively reflects modular monolith architecture.The reorganized module hierarchy clearly shows the Clean Architecture + DDD structure for each module (API, Application, Domain, Infrastructure, Tests folders). This aligns well with the project's modular monolith approach and the learning context about dedicated PostgreSQL schemas per module.
31-66: Documentation updates enhance project navigation and setup guidance.The restructured project overview, improved section headers, and clearer guidance for quick-start scenarios (with options for Aspire vs. Docker Compose) make the README more accessible to new contributors. The additions of concrete port information and troubleshooting sections are valuable.
tests/MeAjudaAi.Shared.Tests/Auth/ConfigurableTestAuthenticationHandler.cs (1)
29-29: LGTM - Formatting improvements only.The changes are purely stylistic (whitespace adjustments and multi-line formatting for expression-bodied members and method parameters). No behavioral modifications. The code correctly uses
Guid.CreateVersion7()(line 147) per coding guidelines, and all public APIs have XML documentation.Also applies to: 43-43, 93-103, 165-169, 259-261
docs/archive/sprint-1/skipped-tests-resolution-summary.md (1)
1-22: Numeric consistency corrected; comprehensive documentation.The previous review flagged a mismatch between the header "Total Tests Fixed: 30" and the table total of 31. This has been correctly fixed to "31 (26 from skipped/failing + 5 Theory cases)" on line 5, which now aligns with the status table totaling 31 tests (line 22: 18+6+1+5+1=31).
The document is well-structured with clear sections for each of the 5 fixes, detailed technical breakdowns (root cause → solution → results), and honest accounting of remaining intentional skips. The 6 commits summary and key learnings sections add value for future reference.
docs/roadmap.md (2)
760-760: Verify markdown link display text matches resolved link target.Line 760 shows:
**Documentação**: [docs/testing/skipped-tests-analysis.md](./testing/skipped-tests-analysis.md)The previous review flagged a path mismatch where display text didn't match the URL target. The current state shows the display text now correctly matches the URL target (both reference
docs/testing/skipped-tests-analysis.md/./testing/skipped-tests-analysis.md), so this issue appears resolved ✓. The URL correctly points to the comprehensive skipped-tests-analysis.md in the testing subdirectory.Note on sprint dates: Several date ranges have been updated (lines 17–18, 66–71), reflecting revised sprint timelines. Ensure stakeholders and CI/CD pipelines are aligned with these new target dates (e.g., Sprint 1 "DIAS 1-6 CONCLUÍDOS, FINALIZANDO", Sprint 2 "3 Dez - 16 Dez").
Also applies to: 17-18, 66-71
833-839: Justified deferral of coverage improvements to Sprint 2.The roadmap appropriately defers "Coverage Improvement" (35.11% → 80%+) from Sprint 1 to Sprint 2 with clear justification: "Focar em code review de qualidade antes de adicionar novos testes" and planning Sprint 2 as a dedicated 2-week cycle (3-16 Dez) for coverage + collections + tools update.
This decision aligns with the PR title ("Improve tests coverage") and the Sprint 2 detailed planning (lines 885–954), which includes comprehensive tasks for unit tests, integration tests, E2E tests, health checks, data seeding, and observability improvements.
src/Modules/Providers/Domain/Entities/ProviderService.cs (1)
42-42: LGTM! Formatting improvement for readability.The blank line improves visual separation between the
ProviderIdvalidation and theServiceIdvalidation, making the constructor more readable.tests/MeAjudaAi.E2E.Tests/Integration/ApiVersioningTests.cs (1)
47-52: AAA structure and readability look good hereThe added blank line between the Arrange (
AuthenticateAsAdmin();) and Act sections improves the AAA separation without changing behavior. No issues from this change.src/Modules/Providers/Application/Handlers/Commands/ActivateProviderCommandHandler.cs (2)
12-27: Well-structured handler with comprehensive documentation.The handler follows the naming convention, uses primary constructor for DI, implements
ICommandHandler<,>properly, and has good XML documentation covering the purpose, parameters, and integration points. The use ofResultpattern aligns with coding guidelines.
116-120: Generic exception handling is appropriate for the handler boundary.The catch-all exception handler logs the error and returns a generic failure, which is suitable for a command handler boundary. However, ensure that upstream callers can distinguish between validation failures and system errors if needed for different handling (e.g., retry logic).
src/Modules/Providers/Tests/Unit/Application/Handlers/Commands/ActivateProviderCommandHandlerTests.cs (2)
35-80: Comprehensive happy path test.The test correctly sets up the provider in
PendingDocumentVerificationstate viaCompleteBasicInfo, mocks all four document validations to pass, and verifies both the result and provider state change. Good verification ofUpdateAsyncbeing called.
16-33: Well-structured test class with proper setup.Clean constructor-based setup with mocks for all dependencies. The handler instantiation follows best practices for unit testing.
src/Modules/Providers/Infrastructure/Migrations/20251126174955_InitialCreate.Designer.cs (1)
15-15: Confirm migration ID change against existing databasesUpdating the
[Migration("20251126174955_InitialCreate")]timestamp changes how EF orders and identifies this migration. If a previousInitialCreatewith a different ID was ever applied to a shared database, this can desync__EFMigrationsHistory.Please double‑check that:
- Old
InitialCreateartifacts (files and history row) are not in use anymore, or- Any existing databases are recreated / migration history is cleaned up to match this new ID.
tests/MeAjudaAi.Shared.Tests/Infrastructure/SharedTestContainers.cs (3)
3-4: LGTM!The Azurite container field addition follows the same pattern as the existing PostgreSQL container, maintaining consistency.
Also applies to: 16-16
33-43: LGTM!The
Azuriteproperty follows the established pattern and includes proper XML documentation as required by coding guidelines.
149-162: LGTM!Parallel container shutdown is well-implemented with proper null checks before adding tasks to the list.
.github/workflows/pr-validation.yml (4)
44-49: Azurite service configuration for Azure Storage testing is well-structured.The addition of Azurite emulator with correct port mappings (10000, 10001, 10002) aligns well with the test infrastructure expansion. The credentials hardcoded in environment variables (lines 174-175) are properly documented as test-only per Azure documentation.
318-388: Hangfire compatibility tests properly enforce deployment blocking.The critical section for Hangfire + Npgsql compatibility is well-implemented: explicit failure modes, clear documentation of the breaking changes, multiple mitigation options provided, and proper deployment block when tests fail. The messaging and error handling are excellent for a critical path test.
649-680: Coverage reporting strategy with multi-format fallback is robust.The sticky PR comment with auto-detection of multiple coverage formats (OpenCover/Cobertura/generic XML) and defensive fallback logic (lines 554-644) is a solid pattern for handling variable CI environments. Clear messaging to users about coverage thresholds and where to find detailed reports is user-friendly.
839-925: Security scan with OSV-Scanner properly fails on HIGH/CRITICAL vulnerabilities.The vulnerability severity evaluation using jq with proper score normalization (lines 873-897) and explicit failure on HIGH/CRITICAL vulnerabilities (score >= 7.0) is appropriate. Unlike coverage, there's no lenient fallback mode here—security validation is enforced, which is correct.
src/Modules/Providers/Tests/Unit/Application/Validators/CreateProviderRequestValidatorTests.cs (1)
1-376: Comprehensive and well-structured validator test suite.The tests provide thorough coverage of
CreateProviderRequestValidatorincluding:
- Required field validations (UserId, Name, BusinessProfile, ContactInfo, PrimaryAddress)
- GUID format validation
- String length constraints (min/max)
- Enum validation
- Nested object validation with precise error messages
The helper methods (
CreateValidRequest,CreateValidBusinessProfile,CreateValidAddress) promote reusability and follow the same patterns asUpdateProviderProfileRequestValidatorTests.src/Modules/Providers/Tests/Unit/Application/Handlers/Commands/RejectProviderCommandHandlerTests.cs (3)
30-59: Good test coverage for the happy path.The test properly arranges a provider with
Pendingverification status, executes the rejection, and verifies both the result success and the repository update call.
61-153: Solid coverage of failure scenarios.The tests appropriately cover:
- Provider not found (returns failure, no update called)
- Empty/blank Reason validation (short-circuits before repository call)
- Empty/blank RejectedBy validation
- Repository exception handling (returns generic failure message)
This aligns well with the handler implementation shown in the relevant code snippets.
52-54: The review comment's concern is unfounded. TheReject()method correctly updates theStatusproperty.The
Provider.Reject()method (lines 521–535) callsUpdateStatus(EProviderStatus.Rejected, updatedBy)on line 533, which properly sets theStatusproperty toRejected. The test assertionprovider.Status.Should().Be(EProviderStatus.Rejected)on line 54 is correct and aligns with the domain behavior. No adjustment is needed.src/Modules/Providers/Tests/Unit/Application/Validators/UpdateVerificationStatusRequestValidatorTests.cs (1)
1-104: Good boundary testing and optional field coverage.The tests properly cover:
- Valid requests with and without notes
- Invalid enum values
- Maximum length boundary (exactly 1000 and exceeding 1000)
The boundary test at exactly 1000 characters is a good practice for catching off-by-one errors in validation rules.
src/Modules/Providers/Tests/Unit/Application/Validators/UpdateProviderProfileRequestValidatorTests.cs (1)
1-158: Well-structured tests with consistent patterns.The test class follows the same structure as
CreateProviderRequestValidatorTests, with matching helper methods for creating valid test data. This consistency aids maintainability.src/Modules/Providers/Tests/Unit/Application/Validators/RejectProviderCommandValidatorTests.cs (1)
1-148: Comprehensive validation coverage matching audit requirements.The tests thoroughly cover:
- ProviderId presence validation
- RejectedBy field validation with audit-focused error messages
- Reason length constraints (min 10, max 1000 characters)
The test structure mirrors
SuspendProviderCommandValidatorTests, maintaining consistency across similar command validators.src/Modules/Providers/Tests/Unit/Application/Validators/AddDocumentRequestValidatorTests.cs (1)
1-159: Good coverage of document number validation rules.The tests cover required fields, length constraints, character restrictions, and enum validation comprehensively.
src/Modules/Providers/Tests/Unit/Application/Validators/SuspendProviderCommandValidatorTests.cs (1)
1-148: Consistent and thorough validation test suite.The tests mirror the structure of
RejectProviderCommandValidatorTests, ensuring consistent validation behavior across similar administrative commands. All validation rules are covered including:
- ProviderId presence
- Audit fields (SuspendedBy) with length limits
- Reason with minimum (10) and maximum (1000) character constraints
tools/MigrationTool/Program.cs (1)
92-147: Reflection-based AddDbContext lookup looks solid; consider adding tests around itThe new method discovery and invocation logic is much safer: you now filter by name, generic definition, and exact parameter types, throw a clear
InvalidOperationExceptionif the method isn’t found, and unwrapTargetInvocationExceptionwith context info. The DbContext options configuration (Npgsql + per-schema history table, optional NetTopologySuite forSearchProviders,UseSnakeCaseNamingConvention, and detailed logging) is consistent and addresses the earlier review concerns.Given this is reflection-heavy and sits on a critical migrations path, I’d recommend adding or extending tests that exercise
RegisterAllDbContexts(or the CLI entry path) to fail fast if the underlyingAddDbContextsignature changes or a module’s DbContext can’t be registered, in line with your “add/update tests for new functionality” practice. Based on learnings, this helps keep coverage strong around this behavior.src/Modules/Providers/Tests/Unit/Application/Handlers/Commands/SuspendProviderCommandHandlerTests.cs (2)
15-29: Fixture setup for handler and mocks looks goodThe constructor cleanly wires up fresh mocks and a
SuspendProviderCommandHandlerinstance per test instance, which keeps tests isolated and easy to reason about. No changes needed here.
98-144: Validation tests forReasonandSuspendedBycleanly cover early returnsThe two
[Theory]tests (HandleAsync_WithEmptyReason_ShouldReturnFailureandHandleAsync_WithEmptySuspendedBy_ShouldReturnFailure) nicely exercise the validation branches for whitespace/empty/null inputs and assert that the correct error messages are returned while the repository is never touched.The use of multiple
InlineDatavalues plusVerify(..., Times.Never)onGetByIdAsyncis clear and effective. No changes needed.
src/Modules/Providers/Tests/Integration/ProvidersIntegrationTestBase.cs
Outdated
Show resolved
Hide resolved
src/Modules/Providers/Tests/Integration/ProvidersIntegrationTestBase.cs
Outdated
Show resolved
Hide resolved
...les/Providers/Tests/Unit/Application/Handlers/Commands/SuspendProviderCommandHandlerTests.cs
Outdated
Show resolved
Hide resolved
…ryDocument commands Add comprehensive unit tests for command handlers with 0% coverage: **CompleteBasicInfoCommandHandler** (3 tests): - Valid provider completes basic info successfully - Non-existent provider returns failure - Repository exception returns failure **SetPrimaryDocumentCommandHandler** (5 tests): - Valid document sets primary successfully - Non-existent provider returns not found - Non-existent document returns not found - Domain exception returns failure - Repository exception returns internal error Test Coverage: - Mocks: IProviderRepository, ILogger - Builder: ProviderBuilder with fluent API - Assertions: FluentAssertions for result validation - Scenarios: Happy path, validation errors, exceptions These tests target Commands showing 0% coverage to improve Providers.Application module coverage towards 80% goal.
**Security & Validation**: - pr-validation.yml: Enforce strict secret validation - fail with exit 1 when POSTGRES_PASSWORD is missing in main repo on pull_request - ProvidersIntegrationTestBase: Fix SQL injection risk by changing ExecuteSqlAsync to ExecuteSqlRawAsync for schema/table identifiers in TRUNCATE/DELETE statements (lines 167, 175-177, 200, 212-214) **Test Fixes**: - SuspendProviderCommandHandlerTests: Unskip test by ensuring Provider is in valid state (CompleteBasicInfo), remove Skip attribute and redundant try/catch - SharedTestContainers: Add comment explaining Azurite 3.33.0 pin for production CI/CD compatibility **Code Quality**: - Run dotnet format to fix whitespace formatting in Activate/Reject/SuspendProviderCommandHandlerTests - Fix migration file encoding issue (UTF-8 BOM) All changes address CI failures and improve code security/quality.
**Test Improvements**: - Add UpdateAsync exception test in ActivateProviderCommandHandlerTests - Add repository interaction verifications in SuspendProviderCommandHandlerTests - Convert valid number formats test to Theory with InlineData in AddDocumentRequestValidatorTests **Documentation**: - Enhance STRICT_COVERAGE comment in pr-validation.yml to emphasize critical merge requirement **Test Coverage**: - New test: HandleAsync_WhenUpdateAsyncThrowsException_ShouldReturnFailure - Enhanced assertions: Verify GetByIdAsync Times.Once and UpdateAsync Times.Never - Better test reporting: Theory tests now show individual parameter failures All tests passing (7/7 in filtered run, 393 total in Providers module)
PROBLEM: Coverage was measuring ALL assemblies instead of targeted modules - Shared tests (782) showed only 3.46% coverage globally - All ~30 modules were instrumented despite Include filter - Multiple -- arguments in dotnet test caused only last one to be used ROOT CAUSE: Incorrect filter syntax and argument passing - Used [Module] instead of [Module]* (wildcard required by coverlet) - Passed -- arguments separately instead of using runsettings file - Coverlet requires wildcards to match types within assemblies SOLUTION: 1. Generate temporary .runsettings file per module with correct XML structure 2. Add wildcard * to Include/Exclude filters: [MeAjudaAi.Shared]* 3. Use --settings parameter instead of multiple -- arguments VERIFICATION (local testing): - Before: 20+ modules instrumented, 6.83% coverage - After: 1 module (Shared only), 31.02% coverage ✅ - Shared: 2040/6575 lines covered by 782 tests EXPECTED PIPELINE IMPACT: - Users: ~600 tests → ~50-60% coverage - Providers: 545 tests → ~55-65% coverage - Documents: 112 tests → ~40-50% coverage - ServiceCatalogs: 155 tests → ~45-55% coverage - Locations: 135 tests → ~50-60% coverage - SearchProviders: 107 tests → ~35-45% coverage - Shared: 782 tests → ~31% coverage - Combined: ~40-50% global coverage (vs 3.46% broken state)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/Shared/Queries/QueryDispatcher.cs (1)
7-15: Class XML documentation accurately describes behaviorThe summary/remarks match the actual behavior (DI-resolved handlers plus pipeline behaviors around execution) and clearly frame this as a mediator-style dispatcher. Nice improvement in API clarity, and it aligns with the requirement for public types to have XML docs.
If you want to go further, you could briefly note expected lifetime/registration pattern (e.g., singleton) in the remarks, but that’s optional. As per coding guidelines, ...
.github/workflows/pr-validation.yml (4)
885-885: Confusing parameter expansion defaults obscure intent.Both lines use
${STRICT_COVERAGE:-true}as a fallback, butSTRICT_COVERAGEis explicitly set tofalseon line 25. The environment variable takes precedence over the default (:-true), so the code works correctly but the intent is misleading to future maintainers.Recommendation: Either remove the misleading default or clarify with an inline comment:
# STRICT_COVERAGE is intentionally set to false (line 25) during development. # The :-true fallback only applies if the variable is unset (shouldn't happen in CI). if [ "${STRICT_COVERAGE:-true}" = "true" ]; thenAlternatively, simplify to:
if [ "${STRICT_COVERAGE}" = "true" ]; thenAlso applies to: 910-910
314-314: Consider parameterizing EXCLUDE_FILTER for module-specific exclusion patterns.Line 314 defines a single
EXCLUDE_FILTERapplied to all modules:EXCLUDE_FILTER="[*.Tests*]*,[*Test*]*,[testhost]*"This works for most modules but prevents module-specific exclusion strategies (e.g., excluding different test categories or infrastructure patterns per module). If future modules require different exclusion logic, the hardcoded pattern will become a bottleneck.
Non-blocking suggestion: Consider making exclusion patterns part of the module definition (parallel to
include_patternon line 298) for flexibility without changing the current workflow logic.
318-318: Minor: Remove trailing whitespace on blank line.Line 318 appears to contain trailing whitespace on an otherwise blank line. Apply this fix:
echo " Exclude: $EXCLUDE_FILTER" - + # Create temporary runsettings file for this module
767-783: Consider extracting coverage validation logic to a reusable GitHub Action.The 140-line validation step (lines 765–918) implements multi-stage coverage fallback logic that could be extracted into
.github/actions/validate-coverage/. This would:
- Enable reuse across workflows (nightly builds, release validation)
- Improve maintainability and testability in isolation
- Clarify the separation between orchestration (workflow) and validation (action)
Suggested structure:
action.ymlwith inputs (coverage-file patterns, threshold, strict-mode flag)- Separate scripts for output selection, extraction, and validation stages
- Clear inline documentation for fallback triggers
This is a nice-to-have improvement (not blocking) but aligns with the inline comments at lines 773–783.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/pr-validation.yml(12 hunks)src/Shared/Queries/QueryDispatcher.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cs,csx}
📄 CodeRabbit inference engine (AGENT.md)
Use UUID v7 (time-ordered) instead of UUID v4 for entity IDs in .NET 9 projects
Files:
src/Shared/Queries/QueryDispatcher.cs
**/*.cs
📄 CodeRabbit inference engine (AGENT.md)
**/*.cs: All public APIs should have XML documentation comments
Code style and formatting must adhere to .editorconfig rules
Files:
src/Shared/Queries/QueryDispatcher.cs
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
📚 Learning: 2025-11-25T16:00:52.775Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
Applied to files:
.github/workflows/pr-validation.yml
📚 Learning: 2025-11-25T16:00:52.775Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Run all tests locally before submitting a PR
Applied to files:
.github/workflows/pr-validation.yml
🪛 actionlint (1.7.9)
.github/workflows/pr-validation.yml
321-321: could not parse as YAML: could not find expected ':'
(syntax-check)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-test
- GitHub Check: code-analysis
🔇 Additional comments (7)
src/Shared/Queries/QueryDispatcher.cs (1)
18-29: Method XML documentation is consistent with implementationThe type parameters, parameters, return value, and documented
InvalidOperationExceptioncase all match the implementation and DI behavior. This brings the public API in line with the XML-doc requirement without altering runtime semantics.As per coding guidelines, ...
.github/workflows/pr-validation.yml (6)
18-25: Approved: STRICT_COVERAGE flag configuration is well-documented.The multi-line comment clearly explains the temporary bypass (lines 18-23) and references tracking issue #33 (Sprint 2 milestone). The structure aligns with the learnings requiring 70% minimum coverage.
73-122: Approved: Comprehensive and context-aware secrets validation.Excellent improvement over the previous version:
- ✅ Validates ALL three database secrets (POSTGRES_PASSWORD, POSTGRES_USER, POSTGRES_DB) with explicit error messages per missing secret
- ✅ Context-aware validation: strict mode (main repo PRs/dispatch) vs lenient mode (forks/local)
- ✅ Clear separation of concerns: environment check → repository context → multi-secret validation → appropriate error handling
269-294: Approved: Well-documented module coverage strategy.The strategy section (lines 269-280) clearly explains:
- Module selection rationale (Domain/Application for core business logic, Shared/ApiService for infrastructure)
- Why infrastructure tests are excluded from module-based runs (avoiding unit/integration distortion)
- Separation of concerns (unit vs system/integration testing)
This directly addresses the root cause identified in the PR objectives (mocked dependencies hiding repository/persistence code).
307-345: Approved: Per-module runsettings for flexible coverage configuration.Generating module-specific runsettings files (lines 320–335) enables per-module include/exclude filters and format customization, which is a solid pattern for multi-module test orchestration.
945-986: Approved: Hardened OSV-Scanner installation with retry and fallback logic.Addresses the fragility of network-dependent tool installation:
- Retry loop (lines 956–966) handles transient download failures
- Go-based fallback (lines 972–976) when curl fails
- Explicit failure on unavailability (line 982) ensures mandatory security scanning
1-1130: Overall: Substantial improvements to test coverage infrastructure.This workflow revision demonstrates significant progress toward the PR objectives:
✅ Root cause addressed: Module-based coverage with strategy documentation (lines 269–294) explicitly separates unit tests (mocked) from infrastructure tests, reducing distortion in coverage metrics.
✅ Security hardened: Context-aware secrets validation (lines 73–122) prevents silent fallback to placeholders in production CI; OSV-Scanner now has retry logic and fails workflow on unavailability (lines 945–986).
✅ Coverage pipeline enhanced: Multi-module testing with per-module runsettings (lines 307–345), multi-stage fallback for coverage reports (lines 554–918), and clear thresholds (70% min, 85% recommended) aligned with learnings.
✅ Operational clarity: Extensive inline documentation explains rationale for module selection, infrastructure test exclusion, and fallback chains—supporting future maintenance and onboarding.
Aligns with learnings:
- Minimum coverage threshold set to 70% ✅
- Context-aware CI validation ✅
Recommended follow-up:
- Work toward 70% threshold to re-enable
STRICT_COVERAGE(issue #33)- Add integration tests for repository/persistence layers (identified root cause from PR objectives)
- Test coverage improvements locally before pushing (per learnings: "run all tests locally before submitting a PR")
feat(ci): clarify STRICT_COVERAGE parameter expansion with inline comments - Added explanatory comments for :-true fallback behavior (lines 887, 913) - STRICT_COVERAGE is explicitly false in env (line 25) for development - Fallback only applies if variable somehow becomes unset in CI - Improves maintainability and prevents future confusion docs(ci): document EXCLUDE_FILTER parameterization opportunity - Added NOTE about current hardcoded exclusion pattern for all modules - Suggested future enhancement: per-module exclusion via MODULES array - Format: Module:path/:Include:Exclude for module-specific patterns - Non-blocking improvement for future flexibility Addresses code review feedback on workflow clarity and extensibility
…ogging PROBLEM: Coverage report not showing in pipeline - Module tests may not be building properly with --no-build flag - No visibility into coverage file generation process CHANGES: 1. Removed --no-build flag from dotnet test command - Ensures tests are built even if solution build didn't include them - Prevents silent failures when dependencies change 2. Added comprehensive debug logging: - List directory contents when searching for coverage files - Show found coverage file path - Display available XML files when coverage not found - Summary of all generated coverage files after module tests 3. Enhanced error messages: - Clear indication when coverage directory missing - List of actual files found vs expected patterns EXPECTED OUTCOME: - Tests will build and run reliably - Debug logs will reveal if coverage files are generated but not found - Easier troubleshooting of coverage collection issues Related to: Coverage report showing 3.46% instead of expected 40-50%
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/pr-validation.yml (1)
770-787: Consider extracting coverage validation to a custom action.The 140-line validation step (lines 769–927) implements a complex multi-stage fallback strategy with clear documentation. As suggested in the inline comment, extracting to
.github/actions/validate-coverage/would improve reusability across workflows (nightly builds, release validation) and testability, though this is not blocking for this PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/pr-validation.yml(14 hunks)src/Shared/Queries/QueryDispatcher.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cs,csx}
📄 CodeRabbit inference engine (AGENT.md)
Use UUID v7 (time-ordered) instead of UUID v4 for entity IDs in .NET 9 projects
Files:
src/Shared/Queries/QueryDispatcher.cs
**/*.cs
📄 CodeRabbit inference engine (AGENT.md)
**/*.cs: All public APIs should have XML documentation comments
Code style and formatting must adhere to .editorconfig rules
Files:
src/Shared/Queries/QueryDispatcher.cs
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
📚 Learning: 2025-11-25T16:00:52.775Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
Applied to files:
.github/workflows/pr-validation.yml
📚 Learning: 2025-11-25T16:00:52.775Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Run all tests locally before submitting a PR
Applied to files:
.github/workflows/pr-validation.yml
🧬 Code graph analysis (1)
src/Shared/Queries/QueryDispatcher.cs (2)
src/Shared/Logging/CorrelationIdEnricher.cs (1)
IServiceProvider(69-74)tests/MeAjudaAi.Shared.Tests/Infrastructure/TestLoggingConfiguration.cs (1)
ILogger(82-82)
🪛 actionlint (1.7.9)
.github/workflows/pr-validation.yml
325-325: could not parse as YAML: could not find expected ':'
(syntax-check)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-test
- GitHub Check: code-analysis
🔇 Additional comments (9)
.github/workflows/pr-validation.yml (6)
73-122: ✅ Secret validation now properly comprehensive.The validation step correctly checks all three critical database secrets (POSTGRES_PASSWORD, POSTGRES_USER, POSTGRES_DB) with environment-aware strict/lenient modes. The previous concern about asymmetric validation has been addressed; misconfiguration in main repo context will now fail loudly, while forks/non-PR events gracefully use test defaults.
269-279: ✅ Well-documented test strategy.The comments clearly explain the rationale for excluding infrastructure tests from module-based coverage runs. The separation ensures unit test coverage accurately reflects business logic coverage rather than being distorted by integration test results.
307-322: ✅ Dynamic filter and runsettings generation is sound.The per-module include/exclude filters and auto-generated runsettings files enable modular coverage collection while maintaining consistency. The approach allows future granularity (e.g., per-module exclusions) without immediate refactoring.
325-325: Actionlint false positive — safe to ignore.Line 325 triggers an actionlint "could not find expected ':'" error, but this is within a shell
catheredoc that generates XML (runsettings), not YAML. Actionlint is confused by the inline XML. No action required.
404-475: ✅ Hangfire/Npgsql compatibility validation is robust.The explicit messaging and deployment-blocking logic around Hangfire + Npgsql 10.x compatibility is well-thought-out. The mitigation options (downgrade, wait, alternative) provide clear guidance if tests fail.
953-995: ✅ OSV-Scanner hardening with retry logic and fallback.The three-attempt retry with exponential backoff (5s between retries) and Go-based fallback installation significantly improve reliability over a simple curl-and-fail approach. The mandatory failure (
exit 1) when vulnerability scanning is unavailable correctly prioritizes security.src/Shared/Queries/QueryDispatcher.cs (3)
7-20: Excellent documentation and visibility change!The XML documentation is comprehensive and clearly explains the mediator pattern, pipeline behavior integration, and DI registration requirements. The visibility change from internal to public aligns with enabling DI registration in consumer code.
22-34: Well-documented public method.The XML documentation comprehensively covers all type parameters, parameters, return value, and exceptions. This follows the coding guideline requirement for public API documentation.
47-64: Test coverage for pipeline execution logic is already adequately addressed.The existing test suite in
tests/MeAjudaAi.Shared.Tests/Unit/Queries/QueryDispatcherTests.cscomprehensively covers theExecuteWithPipelinelogic with nine focused test methods:
- Behavior chaining order verified in
QueryAsync_WithMultiplePipelineBehaviors_ShouldExecuteInRegistrationOrder(confirms reverse execution order)- Handler resolution and invocation tested in
QueryAsync_WithQuery_ShouldInvokeHandler- Error propagation covered in
QueryAsync_WhenHandlerThrowsException_ShouldPropagateException- Multiple behaviors interaction validated through explicit CallOrder tracking
- Cancellation token propagation tested in dedicated methods
No additional test coverage is needed for this component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/pr-validation.yml (1)
787-803: Suggested refactor: Extract coverage validation to reusable action.The comment block you've already included at lines 787–803 documents a good future improvement: extracting this 140-line validation logic to
.github/actions/validate-coverage/. This would improve:
- Reusability across workflows (nightly builds, release validation)
- Testability in isolation
- Clearer separation of concerns (validation logic vs. orchestration)
This is a good-to-have improvement for a future PR, not blocking for this one.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pr-validation.yml(14 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
📚 Learning: 2025-11-25T16:00:52.775Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
Applied to files:
.github/workflows/pr-validation.yml
📚 Learning: 2025-11-25T16:00:52.775Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Run all tests locally before submitting a PR
Applied to files:
.github/workflows/pr-validation.yml
🪛 actionlint (1.7.9)
.github/workflows/pr-validation.yml
325-325: could not parse as YAML: could not find expected ':'
(syntax-check)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-test
- GitHub Check: code-analysis
🔇 Additional comments (6)
.github/workflows/pr-validation.yml (6)
16-30: ✅ Well-structured environment configuration with proper DRY principles.The centralized credential setup with environment-level fallbacks enables both strict validation in main-repo CI and lenient defaults in fork contexts—exactly what the bifurcated validation step (lines 73-122) requires. The TODO comment linking to issue #33 is properly documented.
73-122: ✅ Excellent expansion of secret validation with context-aware enforcement.This step properly addresses the prior review comment requesting validation of all three database credentials (PASSWORD, USER, DB). The bifurcated approach—strict enforcement in main-repo PR/dispatch contexts, lenient defaults in forks—is exactly the right pattern.
268-381: ✅ Excellent per-module coverage strategy with clear rationale and proper fallback handling.The STRATEGY comment block (lines 269–280) clearly explains why infrastructure tests are excluded from module-based coverage runs—essential context for future maintainers. The per-module runsettings generation and recursive file discovery (handling GUID subdirectories) shows good understanding of .NET test output patterns.
969-1083: ✅ Robust OSV-Scanner installation with appropriate retry logic and security-critical fallback.The three-tier approach (direct download → Go fallback → hard failure) is sound for a security-critical step. The safe jq parsing for both numeric CVSS scores and text severity labels (lines 1035–1048) handles real-world vulnerability data edge cases well. Hard failure when scanning is unavailable (line 1006) correctly prioritizes security.
785-943: ✅ Well-documented multi-stage fallback coverage validation with appropriate complexity comments.The three-tier approach (step outputs → direct XML analysis → lenient file-existence check) is defensive and handles real CI fragility. The comments at lines 787–803 explicitly document the complexity rationale and suggest future extraction to
.github/actions/validate-coverage/—excellent UX for future maintainers. Proper handling of both decimal (0.xx) and percentage (xx) format conversions for Cobertura vs. OpenCover compatibility.
324-339: ✅ Static analysis false positive: actionlint incorrectly flags XML as YAML.The actionlint warning at line 325 (
could not find expected ':') is a false positive. The heredoc at line 324 creates an XML file (valid runsettings content), and actionlint incorrectly attempts to parse XML markup as YAML. This does not represent an actual issue—the XML declaration and runsettings structure are correct and will function as intended.
PROBLEM: 140-line validation logic embedded in pr-validation.yml - Hard to maintain and test in isolation - Cannot be reused across workflows (nightly builds, releases) - Violates separation of concerns (validation vs orchestration) CHANGES: 1. Created .github/actions/validate-coverage/ composite action - action.yml with comprehensive inputs/outputs - Multi-stage fallback: step outputs → direct XML analysis - Supports OpenCover, Cobertura, and generic XML formats - Configurable threshold and strict/lenient mode - Detailed debug logging and error messages 2. Comprehensive README.md with: - Usage examples (basic, with CodeCoverageSummary, multi-format) - Development vs release workflow patterns - Troubleshooting guide - Design decisions documentation - Input/output reference tables 3. Refactored pr-validation.yml: - Replaced 140-line inline script with action call - Passes all step outcomes and outputs as inputs - Maintains same behavior (STRICT_COVERAGE, 70% threshold) - Cleaner workflow orchestration (16 lines vs 163 lines) BENEFITS: ✅ Reusability: Can be used in nightly builds, release validation ✅ Testability: Action can be tested in isolation ✅ Maintainability: Single source of truth for validation logic ✅ Clarity: Separates validation logic from workflow orchestration ✅ Extensibility: Easy to add unit tests or new coverage formats RELATED CODE REVIEW: Addresses suggestion at pr-validation.yml:787-803 to extract validation to reusable action for improved architecture. BACKWARD COMPATIBILITY: ✅ Same inputs (env vars, step outputs) ✅ Same outputs (validation results, error messages) ✅ Same behavior (strict/lenient modes, 70% threshold) ✅ Same fallback logic (OpenCover → Cobertura → Direct Analysis) FUTURE ENHANCEMENTS: - Add action unit tests (when GitHub supports testing composite actions) - Support additional coverage formats (JaCoCo, lcov) - Add branch/method coverage validation - Create coverage trend visualization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/actions/validate-coverage/README.md(1 hunks).github/actions/validate-coverage/action.yml(1 hunks).github/workflows/pr-validation.yml(11 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/actions/validate-coverage/README.md
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
📚 Learning: 2025-11-25T16:00:52.775Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
Applied to files:
.github/workflows/pr-validation.yml.github/actions/validate-coverage/action.yml
📚 Learning: 2025-11-25T16:00:52.775Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Run all tests locally before submitting a PR
Applied to files:
.github/workflows/pr-validation.yml
🪛 actionlint (1.7.9)
.github/workflows/pr-validation.yml
325-325: could not parse as YAML: could not find expected ':'
(syntax-check)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: code-analysis
- GitHub Check: build-and-test
🔇 Additional comments (4)
.github/workflows/pr-validation.yml (4)
73-122: Secret validation enforcement successfully addresses prior security concern.The "Validate Secrets Configuration" step properly enforces strict validation in main-repo pull requests. It correctly:
- Checks
github.repository == "frigini/MeAjudaAi"and event is pull_request/workflow_dispatch (lines 82–84)- Validates all three database secrets (POSTGRES_PASSWORD, POSTGRES_USER, POSTGRES_DB) at lines 90–112
- Exits with error code 1 if any secret is missing in strict context (line 116)
- Allows lenient defaults for forks and non-PR events (lines 118–120)
This resolves the earlier concern about silent fallback credentials enabling CI misconfiguration in the main repository.
16-30: Environment variable defaults with secret fallback enable context-aware secret handling.The workflow-level environment variables (lines 26–30) now pair with strict validation (lines 73–122), creating a two-tier approach:
- Fallback defaults (
'test123','postgres','meajudaai_test') enable local dev and fork PRs to run tests.- Strict validation enforces actual secrets in main-repo pull requests, preventing silent misconfiguration.
This DRY principle (single source of truth for credentials) is propagated throughout the workflow and properly used in service startup (lines 43–45), test execution (lines 151–152, 186–188, 200–202, 206–208), and connection strings (lines 218–221).
Per learnings, minimum coverage is 70%; ensure these secrets remain configured to maintain CI reliability.
To confirm the fallback behavior is intentional in all contexts, verify that:
- Fork PRs successfully use the
'test123'/'postgres'defaults without failures- Main-repo PRs fail loudly if any of the three secrets are missing (lines 90–112)
268-381: Module-based coverage strategy properly separates unit tests from infrastructure validation.The coverage collection logic (lines 268–381) is well-structured:
- Module definitions (lines 282–294): Clear mapping of business logic layers (Domain, Application, Shared, ApiService) with per-module test paths.
- Documented rationale (lines 274–279): Explains why infrastructure/integration tests are excluded from unit coverage metrics—mixing mocked and real dependencies distorts coverage percentages.
- Per-module coverage files (lines 304–377): Each module generates isolated coverage XML files with predictable naming (e.g.,
users.opencover.xml).- Separate system validation (lines 395–416): Architecture, Integration, and E2E tests run without coverage collection, reducing noise.
This design directly supports the PR objective to improve coverage from 35.96% to 70%+ by focusing unit-test metrics on business logic layers rather than infrastructure.
Future enhancement note (line 316): The comment suggests per-module exclusion patterns could be added to the MODULES array definition. This is deferred to a follow-up and does not block this PR.
420-490: CRITICAL: Hangfire + Npgsql 10.x compatibility tests enforce explicit deployment safeguards.The Hangfire integration test step (lines 420–490) implements a strong safety posture:
- Explicit context (lines 436–441): Clearly documents the breaking change between Hangfire.PostgreSql 1.20.12 (compiled against Npgsql 6.x) and Npgsql 10.x.
- Test isolation (line 450): Runs only
Category=HangfireIntegrationtests to validate runtime compatibility.- Clear failure messaging (lines 456–490): If tests fail, provides four mitigation options with specific implementation guidance.
- Deployment blocker (line 489):
exit $TEST_EXIT_CODEprevents merges when compatibility is broken.This demonstrates strong defensive programming against a known breaking change. The messaging aligns with the PR's broader test infrastructure improvements by ensuring critical integrations are validated end-to-end.
… zero PROBLEM: Coverage extraction fails for values like '.5' (0.5 without leading zero) - cut returns empty string for '.5' - Conditional only checks for '0', missing empty string case SOLUTION: Extract integer part into variable and check both cases - Check both empty string and '0' for decimal format detection - Covers both '.5' (empty) and '0.5' (zero) formats EXAMPLES: - '.5' -> '50.0%' (fixed) - '0.833' -> '83.3%' (working) Related: Code review feedback
CHANGES: 1. Rename primary_success to opencover_success for clarity - Variables now explicitly reference source format (OpenCover) - Updated all references throughout action.yml - Improves maintainability and reduces confusion 2. Consolidate GITHUB_OUTPUT assignments - Eliminated duplicate output writes in strict-mode block - Outputs set once after validation completes - Exit happens after outputs are written - Prevents inconsistent state if early exit occurs 3. Add INCLUDE_FILTER validation - Guards against empty/trivial filter patterns like []* or [].* - Falls back to [MeAjudaAi.*]* for safety - Prevents coverage collection failures from invalid filters 4. Optimize coverage file search - Changed from 'head -1' to '-print -quit' for efficiency - Stops searching immediately after first match found - Removed redundant echo statements for created files - Reduces execution time and log noise 5. Add explicit GitHub issue reference - STRICT_COVERAGE TODO now links directly to issue #33 - Improves discoverability of Sprint 2 milestone tracking - Makes transition plan more visible 6. Document Go runtime requirement for OSV-Scanner - Added informative message when using Go fallback - Documents minimum Go version requirement (>=1.18) - Provides clear mitigation steps if Go unavailable - Improves troubleshooting for security scan failures RELATED: Code review feedback on PR ci-enable-module-coverage-collection
- HybridCacheServiceAdvancedTests: 14 tests for edge cases - Null/empty key validation - Null factory validation - Cancellation token passthrough - Exception propagation - Custom expiration options - Tag-based invalidation - RemoveAsync/RemoveByTagAsync validation - CommandsExtensionsTests: 8 tests for DI registration - CommandDispatcher registration validation - Scoped lifetime verification - Multiple calls handling - Fluent chaining support - Scope isolation tests - QueriesExtensionsTests: 8 tests for DI registration - QueryDispatcher registration validation - Scoped lifetime verification - Multiple calls handling - Fluent chaining support - Scope isolation tests Contributes to Sprint 2 coverage goal of 70% Related to #33
The 3 test files cannot compile because: - AddCommands/AddQueries are internal extension methods (not accessible from test assembly) - HybridCacheService requires CacheMetrics parameter in constructor - These components are already tested indirectly through existing integration tests Will create alternative tests for coverage gaps.
Force re-run of PR validation to use the updated workflow from master (PR #34) that includes module coverage collection
Add comprehensive test coverage for foundational Shared components: - ValidationExceptionTests: 10 tests - Default and parameterized constructors - Error collection handling - Multiple failures on same property - Null failure handling - FluentValidation.Results integration - DomainExceptionTests: 10 tests - Constructor overloads (message, inner exception) - Exception inheritance chain - Special characters and long messages - Inner exception preservation - Stack trace handling - DomainEventTests: 19 tests - AggregateId and Version assignment - Unique ID generation (UUID v7) - OccurredAt UTC timestamp - EventType name reflection - UUID v7 monotonic ordering - IDomainEvent interface compliance - Record equality semantics Total new tests: 39 Expected coverage increase: +3-5% for Shared module Related to #33 - Sprint 2 coverage goal
Fix compilation error caused by namespace conflict between Bogus.ValidationException and MeAjudaAi.Shared.Exceptions.ValidationException Changes: - Add using alias to disambiguate ValidationException - Fix FluentAssertions ambiguous Contain() call with explicit assertion This ensures tests compile correctly in CI pipeline.
Apply dotnet format to fix whitespace formatting errors in test files: - DomainEventTests.cs: Fix line breaks and indentation - DomainExceptionTests.cs: Fix line breaks - ValidationExceptionTests.cs: Auto-formatting fixes This resolves CI format verification failures.
- Add detailed coverage roadmap (docs/testing/code-coverage-roadmap.md) - Global coverage: ~45% (Shared: 31.21% measured, domain modules: 38-72%) - Document 10 component categories with gap analysis - Create 4-phase improvement plan to reach 70% coverage target - Fix concurrency test skip in PermissionMetricsServiceTests - Users module leads with 68% coverage (684 tests) - ApiService critical gap at 17% (minimal health check coverage) - Phase 1 targets: Database + ApiService health checks (+8%) Related to Sprint 2 coverage goals and PR merge preparation
There was a problem hiding this 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
🧹 Nitpick comments (4)
docs/testing/code-coverage-roadmap.md (4)
8-8: Polish Portuguese grammar per static analysis hints.Minor style improvements recommended by LanguageTool for Portuguese consistency:
- Line 8: Conjunctions separating clauses should use comma formatting.
- Line 422: Similar conjunction issue (mas → , mas).
- - **Status**: 🟡 Progresso mas abaixo da meta + - **Status**: 🟡 Progresso, mas abaixo da meta - - **Status**: 🟡 Workflow atualizado mas relatórios não aparecem + - **Status**: 🟡 Workflow atualizado, mas relatórios não aparecemAlso applies to: 422-422
182-182: Replace foreignisms with Portuguese equivalents for consistency.LanguageTool flagged preferred Portuguese terms:
- Line 182: "performance" → "desempenho"
- Line 381: "suite" → "suíte"
- - **Impacto**: Médio - performance + - **Impacto**: Médio - desempenho - 5. **Tempo de Execução**: <5min para suite completa + 5. **Tempo de Execução**: <5min para suíte completaAlso applies to: 381-381
228-325: Assess time estimates against known infrastructure blockers.The action plan spans Phases 1–4 across 4 sprints (~24 developer-days of work, lines 236–324). However, the PR objectives note that Docker/TestContainers timeout issues cause ~68 integration/E2E test failures locally, preventing reliable local validation. Phase 1's Database Layer and ApiService Health Checks tests (both P0) may require resolving these infrastructure issues first.
Recommendations:
- Consider adding infrastructure remediation (Docker/TestContainers setup) as a prerequisite or parallel workstream.
- Adjust Phase 1 timeline if Docker/TestContainers debugging is needed before writing tests.
- Document any dependencies on infrastructure fixes in the roadmap.
390-391: Remove duplicate critical items from technical notes.Lines 390–391 duplicate priority flags (Authorization, ApiService marked critical) that are already documented in the "Gaps Críticos Identificados" section (lines 135–225). This redundancy reduces clarity.
- - 🔴 Shared Authorization: 40% (CRÍTICO) - - 🔴 ApiService: 15% (CRÍTICO)Remove these lines and rely on the structured gaps section above for priority tracking.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/testing/code-coverage-roadmap.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
📚 Learning: 2025-11-25T16:00:52.775Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
Applied to files:
docs/testing/code-coverage-roadmap.md
🪛 LanguageTool
docs/testing/code-coverage-roadmap.md
[uncategorized] ~8-~8: Esta conjunção deve ser separada por vírgulas e só deve ser utilizada no início duma frase para efeitos de estilo.
Context: ...Meta Sprint 2: 70% - Status: 🟡 Progresso mas abaixo da meta - Total de Testes: ~...
(VERB_COMMA_CONJUNCTION)
[uncategorized] ~85-~85: Encontrada possível ausência de vírgula.
Context: ...es**: Entities, Value Objects, Commands/Queries handlers bem cobertos **Providers Modu...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~109-~109: Encontrada possível ausência de vírgula.
Context: ...diana - Gaps: CEP validation logic, ViaCEP API integration, geocoding **SearchPro...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~115-~115: Encontrada possível ausência de vírgula.
Context: ... 🔴 Abaixo da meta - Gaps Críticos: PostGIS geospatial queries, radius search, dist...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[locale-violation] ~131-~131: “Suite” é um estrangeirismo. É preferível dizer “suíte”.
Context: ...ída (testes end-to-end) - Status: ✅ Suite robusta, mas não conta para coverage me...
(PT_BARBARISMS_REPLACE_SUITE)
[locale-violation] ~182-~182: “performance” é um estrangeirismo. É preferível dizer “desempenho”, “atuação”, “apresentação”, “espetáculo” ou “interpretação”.
Context: ...sure handling - Impacto: Médio - performance - Prioridade: P2 7. **Authoriza...
(PT_BARBARISMS_REPLACE_PERFORMANCE)
[uncategorized] ~219-~219: Encontrada possível ausência de vírgula.
Context: ...Users Domain** (65-75%) - Entities, Value Objects, Events - Status: ✅ Mód...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[typographical] ~342-~342: Símbolo sem par: “(“ aparentemente está ausente
Context: ...55% - SearchProviders: 50%rint Atual) - Global: ~42% - Shared: 41% - ...
(UNPAIRED_BRACKETS)
[locale-violation] ~381-~381: “suite” é um estrangeirismo. É preferível dizer “suíte”.
Context: ...es 5. Tempo de Execução: <5min para suite completa ### Notas Técnicas ### Testes...
(PT_BARBARISMS_REPLACE_SUITE)
[uncategorized] ~422-~422: Esta conjunção deve ser separada por vírgulas e só deve ser utilizada no início duma frase para efeitos de estilo.
Context: ...ipeline CI/CD - Status: 🟡 Workflow atualizado mas relatórios não aparecem - Issue: Gi...
(VERB_COMMA_CONJUNCTION)
[grammar] ~423-~423: Possível erro de concordância.
Context: ...em - Issue: GitHub Actions workflow do base branch - Solução: Workflow já merge...
(GENERAL_GENDER_AGREEMENT_ERRORS)
[style] ~437-~437: Prefira a forma completa do verbo “estar”.
Context: ...ilter "FullyQualifiedName!~Integration")tar cobertura por módulo - ✅ Reusable actio...
(INFORMAL_TÁ)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-test
- GitHub Check: code-analysis
🔇 Additional comments (1)
docs/testing/code-coverage-roadmap.md (1)
1-15: Verify coverage targets align with team standards.The roadmap targets 70% global coverage (line 7, 304, 362) as the final goal. According to team learnings, 70% is the CI warning threshold (minimum), with 85% recommended and 90%+ considered excellent. Consider whether the Phase 4 target should aim higher to align with best practices.
Confirm whether:
- 70% is the intended long-term target, or if the roadmap should project toward 85%+ in a future phase?
- The 4-sprint timeline (lines 447–448) remains realistic given the documented Docker/TestContainers timeout issues that cause ~68 integration test failures locally?
- Fix corrupted text '50%rint Atual)' in coverage metrics - Fix garbled Integration filter line '...Integration\
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/testing/code-coverage-roadmap.md (2)
79-127: Fix language/formatting inconsistencies flagged by static analysis.Several minor issues impact document consistency:
Line 85, 87, 99, 105, 111, 117, 123, 128: Module header formatting is inconsistent; some use
⭐emoji while others don't. Standardize format across all module sections.Line 131: English "Suite" should be Portuguese "Suíte" per language guidelines.
Line 421: Grammar agreement error; "workflow do base branch" should be "workflow da branch base" or similar.
While these don't impact content quality, standardizing language and formatting improves professional polish.
Apply these fixes:
- **Status**: ✅ Suite robusta, mas não conta para coverage metrics + **Status**: ✅ Suíte robusta, mas não conta para coverage metrics- **Pipeline CI/CD** - - **Status**: 🟡 Workflow atualizado, mas relatórios não aparecem - - **Issue**: GitHub Actions workflow do base branch + **Pipeline CI/CD** + - **Status**: 🟡 Workflow atualizado, mas relatórios não aparecem + - **Issue**: GitHub Actions workflow da branch base
361-371: Acknowledge gap between Phase 4 target (70% global) and recommended industry standard (85%).The roadmap achieves the stated 70% target but several modules remain below this in the Phase 4 projection:
- Line 370: ApiService projected at only 45%
- Line 369: SearchProviders at 65%
- Line 368: Locations at 62%
Industry best practice recommends 80% as a starting threshold, and per learnings, 85% is recommended. The Phase 4 result of 70% global meets the minimum CI threshold but not the recommended target.
Consider adding a Phase 5 or post-roadmap plan to address:
- Infrastructure-heavy modules (ApiService, SearchProviders geospatial)
- Cross-module integration scenarios
- Push toward 80-85% recommended coverage
This clarification helps set proper expectations with stakeholders.
Add a section after line 371 (e.g., "## Beyond Phase 4: Roadmap to 85%") that outlines the work needed to reach recommended coverage levels.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/testing/code-coverage-roadmap.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
📚 Learning: 2025-11-25T16:00:52.775Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
Applied to files:
docs/testing/code-coverage-roadmap.md
🪛 LanguageTool
docs/testing/code-coverage-roadmap.md
[uncategorized] ~85-~85: Encontrada possível ausência de vírgula.
Context: ...es**: Entities, Value Objects, Commands/Queries handlers bem cobertos **Providers Modu...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~109-~109: Encontrada possível ausência de vírgula.
Context: ...diana - Gaps: CEP validation logic, ViaCEP API integration, geocoding **SearchPro...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~115-~115: Encontrada possível ausência de vírgula.
Context: ... 🔴 Abaixo da meta - Gaps Críticos: PostGIS geospatial queries, radius search, dist...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[locale-violation] ~131-~131: “Suite” é um estrangeirismo. É preferível dizer “suíte”.
Context: ...ída (testes end-to-end) - Status: ✅ Suite robusta, mas não conta para coverage me...
(PT_BARBARISMS_REPLACE_SUITE)
[uncategorized] ~219-~219: Encontrada possível ausência de vírgula.
Context: ...Users Domain** (65-75%) - Entities, Value Objects, Events - Status: ✅ Mód...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[grammar] ~421-~421: Possível erro de concordância.
Context: ...em - Issue: GitHub Actions workflow do base branch - Solução: Workflow já merge...
(GENERAL_GENDER_AGREEMENT_ERRORS)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-test
- GitHub Check: code-analysis
🔇 Additional comments (4)
docs/testing/code-coverage-roadmap.md (4)
1-50: Excellent roadmap structure and thorough gap analysis.The document provides clear current state assessment, realistic phased approach, and specific action items. The 70% coverage target aligns with industry standards, and per-module breakdowns enable focused effort prioritization.
228-325: Verify feasibility of effort estimates, particularly for infrastructure layers.The roadmap's effort estimates for Fase 1 (Databases, ApiService health checks) appear optimistic. For example:
- Line 242: "2 dias" for 33 tests (UnitOfWork, DbContextFactory, SchemaIsolationInterceptor) including transaction/rollback scenarios averages ~16 tests/day
- Line 250: "1.5 dias" for 19 tests (PostgreSQL, Redis, Azurite, RabbitMQ health checks) averages ~13 tests/day
While unit-level tests can be written quickly, infrastructure-layer tests often require:
- Additional setup/teardown (TestContainers, schema isolation)
- Debugging environmental issues (Docker Desktop timeouts mentioned in PR objectives)
- Coordination with external services
Recommendation: Add 15-20% buffer to infrastructure test estimates, or consider pairing infrastructure tests with integration tests to maximize learning-per-test-hour.
Please review the effort estimates with the team and confirm whether the schedule is realistic given local Docker/TestContainers issues flagged in the PR analysis.
3-11: Clarify baseline coverage discrepancy between CI (35.96%) and roadmap (45%).The PR objectives note that CI reports 35.96% coverage, while line 6 of the roadmap states ~45%. This ~9-point difference significantly affects all subsequent phase targets.
Possible explanations:
- CI coverage report filters out certain modules (e.g., migrations, generated code)
- Local analysis in roadmap includes modules CI excludes
- Coverage was improved between CI run and roadmap creation
Recommendation: Clarify the baseline and explicitly state which modules/exclusions account for the difference. For example:
- "Global: ~45% (including Shared + 7 domain modules, excluding Architecture/Integration tests)"
- "CI reported: 35.96% (different filter configuration—see config/coverage.runsettings line X)"
This context is crucial for stakeholders to understand whether phases are tracking toward actual CI-reported 35.96% or the local-analysis 45%.
427-444: No corruption detected in document; past issues appear resolved.The file has been verified at the previously flagged locations:
- Line 342: Clean — shows
- **Global**: ~53% (+8%)- Line 437: Clean — shows correct Portuguese
para cobertura por módulo(not corruptedtar)No orphaned special characters, encoding corruption, or markdown structure issues found. Non-ASCII characters present (em dashes, emojis, Portuguese diacritics) are valid UTF-8 text, not artifacts. File is ready.
- Standardize module headers with emoji indicators (⭐/🟡/🔴/✅) - Fix Portuguese: 'Suite' → 'Suíte', 'do base branch' → 'da branch base' - Add Phase 5 roadmap to reach 85% recommended coverage target - Document 18 developer-days effort across 6 workstreams - Address gap between 70% Sprint 2 goal and 85% industry standard - Detail ApiService (45%→70%), SearchProviders (65%→80%), and other modules - Include cross-module integration testing scenarios
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/testing/code-coverage-roadmap.md (1)
389-389: Minor language preference: "performance" → "desempenho".Line 389 uses "performance otimizada," which is common in technical Portuguese but the LanguageTool suggestion prefers the native Portuguese term "desempenho" for stylistic consistency.
- Radius search com performance otimizada + Radius search com desempenho otimizado
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/testing/code-coverage-roadmap.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
📚 Learning: 2025-11-25T16:00:52.775Z
Learnt from: CR
Repo: frigini/MeAjudaAi PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:00:52.775Z
Learning: Minimum test coverage: 70% (CI warning threshold), Recommended: 85%, Excellent: 90%+
Applied to files:
docs/testing/code-coverage-roadmap.md
🪛 LanguageTool
docs/testing/code-coverage-roadmap.md
[uncategorized] ~85-~85: Encontrada possível ausência de vírgula.
Context: ...es**: Entities, Value Objects, Commands/Queries handlers bem cobertos **Providers Modu...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~109-~109: Encontrada possível ausência de vírgula.
Context: ...diana - Gaps: CEP validation logic, ViaCEP API integration, geocoding **SearchPro...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~115-~115: Encontrada possível ausência de vírgula.
Context: ... 🔴 Abaixo da meta - Gaps Críticos: PostGIS geospatial queries, radius search, dist...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~219-~219: Encontrada possível ausência de vírgula.
Context: ...Users Domain** (65-75%) - Entities, Value Objects, Events - Status: ✅ Mód...
(AI_PT_HYDRA_LEO_MISSING_COMMA)
[locale-violation] ~389-~389: “performance” é um estrangeirismo. É preferível dizer “desempenho”, “atuação”, “apresentação”, “espetáculo” ou “interpretação”.
Context: ...ueries complexas - Radius search com performance otimizada - Distance calculations e ...
(PT_BARBARISMS_REPLACE_PERFORMANCE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: code-analysis
- GitHub Check: build-and-test
🔇 Additional comments (1)
docs/testing/code-coverage-roadmap.md (1)
1-510: Excellent comprehensive roadmap with past issues now resolved. ✅The corrupted text and duplicate projection entries flagged in past reviews have been successfully addressed. The document now presents a clear, well-structured coverage improvement plan with:
- Current state analysis (45% global) aligned with PR objectives
- Realistic 4-phase action plan (Fases 1–4) with estimates totaling ~8 weeks to reach 70%
- Prioritized gap analysis (🔴 Critical → 🟡 Medium → 🟢 Good) with justification
- Incremental projections showing progression through each phase
- KPIs, tracking metrics, and open issues documented
The roadmap directly addresses the root cause identified in PR context: most unit tests mock infrastructure, leaving repositories and handlers uncovered. Your phased approach (Database Layer → Messaging → Features → Polish) is sound.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.