- Notifications
You must be signed in to change notification settings - Fork1k
chore(coderd/database/dbauthz): migrate TestSystemFunctions to mocked DB#19301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
b615cb1 tod5f03c2Comparebf65b40 to31de126Compare31de126 to3dd08e2CompareUh oh!
There was an error while loading.Please reload this page.
| dbm.EXPECT().GetProvisionerJobByID(gomock.Any(),j.ID).Return(j,nil).AnyTimes() | ||
| dbm.EXPECT().GetWorkspaceBuildByJobID(gomock.Any(),j.ID).Return(b,nil).AnyTimes() | ||
| dbm.EXPECT().GetWorkspaceByID(gomock.Any(),b.WorkspaceID).Return(ws,nil).AnyTimes() | ||
| dbm.EXPECT().GetProvisionerJobTimingsByJobID(gomock.Any(),j.ID).Return([]database.ProvisionerJobTiming{},nil).AnyTimes() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This doesn't return any of the objects which the test is purporting to validate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Seems like it's not needed since we use the job to authz the call 👍
- Convert many System functions to s.Mocked with gomock- Add expectations for read/update/create/delete system paths- Fix UpdateWorkspaceBuildCostByID expectation (returns error)
- Migrate AcquireProvisionerJob and ProvisionerJob update helpers- Convert InsertWorkspaceAgent/UpsertWorkspaceApp and related to s.Mocked- Keep RBAC assertions consistent
… setups and expectations
3dd08e2 to0190b51CompareSigned-off-by: Danny Kopping <danny@coder.com>
0190b51 to1502a6dCompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM
| dbm.EXPECT().GetProvisionerJobByID(gomock.Any(),j.ID).Return(j,nil).AnyTimes() | ||
| dbm.EXPECT().GetWorkspaceBuildByJobID(gomock.Any(),j.ID).Return(b,nil).AnyTimes() | ||
| dbm.EXPECT().GetWorkspaceByID(gomock.Any(),b.WorkspaceID).Return(ws,nil).AnyTimes() | ||
| dbm.EXPECT().GetProvisionerJobTimingsByJobID(gomock.Any(),j.ID).Return([]database.ProvisionerJobTiming{},nil).AnyTimes() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Seems like it's not needed since we use the job to authz the call 👍
7365da1 intomainUh oh!
There was an error while loading.Please reload this page.
Related tocoder/internal#869