test(8.1.15): comprehensive test coverage — 50+ tests, Mvc.Tests project, CI coverage#13
Conversation
INFRA-1: Reorganize Core.Tests into Unit/Integration/Security directories. Add WalkingTec.Mvvm.Mvc.Tests (new xUnit project). Add coverlet.collector to both new projects and Admin.Test. Register WtmTestHelper and TokenTestFixture fixtures. Update solution to include Mvc.Tests project. UNIT-1: Expand PasswordHashHelperTests (17 tests) and RefreshTokenEntityTests (8 tests) with edge cases — unicode, long passwords, timing, boundary expiry. AUTH-1 (P0): DoLoginAsyncTests (8 tests) — PBKDF2 happy path, MD5 migration, hash persistence to DB, disabled user, tenant mismatch. TokenServiceIntegrationTests (15 tests) — issue, refresh/rotate, reuse detection chain revocation, revoke, tenant isolation. ASYNC-VM-1 (P1): BaseCRUDVMAsyncTests (7 tests) — DoAddAsync/DoEditAsync/ DoDeleteAsync with audit field verification and field-subset update via FC dict. ADMIN-1 (P2): FrameworkGroupApiTest, FrameworkRoleApiTest, DataPrivilegeApiTest — Search/Create/Edit/Get/BatchDelete for each admin controller, following existing MSTest pattern. SEC-1: PasswordMigrationFlowTests (7 tests) — post-migration hash properties, salt uniqueness, cross-user isolation. BruteForceResistanceTests (7 tests) — PBKDF2 work-factor timing, 100-hash salt diversity. TokenChainSecurityTests (7 tests) — 3-hop chain, midpoint reuse, distinct tokens, cross-user revoke. CI-COV: Update ci-build.yml to collect coverage (XPlat Code Coverage + coverlet .runsettings), install ReportGenerator, generate HTML/lcov/Cobertura reports, upload coverage-report artifact alongside existing test-results artifact. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f98c86ec5e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mockHttpContext.Setup(x => x.User) | ||
| .Returns(new System.Security.Claims.ClaimsPrincipal()); |
There was a problem hiding this comment.
Seed mock user with a non-null identity
CreateLoginTestContext sets HttpContext.User to new ClaimsPrincipal() with no identities, and WTMContext.DoLoginAsync later reads HttpContext.User.Identity.IsAuthenticated when tenant is null (see src/WalkingTec.Mvvm.Core/WTMContext.cs). An empty principal has a null Identity, so the new login integration tests that pass tenant: null can throw NullReferenceException before authentication logic executes.
Useful? React with 👍 / 👎.
| refreshed.AccessToken.Should().NotBe(issued.AccessToken, | ||
| "access token must also be regenerated on rotation"); |
There was a problem hiding this comment.
Remove flaky access-token inequality check
This assertion assumes refresh must always change the access-token string, but TokenService.GenerateAccessToken is deterministic for the same claims/signing key and uses second-level JWT timestamps; when issue+refresh happen within the same second (common in fast CI), issued.AccessToken and refreshed.AccessToken can be identical even though refresh-token rotation succeeded, making this test intermittently fail.
Useful? React with 👍 / 👎.
Core.Tests: FrameworkUserBase is abstract; define concrete TestLoginUser in Core.Tests fixtures to avoid demo project dependency. Replace all FrameworkUser references with TestLoginUser throughout WtmTestHelper and DoLoginAsyncTests. LoginTestDataContext now uses DbSet<TestLoginUser>. Admin.Test/FrameworkGroupApiTest: FrameworkGroupController.Add() and Edit() return sync IActionResult (not Task<IActionResult>); remove erroneous .Result dereference. FrameworkGroup inherits TreePoco:TopBasePoco (no audit fields); remove CreateBy/CreateTime/UpdateBy/UpdateTime assertions. Admin.Test/FrameworkRoleApiTest: FrameworkRoleController.Add() and Edit() also return sync IActionResult; remove .Result dereference. FrameworkRole inherits BasePoco so audit field assertions are retained. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add `using WalkingTec.Mvvm.Core.Support.Json` for Token type - Replace ReturnsAsync() with Returns(Task.FromResult()) to avoid nullable context ambiguity in projects with <Nullable>enable</> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…r conflict - WtmTestHelper: use ClaimsPrincipal(ClaimsIdentity()) so HttpContext.User.Identity is never null (DoLoginAsync line 375 null-check) - TokenTestFixture: replace AddDbContext<EmptyContext>(opt=>.UseInMemory()) with AddScoped<EmptyContext>(_ => new EmptyContext(DbName, DBTypeEnum.Memory)) to prevent OnConfiguring from adding SqlServer alongside InMemory in the same service provider Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… for TokenFixture DoLoginAsync: LoginTestDataContext now extends EmptyContext instead of FrameworkContext to avoid complex navigation relationships (FrameworkUserRoles, FrameworkUserGroups) that EF Core InMemory cannot translate in LINQ projection expressions. TokenTestFixture: use FrameworkContext (which has FrameworkRefreshTokens) instead of EmptyContext so db.Set<RefreshTokenEntity>() can resolve in the EF model. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
DoLoginAsyncTests: replace EF InMemory with SQLite shared in-memory database. LoadBasicInfoAsync has correlated collection subqueries (FrameworkUserRole, FrameworkUserGroup) that EF Core InMemory cannot translate. SQLite handles these correctly. A keep-alive SqliteConnection ensures the named in-memory DB persists across multiple LoginTestDataContext instances within one test. LoginTestDataContext: back to FrameworkContext (all entities in model) + SQLite via DataSource=seed?mode=memory&cache=shared connection string. TokenService.GenerateAccessToken: add jti (JWT ID) claim so consecutive tokens for the same user are always distinct strings even within the same second. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Core.TestsintoUnit/Integration/Securitydirectories; add newWalkingTec.Mvvm.Mvc.TestsxUnit project; addcoverlet.collectorto all 3 test projects; createWtmTestHelperandTokenTestFixturefixturesPasswordHashHelperTests(17 tests) andRefreshTokenEntityTests(8 tests) with unicode, long passwords, timing boundariesDoLoginAsyncTests(8 tests) covering PBKDF2, MD5 migration with DB persistence, disabled user, tenant mismatch;TokenServiceIntegrationTests(15 tests) covering issue/refresh/revoke lifecycle, reuse detection chain revocation, tenant isolationBaseCRUDVMAsyncTests(7 tests) forDoAddAsync/DoEditAsync/DoDeleteAsyncwith audit field verification and FC field-subset updateFrameworkGroupApiTest,FrameworkRoleApiTest,DataPrivilegeApiTest(13 MSTest tests) — Search/Create/Edit/Get/BatchDelete for each admin controllerPasswordMigrationFlowTests(7 tests),BruteForceResistanceTests(7 tests),TokenChainSecurityTests(7 tests) — security property verification including PBKDF2 work-factor timing, 100-hash salt uniqueness, 3-hop chain integritycoverlet.runsettings, update CI to collectXPlat Code Coverage, install ReportGenerator, upload HTML/lcov/Cobertura coverage report artifactTest plan
feature/8.1.15-testingbranch🤖 Generated with Claude Code