Tighten error handling in query latency test suite #705
Description
Follow-up from PR #648 review. The production slowQueryDB wrapper is solid, but there are several error handling gaps in the test/benchmark code that could mask failures or produce misleading profiling results.
Important
1. Silent error swallowing in TestConnectionPoolTuning goroutines
gtfsdb/query_latency_test.go:547-549
Query errors inside goroutines are silently discarded. If all queries fail for a given pool size, that row is silently omitted from the results table. Should use an error channel matching the pattern from TestQueryLatencyUnderConcurrentLoad.
2. loadPerfFixture catches all import errors as non-fatal
gtfsdb/query_latency_perftest_test.go:51-54
The catch block swallows all import errors (corrupted zip, disk full, schema mismatch), not just the expected duplicate-import case. If the import fails and the database is empty, benchmarks run against an empty database with no test failure. Fix by checking latencyIsEmpty after a failed import and calling b.Fatalf if the database has no data.
3. Missing rows.Err() check in TestExplainQueryPlans
gtfsdb/query_latency_test.go:429-452
After the for rows.Next() loop, rows.Err() is never checked. Per the database/sql contract, rows.Next() can return false due to an iteration error.
Fit and Finish
4. latencyFetchRouteIDsForStop / latencyFetchActiveServiceIDs conflate errors with empty results
gtfsdb/query_latency_test.go:126-150
Both helpers log a WARNING and return nil on error, making database errors indistinguishable from "no data." Consider failing hard with tb.Fatalf since these are test setup helpers.
5. ExecContext and QueryRowContext have no dedicated slow-query tests
gtfsdb/helpers_test.go:133-175
TestSlowQueryDB_LogsSlowQueries only exercises QueryContext. The other two DBTX methods should also be tested to verify maybeLog is called with the correct op attribute.
6. maybeLog error-inclusion branch is untested
gtfsdb/helpers.go:104
No test triggers a slow query that also returns an error. The if err != nil branch that appends an error attribute to the log record has zero test coverage.