test: add SearchCurrentUserPATs handler and PAT List service tests#1554
test: add SearchCurrentUserPATs handler and PAT List service tests#1554
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 38 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds comprehensive test coverage for user PAT (Personal Access Token) functionality at the service and handler layers. New tests cover configuration validation, error handling for repository and policy failures, and successful PAT listing with scope enrichment. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
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.
🧹 Nitpick comments (1)
internal/api/v1beta1connect/user_pat_test.go (1)
1336-1372: Strengthen response assertions beyond id/title/total.The test case builds a fully populated
wantresponse (scopes, offset, limit, timestamps) but the loop at lines 1454-1459 only assertsId,Title, andPagination.TotalCount. Fields likeOffset,Limit,ExpiresAt,CreatedAt,UpdatedAt, andScopesare never verified, so regressions intransformPATToPBor pagination mapping inSearchCurrentUserPATswould slip past this test. Consider a singleassert.Equal(t, tt.want, resp.Msg)(as done inTestHandler_CreateCurrentUserPAT) to make the expected fixture meaningful.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 07cb3667-d61c-4db4-8652-febab43dcda0
📒 Files selected for processing (2)
core/userpat/service_test.gointernal/api/v1beta1connect/user_pat_test.go
Coverage Report for CI Build 24653871743Coverage increased (+0.2%) to 42.258%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
SearchCurrentUserPATshandler (5 cases)userpat.Service.Listmethod (4 cases)Handler tests (
TestHandler_SearchCurrentUserPATs)GetLoggedInPrincipalfailsResolveSubject()and lists PATs correctlyFailedPreconditionwhen PAT feature is disabledInternalerror for unknown service failuresService tests (
TestService_List)ErrDisabledwhen PAT feature is disabledenrichWithScopefails (policy service error)Test plan
go build ./...passes🤖 Generated with Claude Code