test(store): add comprehensive tests for store data access layer#128
test(store): add comprehensive tests for store data access layer#128
Conversation
- Add token store tests: GetTokensByUserIDPaginated, RevokeToken, RevokeTokensByAuthorizationID, RevokeAllActiveTokensByClientID - Add client store tests: ListClientsByUserID, GetClientsByIDs, GetClientByIntID, CountActiveTokensByClientID, ListClientsPaginated - Add device code store tests: GetDeviceCodeByUserCode, UpdateDeviceCode, DeleteDeviceCodeByID - Add user authorization store tests: GetUserAuthorizationByUUID, ListUserAuthorizations, RevokeUserAuthorization, RevokeAllUserAuthorizationsByClientID, GetClientAuthorizations - Add audit log store tests: CreateAuditLog, CreateAuditLogBatch, GetAuditLogsPaginated with filters, DeleteOldAuditLogs, GetAuditLogStats - Add authorization code store tests: GetAuthorizationCodeByHash, MarkAuthorizationCodeUsed with concurrent safety - Add pagination types tests: NewPaginationParams, Offset, CalculatePagination - Coverage: store 63% → 82.6%, store/types 0% → 100% Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR adds a new suite of unit tests to improve coverage of the internal/store data-access layer and the internal/store/types helpers, aligning with the project’s test coverage improvement plan.
Changes:
- Adds new store-focused test files covering tokens, clients, device codes, audit logs, authorization codes, and user authorizations.
- Adds full coverage tests for
internal/store/typespagination helpers. - Exercises important behaviors like filtering/search, pagination metadata, and revoke/update flows.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/store/token_test.go | Adds tests for token pagination/search and revoke operations. |
| internal/store/client_test.go | Adds tests for client listing/search/status filters, lookup helpers, and active token counts. |
| internal/store/device_code_test.go | Adds tests for device-code CRUD and lookup by user code. |
| internal/store/user_authorization_test.go | Adds tests for user authorization retrieval, listing, and revocation flows. |
| internal/store/audit_log_test.go | Adds tests for audit log creation (single/batch), filtering, retention deletes, and stats. |
| internal/store/authorization_code_test.go | Adds tests for authorization code lookup and “mark used” behavior. |
| internal/store/types/pagination_test.go | Adds tests for pagination parameter normalization, offset, and metadata calculation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…check - Use gorm.ErrRecordNotFound instead of generic assert.Error in device_code and authorization_code not-found tests - Add GetAccessTokenByHash verification after RevokeToken to cover the production lookup path - Use require.ErrorIs for fatal error checks in token revocation test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove redundant GetTokensByCategoryAndStatus query by reusing the first result set for cross-client verification - Replace time.Sleep with explicit timestamp manipulation for deterministic ordering in ListClientsPaginated test - Fix assertion argument order for pagination.Total Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ile pollution seedData() writes authgate-credentials.txt to the working directory. Use t.Chdir(t.TempDir()) in createFreshStore so the file is written under a temp path and cleaned up after the test run. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- RevokeUserAuthorization test now verifies revocation persisted in DB via ListUserAuthorizations instead of checking the returned struct (GORM map-based Updates does not reflect back to the struct) - DeleteDeviceCodeByID test now asserts gorm.ErrRecordNotFound instead of generic assert.Error for consistency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Use gorm.ErrRecordNotFound for wrong-user assertions in GetUserAuthorizationByUUID and RevokeUserAuthorization tests - Replace hard-coded ApplicationID: 1 with actual persisted OAuthApplication IDs in authorization code tests - Remove unnecessary comments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The createTestClient helper was missing the ClientSecret field which has a NOT NULL constraint on the OAuthApplication model. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SQLite :memory: opens separate connections per goroutine, each seeing an empty database, making true concurrent MarkAuthorizationCodeUsed testing infeasible. The sequential double-call test already verifies the atomic WHERE used_at IS NULL guard. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
internal/storeandinternal/store/typespackages (Phase 2 of test coverage improvement plan)internal/store63% → 82.6%,internal/store/types0% → 100%Changes
internal/store/token_test.go— GetTokensByUserIDPaginated (search, pagination, isolation), RevokeToken, RevokeTokensByAuthorizationID, RevokeAllActiveTokensByClientIDinternal/store/client_test.go— ListClientsByUserID, GetClientsByIDs, GetClientByIntID, CountActiveTokensByClientID, ListClientsPaginated (search, status filter, pagination)internal/store/device_code_test.go— GetDeviceCodeByUserCode, UpdateDeviceCode, DeleteDeviceCodeByIDinternal/store/user_authorization_test.go— GetUserAuthorizationByUUID, ListUserAuthorizations, RevokeUserAuthorization, RevokeAllUserAuthorizationsByClientID, GetClientAuthorizationsinternal/store/audit_log_test.go— CreateAuditLog, CreateAuditLogBatch, GetAuditLogsPaginated (all filters), DeleteOldAuditLogs, GetAuditLogStatsinternal/store/authorization_code_test.go— GetAuthorizationCodeByHash, MarkAuthorizationCodeUsed (concurrent safety)internal/store/types/pagination_test.go— NewPaginationParams, Offset, CalculatePaginationTest plan
go test ./internal/store/... -short— all tests passmake lint— 0 issues🤖 Generated with Claude Code