chore(health): adding health check tests#19
Merged
Jacobbrewer1 merged 3 commits intomainfrom Apr 10, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
health/status_test.go:54
- When testing invalid statuses, the error message should indicate that an error is expected rather than stating 'should not return an error'. Consider updating the message to accurately reflect the expected behavior.
require.Equal(t, tt.wantErr, err, "MarshalJSON() should not return an error")
health/checker_test.go:76
- The error message is misleading: while the expected HTTP status is ServiceUnavailable, the message indicates 200 OK. Please update the message to accurately reflect the expected status code.
require.Equal(t, http.StatusServiceUnavailable, rec.Code, "Handler() should return a 200 OK status code")
Contributor
There was a problem hiding this comment.
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
health/status_test.go:46
- Verify that the error message produced by MarshalJSON for an invalid status exactly matches the expected error message in the test. If the Status type's String() method returns a numeric value (e.g. "999") rather than "invalid", adjust either the test or the error formatting accordingly.
wantErr: errors.New("invalid is not a valid status"),
health/response.go:1
- Ensure that the removal of the health/response.go file does not leave any dangling references elsewhere in the codebase, which could lead to runtime issues.
package health
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Describe your changes
This pull request includes several changes to the health check system to improve context handling, error handling, and testing. The most important changes include adding context checks, improving error handling for invalid statuses, and enhancing the test coverage.
Context Handling Improvements:
health/check.go: Added a check to ensurectxis not nil and set it tocontext.Background()if it is. [1] [2]health/checker.go: Added a check to ensurectxis not nil and set it tocontext.Background()if it is.Error Handling Enhancements:
health/check.go: Added validation to ensurestatusErr.Statusis valid, setting it toStatusUnknownif it is not.health/status.go: ModifiedMarshalJSONto return an error for invalid statuses.Test Coverage Enhancements:
health/check_test.go: Added new tests for no timeout and no parent context scenarios.health/checker_test.go: Added new tests for status errors and no parent context scenarios. [1] [2]health/result_test.go: Added a new test to ensureaddDetailinitializes the details map if it is nil.health/status_test.go: Added a new test to validate error handling inMarshalJSONfor invalid statuses.Code Simplification:
health/result.go: ChangedDetailsmap to use pointers toResultto simplify memory management. [1] [2] [3]health/time.go: RenamedTimestamptotimestampto follow Go naming conventions for unexported variables.Documentation:
health/check_options.go: Added documentation forWithNoCheckTimeoutfunction.