-
Notifications
You must be signed in to change notification settings - Fork 896
chore: improve testing coverage on ExtractProvisionerDaemonAuthenticated middleware #15622
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, routeCtx)) | ||
res := httptest.NewRecorder() | ||
|
||
r.Header.Set(codersdk.ProvisionerDaemonKey, "5Hl2Qw9kX3nM7vB4jR8pY6tA1cF0eD5uI2oL9gN3mZ4") |
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.
review: Just generated a random string which should be different from the one generate in CreateProvisionerKey
The objective here is to use a non-existing key to have a 404-like from DB.
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.
Thanks @dannykopping - I indeed was missing some test cases - I added one for the compare failing, and another one in case the DB fails - so we now have 100% test coverage on this middleware 👀 |
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.
LGTM, thanks!
This one aims to resolve #15604
Created some table tests for the main cases -
also preferred to create two isolated cases for the most complicated cases in order to keep table tests simple enough.
Give us full coverage on the middleware logic, for both optional and non optional cases - PSK and ProvisionerKey.