feat/tests: tests for error handling & metrics in admin endpoints #6805
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
gdhameeja
force-pushed
the
feat/test-admin-error-handling
branch
from
80816c6 to
704394d
Compare
|
Thanks for the PR! Maybe @mohammed90 or @hairyhenderson would be suitable for reviewing this if they have a few minutes. 👍 |
- TestAdminHandlerErrorHandling - Tests the handler.handleError() functionality by directly verifying error response formatting - TestAdminHandlerBuiltinRouteErrors - Tests the error handling pathway by using real admin server routes and verifying both error responses and prometheus metrics increments - provisionAdminRouters: add unit tests for admin handler registration and routing for admin.api - TestAllowedOriginsUnixSocket: checks unix socket with default origins are added - TestReplaceRemoteAdminServer: test for replaceRemoteAdminServer with certificate validation, custom origins and cleanup
gdhameeja
force-pushed
the
feat/test-admin-error-handling
branch
from
6364add to
cacbe17
Compare
|
@mholt @mohammed90 Just pushed a couple more tests for admin. Please review the same and let me know if any fixes are required. Thanks :) |
gdhameeja
force-pushed
the
feat/test-admin-error-handling
branch
2 times, most recently
from
4486cb9 to
6138fe1
Compare
gdhameeja
force-pushed
the
feat/test-admin-error-handling
branch
from
6138fe1 to
cc504eb
Compare
|
@mholt @mohammed90 gentle reminder to review this PR. Would love to see this merged as it is my first open source contribution after a long time :) |
|
We're glad for the contribution, thank you! I've just been very busy. I haven't reviewed the change super thoroughly, but it looks acceptable at a glance. Generally, we welcome more tests! |
mholt
approved these changes
Feb 12, 2025
mohammed90
approved these changes
Feb 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
functionality by directly verifying error response formatting
handling pathway by using real admin server routes and verifying
both error responses and prometheus metrics increments
this increases the test coverage of admin.go from 31% to 59%.
Increases total coverage from 37.9% to 47%.
I added this PR as a way to learn more about caddy's codebase and to contribute to caddy. I'd love to contribute more of these tests if such contributions are welcome.