Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
defelmnq merged 8 commits intomainfrom15604-improve-middleware-coverage
Nov 26, 2024

Conversation

defelmnq
Copy link
Contributor

This one aims toresolve#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.

r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, routeCtx))
res := httptest.NewRecorder()

r.Header.Set(codersdk.ProvisionerDaemonKey, "5Hl2Qw9kX3nM7vB4jR8pY6tA1cF0eD5uI2oL9gN3mZ4")
Copy link
ContributorAuthor

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 inCreateProvisionerKey
The objective here is to use a non-existing key to have a 404-like from DB.

@defelmnqdefelmnq changed the titlechore(coderd): Improve testing coverage on ExtractProvisionerDaemonAuthenticated middlewarechore(coderd): improve testing coverage on ExtractProvisionerDaemonAuthenticated middlewareNov 22, 2024
@defelmnqdefelmnq changed the titlechore(coderd): improve testing coverage on ExtractProvisionerDaemonAuthenticated middlewarechore: improve testing coverage on ExtractProvisionerDaemonAuthenticated middlewareNov 23, 2024
Copy link
Contributor

@dannykoppingdannykopping left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Firstly, thanks for following up on this!

image

There's this block which seems to not be covered by tests, and this feels like the most important block. Is there a reason you're not testing that one specifically?

@defelmnq
Copy link
ContributorAuthor

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 👀

Copy link
Contributor

@dannykoppingdannykopping left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM, thanks!

@defelmnqdefelmnq merged commit60ddcf5 intomainNov 26, 2024
27 checks passed
@defelmnqdefelmnq deleted the 15604-improve-middleware-coverage branchNovember 26, 2024 03:02
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@dannykoppingdannykoppingdannykopping approved these changes

@johnstcnjohnstcnjohnstcn approved these changes

Assignees

@defelmnqdefelmnq

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve testing coverage on ExtractProvisionerDaemonAuthenticated middleware
3 participants
@defelmnq@dannykopping@johnstcn
[8]ページ先頭
©2009-2025 Movatter.jp