Add unit tests for Prometheus (pkg/metrics) package#17
Open
abdultolba wants to merge 2 commits intopmady:mainfrom
Open
Add unit tests for Prometheus (pkg/metrics) package#17abdultolba wants to merge 2 commits intopmady:mainfrom
abdultolba wants to merge 2 commits intopmady:mainfrom
Conversation
- Add helper functions for generating Prometheus API JSON responses
- Add success tests for Query, GetLatencyP99, GetLatencyP95,
GetGPUUtilization, and GetQueueDepth methods
- Add error handling tests covering connection failures, HTTP errors,
Prometheus errors, empty results, invalid JSON, and unexpected types
- Add tests for context cancellation, custom queries, multiple samples,
and NaN values
- Achieve 93.8% test coverage for the metrics package
Closes pmady#2
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive unit tests for the PrometheusClient in the pkg/metrics package, which previously only had tests for the MockClient implementation. The tests use httptest to mock the Prometheus HTTP API, achieving 93.8% test coverage (exceeding the 80% target).
Changes:
- Added helper functions to generate various Prometheus API response types (vector, scalar, error, etc.)
- Added success path tests for all client methods (GetLatencyP99, GetLatencyP95, GetGPUUtilization, GetQueueDepth)
- Added comprehensive error handling tests (connection failures, HTTP errors, empty results, invalid JSON, unexpected result types)
- Added edge case tests (context cancellation, multiple samples, custom queries, NaN values)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Rename unused parameters to _ in HTTP handler functions - Use strings.Builder instead of string concatenation in loop - Rename TestPrometheusClient_ContextCancellation to TestPrometheusClient_ContextTimeout and fix hanging test
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 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.
Summary
PrometheusClientusinghttptestto mock the Prometheus HTTP APIMockClientwas tested; now the real client implementation has full test coverageChanges
Helper Functions:
makeVectorResponse()- generates successful vector responsesmakeScalarResponse()- generates scalar responsesmakeEmptyVectorResponse()- generates empty result responsesmakeErrorResponse()- generates Prometheus error responsesmakeMatrixResponse()- generates unexpected matrix type responsesmakeVectorResponseMultiple()- generates multi-sample responsesSuccess Tests:
TestPrometheusClient_ImplementsInterface- compile-time interface checkTestNewPrometheusClient- constructor validationTestPrometheusClient_Query_Success- vector and scalar response handlingTestPrometheusClient_GetLatencyP99- verifies default query and return valueTestPrometheusClient_GetLatencyP95- verifies default query and return valueTestPrometheusClient_GetGPUUtilization- verifies default query and return valueTestPrometheusClient_GetQueueDepth- verifies int64 conversion and default queryError Handling Tests:
Edge Case Tests:
Test Plan
go test ./pkg/metrics/...- all tests passgo test -cover ./pkg/metrics/...- 93.8% coverage (exceeds 80% target)go vet ./pkg/metrics/...- no issuesCloses #2