Add new data test#12
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds testability seams around the /data handler and expands unit/HTTP test coverage for JSON, CSV, error handling, and record-merging behavior.
Changes:
- Introduces
recordIterator,fetchPoints, andcollectPointsto isolate DB querying from record collection. - Updates
Handleto call the newfetchPointsseam before JSON/CSV response formatting. - Adds handler, CSV, and collectPoints-focused tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
internal/data/data.go |
Extracts data fetching/collection logic behind testable seams while preserving response formatting flow. |
internal/data/data_test.go |
Adds HTTP handler tests, CSV output tests, and targeted collectPoints tests using fake Flux records. |
Comments suppressed due to low confidence (1)
internal/data/data_test.go:223
- This nil-value test only checks the zero-value field on a newly created placeholder, so it would not catch a regression that overwrites an existing nonzero CO2 value with zero when a later nil record for the same host/time arrives. Seed the iterator with a valid value before the nil record to verify the "does not overwrite" behavior described by the assertion.
require.Len(t, points, 1, "a placeholder Data entry is created before the nil-value check")
for _, d := range points {
require.Equal(t, "frog-01", d.Host)
require.Equal(t, 0.0, d.CO2, "nil Value() must not be written into the field")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.
Adds HTTP-level test coverage for the
/datahandler and the CSV/JSON formatting paths added in PR #8, plus targeted coverage for the reflect-based record-merging logic.What changed
internal/data/data.go — two small testability seams, no behavior change:
recordIteratorinterface (Next/Record/Err); the real*api.QueryTableResultalready satisfies it.fetchPointspackage-level function variable wrapping the DB call soHandlecan be exercised end-to-end without a live InfluxDB.collectPoints(res recordIterator) map[string]*DatafromHandleso the iteration branches are independently testable.internal/data/data_test.go — 10 new tests:
TestHandle_MethodNotAllowed— non-GET returns 405.TestHandle_BadStart_Returns400WithBody— missing and unparseablestartboth return 400 with a non-empty body.TestHandle_DefaultReturnsJSON— default response hasContent-Type: application/jsonand decodes cleanly.TestHandle_FormatCSV_ReturnsCSV—?format=csvreturnsContent-Type: text/csvand a valid CSV body.TestHandle_AcceptCSV_ReturnsCSV—Accept: text/csvproduces the same.TestHandle_FetchError_Returns500— error path fromfetchPointssurfaces as 500.TestWriteCSV_HeaderRowAndColumnOrder— first CSV row equalscsvHeadersexactly.TestCollectPoints_SkipsMissingHostTag— records without ahosttag are dropped.TestCollectPoints_SkipsNilValue— records withValue() == nildo not panic and do not overwrite the field.TestCollectPoints_PopulatesKnownFields— mapped fields land in the right struct fields; unknown fields are ignored.Notes / follow-up
The nil-Value test pins current behavior: when
Value()is nil but the record has a valid host, a placeholderData{Host: ...}entry is still created in the map (the write happens before the nil guard inHandle's loop). The test documents this rather than changing it — worth a separate decision on whether to move the placeholder write below the nil check.