test: enhance coverage and stabilize infrastructure#1240
Open
cesarcastrocuba wants to merge 10 commits intofullstackhero:developfrom
Open
test: enhance coverage and stabilize infrastructure#1240cesarcastrocuba wants to merge 10 commits intofullstackhero:developfrom
cesarcastrocuba wants to merge 10 commits intofullstackhero:developfrom
Conversation
added 10 commits
May 9, 2026 20:26
## Build infrastructure
- Directory.Build.props: suppress MSG0005 globally (Mediator domain events
without registered handlers — intentional, handlers wired up at runtime)
- src/.editorconfig: suppress CA1054, CA1056 (URI string params are
intentional API design), MSG0005 (redundant after Build.props change)
## BuildingBlocks — Quota
- InMemoryQuotaService, RedisQuotaService: change _gauges field type from
IReadOnlyDictionary to Dictionary (CA1859 — use concrete type for perf)
- InMemoryQuotaStore: add SuppressMessage CA1812 — class is instantiated
by the DI container, invisible to static analysis
- NoopQuotaService: same CA1812 suppression for DI-instantiated class
- QuotaOptions.Plans: change { get; set; } to { get; } — collection is
mutated in-place; the setter was never used (CA2227)
- Quota.csproj: remove redundant Microsoft.Extensions.* package references
that are already transitively pulled by other dependencies
## BuildingBlocks — Mailing
- SmtpMailService: guard AuthenticateAsync call — only authenticate when
both UserName and Password are present, avoiding null argument exceptions
and supporting anonymous SMTP relays (CS8604 null safety fix)
## BuildingBlocks — Storage
- Storage.csproj: remove BOM character from file header and remove
redundant Microsoft.Extensions.Logging.Abstractions reference
## BuildingBlocks — Web
- Versioning/Extensions.cs: add ArgumentNullException.ThrowIfNull(services)
to fix CA1062 (validate non-nullable parameters in public methods)
## Modules — Auditing
- AuditingModule.cs: add ArgumentNullException.ThrowIfNull(endpoints)
to fix CA1062
## Modules — Multitenancy
- MultitenancyModule.cs: add ArgumentNullException.ThrowIfNull(endpoints)
to fix CA1062
- AppTenantInfoConfiguration.cs: add ArgumentNullException.ThrowIfNull(builder)
to fix CA1062
## Modules — Billing
- BillingService.cs: wrap LogInformation calls in IsEnabled(Information)
guards to fix CA1873 (avoid potentially expensive logging)
- MonthlyInvoiceJob.cs: same CA1873 fix for both log calls
- UsageReporter.cs: same CA1873 fix
## Modules — Catalog (Contracts)
- SearchBrandsQuery.cs: replace misplaced XML /// comments inside record
positional parameters with standard // comments (CS1587/CS1573)
- SearchCategoriesQuery.cs: same CS1587/CS1573 fix
- SearchProductsQuery.cs: same CS1587/CS1573 fix
## Modules — Catalog (Implementation)
- CatalogDbInitializer.cs: wrap LogInformation in IsEnabled guard (CA1873)
- ListTrashedBrandsQueryHandler.cs: rewrite comment to remove S125
false positive (comment contained brackets that Sonar read as code)
- SearchBrandsQueryHandler.cs: replace ToLowerInvariant with ToUpperInvariant
and update switch labels to fix CA1308 (prefer upper-case normalization)
- SearchCategoriesQueryHandler.cs: same CA1308 fix
- SearchProductsQueryHandler.cs: same CA1308 fix
- GetCategoryTreeQueryHandler.cs: replace GroupBy+ToDictionary with ToLookup
which handles null keys natively, removing the TryGetValue null guard
## Modules — Identity (Contracts)
- IIdentityService.cs: fix XML doc param name (tenant -> twoFactorCode)
and remove stray BOM character
- ISessionService.cs: add missing cancellationToken XML doc param tag
## Modules — Identity (Implementation)
- RolePermissionSyncHostedService.cs: wrap LogDebug in IsEnabled guard
(CA1873)
- RolePermissionSyncer.cs: wrap LogInformation in IsEnabled guard (CA1873)
- GetTenantSessionsValidator.cs: add missing FluentValidation validator for
GetTenantSessionsQuery to satisfy architecture rule requiring all paginated
queries to have a validator (PageNumber >= 1, PageSize >= 1)
## Modules — Tickets
- SearchTicketsQueryHandler.cs: replace ToLowerInvariant with ToUpperInvariant
in sort switch to fix CA1308
- TicketComment.cs: restore private set on ISoftDeletable properties with
#pragma suppress for S1144 — EF Core writes these via SaveChanges
interceptor (entry.Property(...).CurrentValue) which bypasses C# setters
and is invisible to static analysis; removing the setter would break
domain model consistency with other ISoftDeletable entities
## Modules — Webhooks
- WebhookDispatchJob.cs: wrap logging calls in IsEnabled guards (CA1873);
add parameterless constructor to WebhookDeliveryFailedException (CA1032 —
custom exceptions should provide all four standard constructors)
- WebhookDispatchJobTests.cs: wrap DispatchAsync call in Should.NotThrowAsync
to correctly assert the no-throw expectation
## Result
Build succeeded with 0 warnings, 0 errors across all 37 projects.
Comprehensive update to the test suite focusing on architecture integrity, Identity module logic, and integration test stability. Key improvements: 1. Architecture Hardening: - Refactored HandlerValidatorPairingTests and DomainEntityTests to use strict assertions (ShouldBeEmpty). Previously, these tests were using ShouldNotBeNull on violation lists, which silently passed even if violations were found. - Implemented ModuleAssemblyDiscovery helper to dynamically find and scan business modules (FSH.Modules.*), eliminating hardcoded assembly lists and ensuring automatic coverage for new modules. 2. Identity Module Coverage Expansion: - Added unit tests for 7 core command handlers: DeleteUser, UpdateUser, ToggleUserStatus, ForgotPassword, ResetPassword, ConfirmEmail, and UpsertRole. - Added unit tests for 4 core validators: DeleteUser, UpdateUser, ForgotPassword, and ResetPassword. - Replaced system UnauthorizedAccessException with project-standard UnauthorizedException in RefreshTokenCommandHandler and updated corresponding tests. 3. Integration Test Stabilization: - Refactored FshWebApplicationFactory to use deterministic migration and seeding. Introduced SemaphoreSlim to prevent race conditions during parallel container initialization. - Disabled conflicting background services (TenantStoreInitializer, RolePermissionSync) during integration tests to prevent database deadlocks and duplicate key errors. - Verified end-to-end stability for EmailConfirmation and RefreshTokenRevocation scenarios. 4. Auditing Test Fixes: - Fixed 3 pre-existing failures in JsonMaskingServiceTests in Auditing.Tests. These failures were caused by a recent optimization where the service returns the original object if no masking is needed, which the tests weren't accounting for. 5. Code Quality: - Resolved CA2201 warnings by replacing generic Exception usage with descriptive Shouldly assertions across the test projects. - Maintained 0 warnings in all newly created and modified files. Full suite results: 700 tests (699 passed, 1 skipped).
…ing base codebase
GitHub Advanced Security flagged LogInformation passing SharedPassword directly as an argument (high-severity alert). Replaced with a neutral completion message that confirms seeding succeeded without exposing the credential value to any log sink or aggregator.
Contributor
|
Great PR! Please @iammukeshm merge the pull request before it becomes outdated again as we continue building. |
|
Hi @iammukeshm, I’m trying to understand the contribution process for the project. I noticed that several well-documented pull requests, including contributions from previous developers, were not accepted. Some of those contributors also seem to have stopped contributing afterward. Since detailed documentation and improvements are being prepared for each contribution, accepting community input could help the project grow faster and become more stable over time. Could you please share the reasoning behind the current contribution approach? I believe more transparency on this would help contributors better understand the expectations and project direction. Thank you. |
|
Hi @cesarcastrocuba, I just wanted to thank you for your contributions and the effort you’ve put into the project. Your work and documentation are really valuable and helpful for the community. I hope you continue contributing to the project. Having active developers like you makes a big difference and helps the project improve much faster. Thank you again for your time and effort. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Comprehensive update to the test suite focusing on architecture integrity, Identity module logic, and integration test stability.
Key improvements:
Architecture Hardening:
Identity Module Coverage Expansion:
Integration Test Stabilization:
Auditing Test Fixes:
Code Quality:
Full suite results: 700 tests (699 passed, 1 skipped).