fix(rpc): build against the api with merge request notes and reject scope mr:review
The api now has the note methods of MergeRequestService and SCOPE_MR_REVIEW. The note methods answer unimplemented until the notes work lands, and the scope converter rejects mr:review like any unknown scope.
- commit
- 30f7bea2577e07ba505739d71eed27c841e7096b
- author
- brice · · 2026-10-19 08:36 UTC
- parent
- 68460c1
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +30 -17
M internal/adapters/client/scopes.go +5 -3
| @@ -35,12 +35,14 @@ func ScopeName(s ermite.Scope) string { | |||
| 35 | 35 | return strings.Replace(strings.ToLower(strings.TrimPrefix(name, "SCOPE_")), "_", ":", 1) | |
| 36 | 36 | } | |
| 37 | 37 | ||
| 38 | - | // AllScopes lists every scope the contract defines, in enum order. Deriving it | |
| 39 | - | // keeps the `cli` preset ("every scope") true when the contract grows. | |
| 38 | + | // AllScopes lists every scope the contract defines that this server accepts, in | |
| 39 | + | // enum order. Deriving it keeps the `cli` preset ("every scope") true when the | |
| 40 | + | // contract grows. SCOPE_MR_REVIEW is in the contract but not yet implemented | |
| 41 | + | // here, and the server rejects it like any unknown scope, so it is left out. | |
| 40 | 42 | func AllScopes() []ermite.Scope { | |
| 41 | 43 | var scopes []ermite.Scope | |
| 42 | 44 | for n := range ermite.Scope_name { | |
| 43 | - | if s := ermite.Scope(n); s != ermite.Scope_SCOPE_UNSPECIFIED { | |
| 45 | + | if s := ermite.Scope(n); s != ermite.Scope_SCOPE_UNSPECIFIED && s != ermite.Scope_SCOPE_MR_REVIEW { | |
| 44 | 46 | scopes = append(scopes, s) | |
| 45 | 47 | } | |
| 46 | 48 | } | |
| @@ -35,12 +35,14 @@ func ScopeName(s ermite.Scope) string { | |||||
| 35 | return strings.Replace(strings.ToLower(strings.TrimPrefix(name, "SCOPE_")), "_", ":", 1) | 35 | return strings.Replace(strings.ToLower(strings.TrimPrefix(name, "SCOPE_")), "_", ":", 1) | ||
| 36 | } | 36 | } | ||
| 37 | 37 | ||||
| 38 | - | // AllScopes lists every scope the contract defines, in enum order. Deriving it | 38 | + | // AllScopes lists every scope the contract defines that this server accepts, in |
| 39 | - | // keeps the `cli` preset ("every scope") true when the contract grows. | 39 | + | // enum order. Deriving it keeps the `cli` preset ("every scope") true when the |
| 40 | + | // contract grows. SCOPE_MR_REVIEW is in the contract but not yet implemented | |||
| 41 | + | // here, and the server rejects it like any unknown scope, so it is left out. | |||
| 40 | func AllScopes() []ermite.Scope { | 42 | func AllScopes() []ermite.Scope { | ||
| 41 | var scopes []ermite.Scope | 43 | var scopes []ermite.Scope | ||
| 42 | for n := range ermite.Scope_name { | 44 | for n := range ermite.Scope_name { | ||
| 43 | - | if s := ermite.Scope(n); s != ermite.Scope_SCOPE_UNSPECIFIED { | 45 | + | if s := ermite.Scope(n); s != ermite.Scope_SCOPE_UNSPECIFIED && s != ermite.Scope_SCOPE_MR_REVIEW { |
| 44 | scopes = append(scopes, s) | 46 | scopes = append(scopes, s) | ||
| 45 | } | 47 | } | ||
| 46 | } | 48 | } | ||
M internal/adapters/rpc/api.go +1 -1
| @@ -143,7 +143,7 @@ func New(cfg Config) *API { | |||
| 143 | 143 | features = append(features, FeatureCommitSignatures) | |
| 144 | 144 | } | |
| 145 | 145 | mux.Handle(ermiteconnect.NewServerServiceHandler(&serverService{version: cfg.Version, features: features, key: cfg.SigningKey, oidc: cfg.OIDCKeys}, opts...)) | |
| 146 | - | mux.Handle(ermiteconnect.NewMergeRequestServiceHandler(&mergeRequestService{cfg.MergeRequests}, opts...)) | |
| 146 | + | mux.Handle(ermiteconnect.NewMergeRequestServiceHandler(&mergeRequestService{mrs: cfg.MergeRequests}, opts...)) | |
| 147 | 147 | mux.Handle(ermiteconnect.NewIssueServiceHandler(&issueService{cfg.Issues}, opts...)) | |
| 148 | 148 | if cfg.Sessions != nil { | |
| 149 | 149 | mux.Handle(ermiteconnect.NewSessionServiceHandler(&sessionService{sessions: cfg.Sessions, passkeys: cfg.Passkeys}, opts...)) | |
| @@ -143,7 +143,7 @@ func New(cfg Config) *API { | |||||
| 143 | features = append(features, FeatureCommitSignatures) | 143 | features = append(features, FeatureCommitSignatures) | ||
| 144 | } | 144 | } | ||
| 145 | mux.Handle(ermiteconnect.NewServerServiceHandler(&serverService{version: cfg.Version, features: features, key: cfg.SigningKey, oidc: cfg.OIDCKeys}, opts...)) | 145 | mux.Handle(ermiteconnect.NewServerServiceHandler(&serverService{version: cfg.Version, features: features, key: cfg.SigningKey, oidc: cfg.OIDCKeys}, opts...)) | ||
| 146 | - | mux.Handle(ermiteconnect.NewMergeRequestServiceHandler(&mergeRequestService{cfg.MergeRequests}, opts...)) | 146 | + | mux.Handle(ermiteconnect.NewMergeRequestServiceHandler(&mergeRequestService{mrs: cfg.MergeRequests}, opts...)) |
| 147 | mux.Handle(ermiteconnect.NewIssueServiceHandler(&issueService{cfg.Issues}, opts...)) | 147 | mux.Handle(ermiteconnect.NewIssueServiceHandler(&issueService{cfg.Issues}, opts...)) | ||
| 148 | if cfg.Sessions != nil { | 148 | if cfg.Sessions != nil { | ||
| 149 | mux.Handle(ermiteconnect.NewSessionServiceHandler(&sessionService{sessions: cfg.Sessions, passkeys: cfg.Passkeys}, opts...)) | 149 | mux.Handle(ermiteconnect.NewSessionServiceHandler(&sessionService{sessions: cfg.Sessions, passkeys: cfg.Passkeys}, opts...)) | ||
M internal/adapters/rpc/convert_test.go +6 -1
| @@ -50,12 +50,17 @@ func TestEnumMappersCoverEveryDomainValue(t *testing.T) { | |||
| 50 | 50 | } | |
| 51 | 51 | _, err := scopeFromPB(pb.Scope_SCOPE_UNSPECIFIED) | |
| 52 | 52 | assert.ErrorIs(t, err, domain.ErrInvalidScope) | |
| 53 | - | // Every scope of the contract is a scope of the domain. | |
| 53 | + | // Every scope of the contract is a scope of the domain, except mr:review. | |
| 54 | 54 | for v := range pb.Scope_name { | |
| 55 | 55 | if pb.Scope(v) == pb.Scope_SCOPE_UNSPECIFIED { | |
| 56 | 56 | continue | |
| 57 | 57 | } | |
| 58 | 58 | _, err := scopeFromPB(pb.Scope(v)) | |
| 59 | + | if pb.Scope(v) == pb.Scope_SCOPE_MR_REVIEW { | |
| 60 | + | // In the contract, not implemented here yet: rejected like an unknown scope. | |
| 61 | + | assert.ErrorIs(t, err, domain.ErrInvalidScope, pb.Scope(v)) | |
| 62 | + | continue | |
| 63 | + | } | |
| 59 | 64 | assert.NoError(t, err, pb.Scope(v)) | |
| 60 | 65 | } | |
| 61 | 66 | }) | |
| @@ -50,12 +50,17 @@ func TestEnumMappersCoverEveryDomainValue(t *testing.T) { | |||||
| 50 | } | 50 | } | ||
| 51 | _, err := scopeFromPB(pb.Scope_SCOPE_UNSPECIFIED) | 51 | _, err := scopeFromPB(pb.Scope_SCOPE_UNSPECIFIED) | ||
| 52 | assert.ErrorIs(t, err, domain.ErrInvalidScope) | 52 | assert.ErrorIs(t, err, domain.ErrInvalidScope) | ||
| 53 | - | // Every scope of the contract is a scope of the domain. | 53 | + | // Every scope of the contract is a scope of the domain, except mr:review. |
| 54 | for v := range pb.Scope_name { | 54 | for v := range pb.Scope_name { | ||
| 55 | if pb.Scope(v) == pb.Scope_SCOPE_UNSPECIFIED { | 55 | if pb.Scope(v) == pb.Scope_SCOPE_UNSPECIFIED { | ||
| 56 | continue | 56 | continue | ||
| 57 | } | 57 | } | ||
| 58 | _, err := scopeFromPB(pb.Scope(v)) | 58 | _, err := scopeFromPB(pb.Scope(v)) | ||
| 59 | + | if pb.Scope(v) == pb.Scope_SCOPE_MR_REVIEW { | |||
| 60 | + | // In the contract, not implemented here yet: rejected like an unknown scope. | |||
| 61 | + | assert.ErrorIs(t, err, domain.ErrInvalidScope, pb.Scope(v)) | |||
| 62 | + | continue | |||
| 63 | + | } | |||
| 59 | assert.NoError(t, err, pb.Scope(v)) | 64 | assert.NoError(t, err, pb.Scope(v)) | ||
| 60 | } | 65 | } | ||
| 61 | }) | 66 | }) | ||
M internal/adapters/rpc/mergerequest.go +7 -1
| @@ -10,12 +10,18 @@ import ( | |||
| 10 | 10 | "connectrpc.com/connect" | |
| 11 | 11 | ||
| 12 | 12 | pb "bnema.dev/ermite-api/gen/ermite" | |
| 13 | + | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | |
| 13 | 14 | "bnema.dev/ermite/internal/domain" | |
| 14 | 15 | "bnema.dev/ermite/internal/ports" | |
| 15 | 16 | ) | |
| 16 | 17 | ||
| 17 | 18 | // mergeRequestService implements ermiteconnect.MergeRequestServiceHandler. | |
| 18 | - | type mergeRequestService struct{ mrs ports.MergeRequestService } | |
| 19 | + | type mergeRequestService struct { | |
| 20 | + | // The note methods (edit, delete, verdicts) are implemented by the merge | |
| 21 | + | // request notes work; until then they answer CodeUnimplemented. | |
| 22 | + | ermiteconnect.UnimplementedMergeRequestServiceHandler | |
| 23 | + | mrs ports.MergeRequestService | |
| 24 | + | } | |
| 19 | 25 | ||
| 20 | 26 | func mergeRequestStateToPB(s domain.MRState) pb.MergeRequestState { | |
| 21 | 27 | switch s { | |
| @@ -10,12 +10,18 @@ import ( | |||||
| 10 | "connectrpc.com/connect" | 10 | "connectrpc.com/connect" | ||
| 11 | 11 | ||||
| 12 | pb "bnema.dev/ermite-api/gen/ermite" | 12 | pb "bnema.dev/ermite-api/gen/ermite" | ||
| 13 | + | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | |||
| 13 | "bnema.dev/ermite/internal/domain" | 14 | "bnema.dev/ermite/internal/domain" | ||
| 14 | "bnema.dev/ermite/internal/ports" | 15 | "bnema.dev/ermite/internal/ports" | ||
| 15 | ) | 16 | ) | ||
| 16 | 17 | ||||
| 17 | // mergeRequestService implements ermiteconnect.MergeRequestServiceHandler. | 18 | // mergeRequestService implements ermiteconnect.MergeRequestServiceHandler. | ||
| 18 | - | type mergeRequestService struct{ mrs ports.MergeRequestService } | 19 | + | type mergeRequestService struct { |
| 20 | + | // The note methods (edit, delete, verdicts) are implemented by the merge | |||
| 21 | + | // request notes work; until then they answer CodeUnimplemented. | |||
| 22 | + | ermiteconnect.UnimplementedMergeRequestServiceHandler | |||
| 23 | + | mrs ports.MergeRequestService | |||
| 24 | + | } | |||
| 19 | 25 | ||||
| 20 | func mergeRequestStateToPB(s domain.MRState) pb.MergeRequestState { | 26 | func mergeRequestStateToPB(s domain.MRState) pb.MergeRequestState { | ||
| 21 | switch s { | 27 | switch s { | ||
M internal/adapters/rpc/mergerequest_test.go +11 -11
| @@ -41,7 +41,7 @@ func TestMergeRequestServiceCreate(t *testing.T) { | |||
| 41 | 41 | Title: "t", Description: "d", Source: "feat", Target: "main", | |
| 42 | 42 | }).Return(domain.MergeRequest{Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen}, tt.usecase).Once() | |
| 43 | 43 | } | |
| 44 | - | resp, err := (&mergeRequestService{mrs}).CreateMergeRequest(ownerCtx(), connect.NewRequest(&pb.CreateMergeRequestRequest{ | |
| 44 | + | resp, err := (&mergeRequestService{mrs: mrs}).CreateMergeRequest(ownerCtx(), connect.NewRequest(&pb.CreateMergeRequestRequest{ | |
| 45 | 45 | Repo: tt.repo, Title: "t", Description: "d", SourceBranch: "feat", TargetBranch: "main", | |
| 46 | 46 | })) | |
| 47 | 47 | if tt.wantCode != 0 { | |
| @@ -57,7 +57,7 @@ func TestMergeRequestServiceCreate(t *testing.T) { | |||
| 57 | 57 | ||
| 58 | 58 | func TestMergeRequestServiceListRejectsUnknownState(t *testing.T) { | |
| 59 | 59 | t.Parallel() | |
| 60 | - | _, err := (&mergeRequestService{mocks.NewMergeRequestService(t)}).ListMergeRequests(ownerCtx(), | |
| 60 | + | _, err := (&mergeRequestService{mrs: mocks.NewMergeRequestService(t)}).ListMergeRequests(ownerCtx(), | |
| 61 | 61 | connect.NewRequest(&pb.ListMergeRequestsRequest{Repo: "demo", State: pb.MergeRequestState(99)})) | |
| 62 | 62 | requireCode(t, connect.CodeInvalidArgument, err) | |
| 63 | 63 | } | |
| @@ -68,7 +68,7 @@ func TestMergeRequestServiceListFiltersByState(t *testing.T) { | |||
| 68 | 68 | mrs.EXPECT().List(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), ports.ListMergeRequestsParams{ | |
| 69 | 69 | State: domain.MRStateMerged, PageSize: 3, PageToken: "tok", | |
| 70 | 70 | }).Return(ports.MergeRequestPage{MergeRequests: []domain.MergeRequest{{Number: 1}}, NextPageToken: "n"}, nil).Once() | |
| 71 | - | resp, err := (&mergeRequestService{mrs}).ListMergeRequests(ownerCtx(), connect.NewRequest(&pb.ListMergeRequestsRequest{ | |
| 71 | + | resp, err := (&mergeRequestService{mrs: mrs}).ListMergeRequests(ownerCtx(), connect.NewRequest(&pb.ListMergeRequestsRequest{ | |
| 72 | 72 | Repo: "demo", State: pb.MergeRequestState_MERGE_REQUEST_STATE_MERGED, PageSize: 3, PageToken: "tok", | |
| 73 | 73 | })) | |
| 74 | 74 | require.NoError(t, err) | |
| @@ -99,7 +99,7 @@ func TestMergeRequestServiceAddComment(t *testing.T) { | |||
| 99 | 99 | mrs.EXPECT().AddComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(2), mock.Anything). | |
| 100 | 100 | Return(domain.Comment{ID: 5}, tt.usecase).Once() | |
| 101 | 101 | } | |
| 102 | - | resp, err := (&mergeRequestService{mrs}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{ | |
| 102 | + | resp, err := (&mergeRequestService{mrs: mrs}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{ | |
| 103 | 103 | Repo: "demo", Number: 2, Path: "f.go", Line: 3, Side: tt.side, Text: "hi", | |
| 104 | 104 | })) | |
| 105 | 105 | if tt.wantCode != 0 { | |
| @@ -120,7 +120,7 @@ func TestMergeRequestServiceResolveCommentMapsUnissuedIDsToZero(t *testing.T) { | |||
| 120 | 120 | mrs := mocks.NewMergeRequestService(t) | |
| 121 | 121 | mrs.EXPECT().ResolveComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), int64(0), true). | |
| 122 | 122 | Return(domain.Comment{}, domain.ErrNotFound).Once() | |
| 123 | - | _, err := (&mergeRequestService{mrs}).ResolveComment(ownerCtx(), connect.NewRequest(&pb.ResolveCommentRequest{ | |
| 123 | + | _, err := (&mergeRequestService{mrs: mrs}).ResolveComment(ownerCtx(), connect.NewRequest(&pb.ResolveCommentRequest{ | |
| 124 | 124 | Repo: "demo", Number: 1, CommentId: id, Resolved: true, | |
| 125 | 125 | })) | |
| 126 | 126 | requireCode(t, connect.CodeNotFound, err) | |
| @@ -154,7 +154,7 @@ func TestMergeRequestServiceMerge(t *testing.T) { | |||
| 154 | 154 | return p.DeleteSource == tt.req.GetDeleteSourceBranch() && (tt.req.GetHeadId() == "") == p.HeadID.IsZero() | |
| 155 | 155 | })).Return(ports.MergeResult{MergeRequest: domain.MergeRequest{Number: 1}, SourceDeleted: true}, tt.usecase).Once() | |
| 156 | 156 | } | |
| 157 | - | resp, err := (&mergeRequestService{mrs}).MergeMergeRequest(ownerCtx(), connect.NewRequest(tt.req)) | |
| 157 | + | resp, err := (&mergeRequestService{mrs: mrs}).MergeMergeRequest(ownerCtx(), connect.NewRequest(tt.req)) | |
| 158 | 158 | if tt.wantCode != 0 { | |
| 159 | 159 | requireCode(t, tt.wantCode, err) | |
| 160 | 160 | return | |
| @@ -171,20 +171,20 @@ func TestMergeRequestServiceCloseAndMarkRead(t *testing.T) { | |||
| 171 | 171 | t.Parallel() | |
| 172 | 172 | mrs := mocks.NewMergeRequestService(t) | |
| 173 | 173 | mrs.EXPECT().Close(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(9)).Return(domain.MergeRequest{}, domain.ErrNotFound).Once() | |
| 174 | - | _, err := (&mergeRequestService{mrs}).CloseMergeRequest(ownerCtx(), connect.NewRequest(&pb.CloseMergeRequestRequest{Repo: "demo", Number: 9})) | |
| 174 | + | _, err := (&mergeRequestService{mrs: mrs}).CloseMergeRequest(ownerCtx(), connect.NewRequest(&pb.CloseMergeRequestRequest{Repo: "demo", Number: 9})) | |
| 175 | 175 | requireCode(t, connect.CodeNotFound, err) | |
| 176 | 176 | }) | |
| 177 | 177 | t.Run("close ok", func(t *testing.T) { | |
| 178 | 178 | t.Parallel() | |
| 179 | 179 | mrs := mocks.NewMergeRequestService(t) | |
| 180 | 180 | mrs.EXPECT().Close(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(9)).Return(domain.MergeRequest{Number: 9, State: domain.MRStateClosed}, nil).Once() | |
| 181 | - | resp, err := (&mergeRequestService{mrs}).CloseMergeRequest(ownerCtx(), connect.NewRequest(&pb.CloseMergeRequestRequest{Repo: "demo", Number: 9})) | |
| 181 | + | resp, err := (&mergeRequestService{mrs: mrs}).CloseMergeRequest(ownerCtx(), connect.NewRequest(&pb.CloseMergeRequestRequest{Repo: "demo", Number: 9})) | |
| 182 | 182 | require.NoError(t, err) | |
| 183 | 183 | assert.Equal(t, pb.MergeRequestState_MERGE_REQUEST_STATE_CLOSED, resp.Msg.GetMergeRequest().GetState()) | |
| 184 | 184 | }) | |
| 185 | 185 | t.Run("mark read rejects a short head", func(t *testing.T) { | |
| 186 | 186 | t.Parallel() | |
| 187 | - | _, err := (&mergeRequestService{mocks.NewMergeRequestService(t)}).MarkRead(ownerCtx(), | |
| 187 | + | _, err := (&mergeRequestService{mrs: mocks.NewMergeRequestService(t)}).MarkRead(ownerCtx(), | |
| 188 | 188 | connect.NewRequest(&pb.MarkReadRequest{Repo: "demo", Number: 1, HeadId: "abc"})) | |
| 189 | 189 | requireCode(t, connect.CodeInvalidArgument, err) | |
| 190 | 190 | }) | |
| @@ -192,7 +192,7 @@ func TestMergeRequestServiceCloseAndMarkRead(t *testing.T) { | |||
| 192 | 192 | t.Parallel() | |
| 193 | 193 | mrs := mocks.NewMergeRequestService(t) | |
| 194 | 194 | mrs.EXPECT().MarkRead(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), domain.ObjectID{}).Return(domain.ObjectID{}, domain.ErrUnauthenticated).Once() | |
| 195 | - | _, err := (&mergeRequestService{mrs}).MarkRead(ownerCtx(), connect.NewRequest(&pb.MarkReadRequest{Repo: "demo", Number: 1})) | |
| 195 | + | _, err := (&mergeRequestService{mrs: mrs}).MarkRead(ownerCtx(), connect.NewRequest(&pb.MarkReadRequest{Repo: "demo", Number: 1})) | |
| 196 | 196 | requireCode(t, connect.CodeUnauthenticated, err) | |
| 197 | 197 | }) | |
| 198 | 198 | } | |
| @@ -214,7 +214,7 @@ func TestMergeRequestServiceGet(t *testing.T) { | |||
| 214 | 214 | mrs := mocks.NewMergeRequestService(t) | |
| 215 | 215 | mrs.EXPECT().Get(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(4), ports.GetMergeRequestOptions{SinceLastRead: true}). | |
| 216 | 216 | Return(ports.MergeRequestDetail{MergeRequest: domain.MergeRequest{Number: 4}, CommitsTruncated: true}, tt.usecase).Once() | |
| 217 | - | resp, err := (&mergeRequestService{mrs}).GetMergeRequest(ownerCtx(), connect.NewRequest(&pb.GetMergeRequestRequest{Repo: "demo", Number: 4, SinceLastRead: true})) | |
| 217 | + | resp, err := (&mergeRequestService{mrs: mrs}).GetMergeRequest(ownerCtx(), connect.NewRequest(&pb.GetMergeRequestRequest{Repo: "demo", Number: 4, SinceLastRead: true})) | |
| 218 | 218 | if tt.wantCode != 0 { | |
| 219 | 219 | requireCode(t, tt.wantCode, err) | |
| 220 | 220 | return | |
| @@ -41,7 +41,7 @@ func TestMergeRequestServiceCreate(t *testing.T) { | |||||
| 41 | Title: "t", Description: "d", Source: "feat", Target: "main", | 41 | Title: "t", Description: "d", Source: "feat", Target: "main", | ||
| 42 | }).Return(domain.MergeRequest{Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen}, tt.usecase).Once() | 42 | }).Return(domain.MergeRequest{Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen}, tt.usecase).Once() | ||
| 43 | } | 43 | } | ||
| 44 | - | resp, err := (&mergeRequestService{mrs}).CreateMergeRequest(ownerCtx(), connect.NewRequest(&pb.CreateMergeRequestRequest{ | 44 | + | resp, err := (&mergeRequestService{mrs: mrs}).CreateMergeRequest(ownerCtx(), connect.NewRequest(&pb.CreateMergeRequestRequest{ |
| 45 | Repo: tt.repo, Title: "t", Description: "d", SourceBranch: "feat", TargetBranch: "main", | 45 | Repo: tt.repo, Title: "t", Description: "d", SourceBranch: "feat", TargetBranch: "main", | ||
| 46 | })) | 46 | })) | ||
| 47 | if tt.wantCode != 0 { | 47 | if tt.wantCode != 0 { | ||
| @@ -57,7 +57,7 @@ func TestMergeRequestServiceCreate(t *testing.T) { | |||||
| 57 | 57 | ||||
| 58 | func TestMergeRequestServiceListRejectsUnknownState(t *testing.T) { | 58 | func TestMergeRequestServiceListRejectsUnknownState(t *testing.T) { | ||
| 59 | t.Parallel() | 59 | t.Parallel() | ||
| 60 | - | _, err := (&mergeRequestService{mocks.NewMergeRequestService(t)}).ListMergeRequests(ownerCtx(), | 60 | + | _, err := (&mergeRequestService{mrs: mocks.NewMergeRequestService(t)}).ListMergeRequests(ownerCtx(), |
| 61 | connect.NewRequest(&pb.ListMergeRequestsRequest{Repo: "demo", State: pb.MergeRequestState(99)})) | 61 | connect.NewRequest(&pb.ListMergeRequestsRequest{Repo: "demo", State: pb.MergeRequestState(99)})) | ||
| 62 | requireCode(t, connect.CodeInvalidArgument, err) | 62 | requireCode(t, connect.CodeInvalidArgument, err) | ||
| 63 | } | 63 | } | ||
| @@ -68,7 +68,7 @@ func TestMergeRequestServiceListFiltersByState(t *testing.T) { | |||||
| 68 | mrs.EXPECT().List(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), ports.ListMergeRequestsParams{ | 68 | mrs.EXPECT().List(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), ports.ListMergeRequestsParams{ | ||
| 69 | State: domain.MRStateMerged, PageSize: 3, PageToken: "tok", | 69 | State: domain.MRStateMerged, PageSize: 3, PageToken: "tok", | ||
| 70 | }).Return(ports.MergeRequestPage{MergeRequests: []domain.MergeRequest{{Number: 1}}, NextPageToken: "n"}, nil).Once() | 70 | }).Return(ports.MergeRequestPage{MergeRequests: []domain.MergeRequest{{Number: 1}}, NextPageToken: "n"}, nil).Once() | ||
| 71 | - | resp, err := (&mergeRequestService{mrs}).ListMergeRequests(ownerCtx(), connect.NewRequest(&pb.ListMergeRequestsRequest{ | 71 | + | resp, err := (&mergeRequestService{mrs: mrs}).ListMergeRequests(ownerCtx(), connect.NewRequest(&pb.ListMergeRequestsRequest{ |
| 72 | Repo: "demo", State: pb.MergeRequestState_MERGE_REQUEST_STATE_MERGED, PageSize: 3, PageToken: "tok", | 72 | Repo: "demo", State: pb.MergeRequestState_MERGE_REQUEST_STATE_MERGED, PageSize: 3, PageToken: "tok", | ||
| 73 | })) | 73 | })) | ||
| 74 | require.NoError(t, err) | 74 | require.NoError(t, err) | ||
| @@ -99,7 +99,7 @@ func TestMergeRequestServiceAddComment(t *testing.T) { | |||||
| 99 | mrs.EXPECT().AddComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(2), mock.Anything). | 99 | mrs.EXPECT().AddComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(2), mock.Anything). | ||
| 100 | Return(domain.Comment{ID: 5}, tt.usecase).Once() | 100 | Return(domain.Comment{ID: 5}, tt.usecase).Once() | ||
| 101 | } | 101 | } | ||
| 102 | - | resp, err := (&mergeRequestService{mrs}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{ | 102 | + | resp, err := (&mergeRequestService{mrs: mrs}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{ |
| 103 | Repo: "demo", Number: 2, Path: "f.go", Line: 3, Side: tt.side, Text: "hi", | 103 | Repo: "demo", Number: 2, Path: "f.go", Line: 3, Side: tt.side, Text: "hi", | ||
| 104 | })) | 104 | })) | ||
| 105 | if tt.wantCode != 0 { | 105 | if tt.wantCode != 0 { | ||
| @@ -120,7 +120,7 @@ func TestMergeRequestServiceResolveCommentMapsUnissuedIDsToZero(t *testing.T) { | |||||
| 120 | mrs := mocks.NewMergeRequestService(t) | 120 | mrs := mocks.NewMergeRequestService(t) | ||
| 121 | mrs.EXPECT().ResolveComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), int64(0), true). | 121 | mrs.EXPECT().ResolveComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), int64(0), true). | ||
| 122 | Return(domain.Comment{}, domain.ErrNotFound).Once() | 122 | Return(domain.Comment{}, domain.ErrNotFound).Once() | ||
| 123 | - | _, err := (&mergeRequestService{mrs}).ResolveComment(ownerCtx(), connect.NewRequest(&pb.ResolveCommentRequest{ | 123 | + | _, err := (&mergeRequestService{mrs: mrs}).ResolveComment(ownerCtx(), connect.NewRequest(&pb.ResolveCommentRequest{ |
| 124 | Repo: "demo", Number: 1, CommentId: id, Resolved: true, | 124 | Repo: "demo", Number: 1, CommentId: id, Resolved: true, | ||
| 125 | })) | 125 | })) | ||
| 126 | requireCode(t, connect.CodeNotFound, err) | 126 | requireCode(t, connect.CodeNotFound, err) | ||
| @@ -154,7 +154,7 @@ func TestMergeRequestServiceMerge(t *testing.T) { | |||||
| 154 | return p.DeleteSource == tt.req.GetDeleteSourceBranch() && (tt.req.GetHeadId() == "") == p.HeadID.IsZero() | 154 | return p.DeleteSource == tt.req.GetDeleteSourceBranch() && (tt.req.GetHeadId() == "") == p.HeadID.IsZero() | ||
| 155 | })).Return(ports.MergeResult{MergeRequest: domain.MergeRequest{Number: 1}, SourceDeleted: true}, tt.usecase).Once() | 155 | })).Return(ports.MergeResult{MergeRequest: domain.MergeRequest{Number: 1}, SourceDeleted: true}, tt.usecase).Once() | ||
| 156 | } | 156 | } | ||
| 157 | - | resp, err := (&mergeRequestService{mrs}).MergeMergeRequest(ownerCtx(), connect.NewRequest(tt.req)) | 157 | + | resp, err := (&mergeRequestService{mrs: mrs}).MergeMergeRequest(ownerCtx(), connect.NewRequest(tt.req)) |
| 158 | if tt.wantCode != 0 { | 158 | if tt.wantCode != 0 { | ||
| 159 | requireCode(t, tt.wantCode, err) | 159 | requireCode(t, tt.wantCode, err) | ||
| 160 | return | 160 | return | ||
| @@ -171,20 +171,20 @@ func TestMergeRequestServiceCloseAndMarkRead(t *testing.T) { | |||||
| 171 | t.Parallel() | 171 | t.Parallel() | ||
| 172 | mrs := mocks.NewMergeRequestService(t) | 172 | mrs := mocks.NewMergeRequestService(t) | ||
| 173 | mrs.EXPECT().Close(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(9)).Return(domain.MergeRequest{}, domain.ErrNotFound).Once() | 173 | mrs.EXPECT().Close(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(9)).Return(domain.MergeRequest{}, domain.ErrNotFound).Once() | ||
| 174 | - | _, err := (&mergeRequestService{mrs}).CloseMergeRequest(ownerCtx(), connect.NewRequest(&pb.CloseMergeRequestRequest{Repo: "demo", Number: 9})) | 174 | + | _, err := (&mergeRequestService{mrs: mrs}).CloseMergeRequest(ownerCtx(), connect.NewRequest(&pb.CloseMergeRequestRequest{Repo: "demo", Number: 9})) |
| 175 | requireCode(t, connect.CodeNotFound, err) | 175 | requireCode(t, connect.CodeNotFound, err) | ||
| 176 | }) | 176 | }) | ||
| 177 | t.Run("close ok", func(t *testing.T) { | 177 | t.Run("close ok", func(t *testing.T) { | ||
| 178 | t.Parallel() | 178 | t.Parallel() | ||
| 179 | mrs := mocks.NewMergeRequestService(t) | 179 | mrs := mocks.NewMergeRequestService(t) | ||
| 180 | mrs.EXPECT().Close(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(9)).Return(domain.MergeRequest{Number: 9, State: domain.MRStateClosed}, nil).Once() | 180 | mrs.EXPECT().Close(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(9)).Return(domain.MergeRequest{Number: 9, State: domain.MRStateClosed}, nil).Once() | ||
| 181 | - | resp, err := (&mergeRequestService{mrs}).CloseMergeRequest(ownerCtx(), connect.NewRequest(&pb.CloseMergeRequestRequest{Repo: "demo", Number: 9})) | 181 | + | resp, err := (&mergeRequestService{mrs: mrs}).CloseMergeRequest(ownerCtx(), connect.NewRequest(&pb.CloseMergeRequestRequest{Repo: "demo", Number: 9})) |
| 182 | require.NoError(t, err) | 182 | require.NoError(t, err) | ||
| 183 | assert.Equal(t, pb.MergeRequestState_MERGE_REQUEST_STATE_CLOSED, resp.Msg.GetMergeRequest().GetState()) | 183 | assert.Equal(t, pb.MergeRequestState_MERGE_REQUEST_STATE_CLOSED, resp.Msg.GetMergeRequest().GetState()) | ||
| 184 | }) | 184 | }) | ||
| 185 | t.Run("mark read rejects a short head", func(t *testing.T) { | 185 | t.Run("mark read rejects a short head", func(t *testing.T) { | ||
| 186 | t.Parallel() | 186 | t.Parallel() | ||
| 187 | - | _, err := (&mergeRequestService{mocks.NewMergeRequestService(t)}).MarkRead(ownerCtx(), | 187 | + | _, err := (&mergeRequestService{mrs: mocks.NewMergeRequestService(t)}).MarkRead(ownerCtx(), |
| 188 | connect.NewRequest(&pb.MarkReadRequest{Repo: "demo", Number: 1, HeadId: "abc"})) | 188 | connect.NewRequest(&pb.MarkReadRequest{Repo: "demo", Number: 1, HeadId: "abc"})) | ||
| 189 | requireCode(t, connect.CodeInvalidArgument, err) | 189 | requireCode(t, connect.CodeInvalidArgument, err) | ||
| 190 | }) | 190 | }) | ||
| @@ -192,7 +192,7 @@ func TestMergeRequestServiceCloseAndMarkRead(t *testing.T) { | |||||
| 192 | t.Parallel() | 192 | t.Parallel() | ||
| 193 | mrs := mocks.NewMergeRequestService(t) | 193 | mrs := mocks.NewMergeRequestService(t) | ||
| 194 | mrs.EXPECT().MarkRead(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), domain.ObjectID{}).Return(domain.ObjectID{}, domain.ErrUnauthenticated).Once() | 194 | mrs.EXPECT().MarkRead(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), domain.ObjectID{}).Return(domain.ObjectID{}, domain.ErrUnauthenticated).Once() | ||
| 195 | - | _, err := (&mergeRequestService{mrs}).MarkRead(ownerCtx(), connect.NewRequest(&pb.MarkReadRequest{Repo: "demo", Number: 1})) | 195 | + | _, err := (&mergeRequestService{mrs: mrs}).MarkRead(ownerCtx(), connect.NewRequest(&pb.MarkReadRequest{Repo: "demo", Number: 1})) |
| 196 | requireCode(t, connect.CodeUnauthenticated, err) | 196 | requireCode(t, connect.CodeUnauthenticated, err) | ||
| 197 | }) | 197 | }) | ||
| 198 | } | 198 | } | ||
| @@ -214,7 +214,7 @@ func TestMergeRequestServiceGet(t *testing.T) { | |||||
| 214 | mrs := mocks.NewMergeRequestService(t) | 214 | mrs := mocks.NewMergeRequestService(t) | ||
| 215 | mrs.EXPECT().Get(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(4), ports.GetMergeRequestOptions{SinceLastRead: true}). | 215 | mrs.EXPECT().Get(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(4), ports.GetMergeRequestOptions{SinceLastRead: true}). | ||
| 216 | Return(ports.MergeRequestDetail{MergeRequest: domain.MergeRequest{Number: 4}, CommitsTruncated: true}, tt.usecase).Once() | 216 | Return(ports.MergeRequestDetail{MergeRequest: domain.MergeRequest{Number: 4}, CommitsTruncated: true}, tt.usecase).Once() | ||
| 217 | - | resp, err := (&mergeRequestService{mrs}).GetMergeRequest(ownerCtx(), connect.NewRequest(&pb.GetMergeRequestRequest{Repo: "demo", Number: 4, SinceLastRead: true})) | 217 | + | resp, err := (&mergeRequestService{mrs: mrs}).GetMergeRequest(ownerCtx(), connect.NewRequest(&pb.GetMergeRequestRequest{Repo: "demo", Number: 4, SinceLastRead: true})) |
| 218 | if tt.wantCode != 0 { | 218 | if tt.wantCode != 0 { | ||
| 219 | requireCode(t, tt.wantCode, err) | 219 | requireCode(t, tt.wantCode, err) | ||
| 220 | return | 220 | return | ||