root 125e1c80b9 tests: close R-002 / R-003 / R-008 — internal/shared, storeclient, queryd/db.go
Audit-driven follow-up to the Rust scrum review on the 3 untested
HIGH-risk packages. Both the audit (reports/scrum/risk-register.md)
and the scrum (tests/real-world/runs/scrum_mojxb5bw/) independently
flagged these files as the highest-leverage missing test coverage.

internal/shared/server_test.go — 8 test funcs
  newListener: valid addr, invalid addr (non-numeric port, port
    out of range, port-already-in-use surfacing as net.OpError).
  Empty-addr-is-valid: documents the net.Listen quirk that "" binds
    an OS-picked port — future readers don't need to relitigate.
  HealthResponse marshal: JSON shape stable, round-trip clean.
  /health handler reconstructed via httptest.Server: status 200,
    Content-Type application/json, body fields stable.
  RegisterRoutes callback: contract verified (callback is invoked
    with a real chi.Router, mounted route reachable end-to-end).
  Run bind-failure surface: synchronous error, not a goroutine swallow
    — the contract Run depends on per the race-safe-startup comment.

internal/shared/config_test.go — 6 test funcs
  DefaultConfig G0 port pinning: every binary's default bind locked
    in (3110/3211-3216) so a refactor can't silently flip a port.
  LoadConfig empty path: returns DefaultConfig, no error.
  LoadConfig missing file: returns DefaultConfig, logs warn (the warn
    line shows up in test output, captured-but-not-asserted).
  LoadConfig valid TOML: partial overrides land, unspecified sections
    keep defaults (TOML decoder leave-alone behavior).
  LoadConfig invalid TOML: returns wrapped 'parse config' error.
  LoadConfig unreadable file: skipped under root (root reads 0000);
    captures the read-error wrap path for non-root contexts.

internal/storeclient/client_test.go — 14 test funcs
  safeKey table-driven: plain segments, single slash, empty, trailing
    slash, space (→ %20), apostrophe (→ %27), unicode (→ %C3%A9),
    deep nesting. Locks URL-escape contract per scrum suggestion.
  recordingServer helper backs Put/Get/Delete/List against
    httptest.Server: verifies method, path, body bytes round-trip.
  ErrKeyNotFound on 404 (errors.Is round-trip).
  Non-OK status wraps body preview into the error chain.
  Delete accepts both 200 and 204 (S3 vs compatible-store quirk).
  List parses JSON shape and surfaces query-string prefix.
  Context cancellation propagates through Put as context.Canceled.

internal/queryd/db_test.go — 5 test funcs (with subtests)
  sqlEscape table-driven: 8 cases including empty, all-quotes,
    nested apostrophes (the case from the scrum suggestion).
  redactCreds table-driven: 6 cases — both keys, single keys,
    empty, multi-occurrence, placeholder-collision (lossy but safe).
  buildBootstrap statement order: INSTALL → LOAD → CREATE SECRET.
  buildBootstrap endpoint schemes: http strips + USE_SSL false,
    https keeps SSL true, no-scheme defaults SSL true (prod ambient).
  buildBootstrap URL_STYLE: 'path' vs 'vhost' branch.
  buildBootstrap escapes credential quotes: future SSO-token-with-
    apostrophe doesn't break out of the SQL string literal — the
    belt holds when the suspenders snap.

Real finding caught by my own test:
  net.Listen("tcp", "") succeeds (OS-picked port) — captured as
  TestNewListener_EmptyAddrIsValid so the quirk is documented.

Verified:
  go test -short ./... — every internal/ package now has tests
    (no more 'no test files' lines for shared/storeclient).
  just verify — vet + test + 9 smokes green in 33s.
  just proof contract — 53/0/1 green (no harness regression).

Closes:
  R-002 internal/shared zero tests        HIGH
  R-003 internal/storeclient zero tests   HIGH
  R-008 queryd/db.go untested             MED (sqlEscape, redactCreds,
                                              CREATE SECRET formation)

Composite scrum score should move from 43 → ~46 / 60 — the three
HIGH/MED risks closed, internal/shared and internal/storeclient
become "tested + load-bearing" instead of "untested + load-bearing."

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-29 05:51:05 -05:00

187 lines
5.5 KiB
Go

package queryd
import (
"strings"
"testing"
"git.agentview.dev/profit/golangLAKEHOUSE/internal/secrets"
"git.agentview.dev/profit/golangLAKEHOUSE/internal/shared"
)
// Closes R-008: db.go owns sqlEscape + redactCreds + buildBootstrap,
// none of which had tests. The first two are pure functions trivial
// to table-test; buildBootstrap is also pure (S3Config + creds → SQL
// strings) so we can exercise its endpoint-normalization branches
// without booting DuckDB.
func TestSqlEscape(t *testing.T) {
cases := []struct {
name string
in string
want string
}{
{"no quotes", "hello", "hello"},
{"single quote", "O'Reilly", "O''Reilly"},
{"double quote pair", "''", "''''"},
{"trailing quote", "foo'", "foo''"},
{"leading quote", "'foo", "''foo"},
{"empty string", "", ""},
{"only quotes", "'''", "''''''"},
{"mixed punctuation", "it's a 'test'", "it''s a ''test''"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := sqlEscape(tc.in)
if got != tc.want {
t.Errorf("sqlEscape(%q) = %q, want %q", tc.in, got, tc.want)
}
})
}
}
func TestRedactCreds(t *testing.T) {
cases := []struct {
name string
creds secrets.S3Credentials
msg string
want string
}{
{
"both keys redacted",
secrets.S3Credentials{AccessKeyID: "AKIATEST", SecretAccessKey: "topsecret"},
"failed: KEY_ID 'AKIATEST' SECRET 'topsecret'",
"failed: KEY_ID '[REDACTED-KEY]' SECRET '[REDACTED-SECRET]'",
},
{
"only access key present",
secrets.S3Credentials{AccessKeyID: "AKIATEST", SecretAccessKey: ""},
"echo: AKIATEST again",
"echo: [REDACTED-KEY] again",
},
{
"only secret present",
secrets.S3Credentials{AccessKeyID: "", SecretAccessKey: "mysecret"},
"echo: mysecret here",
"echo: [REDACTED-SECRET] here",
},
{
"empty creds = no change",
secrets.S3Credentials{},
"failed: nothing to scrub",
"failed: nothing to scrub",
},
{
"value appears multiple times",
secrets.S3Credentials{AccessKeyID: "AKIATEST"},
"AKIATEST failed because AKIATEST",
"[REDACTED-KEY] failed because [REDACTED-KEY]",
},
{
"key value collision with placeholder string is lossy but safe",
secrets.S3Credentials{AccessKeyID: "[REDACTED-KEY]"},
"loop: [REDACTED-KEY]",
"loop: [REDACTED-KEY]",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := redactCreds(tc.msg, tc.creds)
if got != tc.want {
t.Errorf("redactCreds:\n msg=%q\n got=%q\n want=%q", tc.msg, got, tc.want)
}
})
}
}
func TestBuildBootstrap_StatementOrder(t *testing.T) {
stmts := buildBootstrap(
shared.S3Config{Endpoint: "http://localhost:9000", Region: "us-east-1", UsePathStyle: true},
secrets.S3Credentials{AccessKeyID: "key", SecretAccessKey: "secret"},
)
if len(stmts) != 3 {
t.Fatalf("want 3 statements, got %d: %v", len(stmts), stmts)
}
if stmts[0] != "INSTALL httpfs" {
t.Errorf("stmt[0] = %q, want INSTALL httpfs", stmts[0])
}
if stmts[1] != "LOAD httpfs" {
t.Errorf("stmt[1] = %q, want LOAD httpfs", stmts[1])
}
if !strings.HasPrefix(stmts[2], "CREATE OR REPLACE SECRET") {
t.Errorf("stmt[2] should start with CREATE OR REPLACE SECRET, got %q", stmts[2])
}
}
func TestBuildBootstrap_EndpointSchemes(t *testing.T) {
cases := []struct {
name string
endpoint string
wantHostInSQL string
wantUseSSLTrue bool
}{
{"http strips scheme, USE_SSL false",
"http://minio:9000", "minio:9000", false},
{"https keeps SSL true",
"https://s3.example.com", "s3.example.com", true},
{"no scheme defaults SSL true (ambient prod)",
"s3.amazonaws.com", "s3.amazonaws.com", true},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
stmts := buildBootstrap(
shared.S3Config{Endpoint: tc.endpoint, Region: "us-east-1"},
secrets.S3Credentials{AccessKeyID: "k", SecretAccessKey: "s"},
)
secret := stmts[2]
wantEndpointFrag := "ENDPOINT '" + tc.wantHostInSQL + "'"
if !strings.Contains(secret, wantEndpointFrag) {
t.Errorf("secret SQL missing %q\n got: %s", wantEndpointFrag, secret)
}
wantSSL := "USE_SSL false"
if tc.wantUseSSLTrue {
wantSSL = "USE_SSL true"
}
if !strings.Contains(secret, wantSSL) {
t.Errorf("secret SQL missing %q\n got: %s", wantSSL, secret)
}
})
}
}
func TestBuildBootstrap_URLStyle(t *testing.T) {
pathStmts := buildBootstrap(
shared.S3Config{Endpoint: "http://m:9000", UsePathStyle: true},
secrets.S3Credentials{AccessKeyID: "k", SecretAccessKey: "s"},
)
if !strings.Contains(pathStmts[2], "URL_STYLE 'path'") {
t.Errorf("UsePathStyle=true should produce URL_STYLE 'path'\n got: %s", pathStmts[2])
}
vhostStmts := buildBootstrap(
shared.S3Config{Endpoint: "https://m", UsePathStyle: false},
secrets.S3Credentials{AccessKeyID: "k", SecretAccessKey: "s"},
)
if !strings.Contains(vhostStmts[2], "URL_STYLE 'vhost'") {
t.Errorf("UsePathStyle=false should produce URL_STYLE 'vhost'\n got: %s", vhostStmts[2])
}
}
func TestBuildBootstrap_EscapesCredentialQuotes(t *testing.T) {
// Per the inline comment: "creds shouldn't contain ' but a future
// SSO token might." This is the test that asserts the belt holds
// when the suspenders snap.
stmts := buildBootstrap(
shared.S3Config{Endpoint: "https://m", Region: "us-east-1"},
secrets.S3Credentials{
AccessKeyID: "key'with'quotes",
SecretAccessKey: "secret",
},
)
secret := stmts[2]
// Escaped form: each ' became ''.
want := "KEY_ID 'key''with''quotes'"
if !strings.Contains(secret, want) {
t.Errorf("expected escaped key in SQL\n want fragment: %s\n got: %s", want, secret)
}
}