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>
151 lines
5.0 KiB
Go
151 lines
5.0 KiB
Go
package shared
|
|
|
|
import (
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
// Closes the config.go side of R-002 — TOML loader, default values,
|
|
// missing-file warn semantics. The audit flagged "internal/shared
|
|
// has zero tests" without distinguishing server.go from config.go;
|
|
// this file covers the latter.
|
|
|
|
func TestDefaultConfig_G0Ports(t *testing.T) {
|
|
cfg := DefaultConfig()
|
|
// Ports are shifted to 3110+ to coexist with the live Rust
|
|
// lakehouse on 3100/3201-3204 during the migration. Locking
|
|
// these values via test means a refactor that flips a port
|
|
// silently can't ship without a test edit.
|
|
checks := []struct {
|
|
name string
|
|
actual string
|
|
expected string
|
|
}{
|
|
{"gateway bind", cfg.Gateway.Bind, "127.0.0.1:3110"},
|
|
{"storaged bind", cfg.Storaged.Bind, "127.0.0.1:3211"},
|
|
{"catalogd bind", cfg.Catalogd.Bind, "127.0.0.1:3212"},
|
|
{"ingestd bind", cfg.Ingestd.Bind, "127.0.0.1:3213"},
|
|
{"queryd bind", cfg.Queryd.Bind, "127.0.0.1:3214"},
|
|
{"vectord bind", cfg.Vectord.Bind, "127.0.0.1:3215"},
|
|
{"embedd bind", cfg.Embedd.Bind, "127.0.0.1:3216"},
|
|
}
|
|
for _, c := range checks {
|
|
if c.actual != c.expected {
|
|
t.Errorf("%s = %q, want %q", c.name, c.actual, c.expected)
|
|
}
|
|
}
|
|
// G0 default: 256 MiB ingest cap (real-scale 500K test bumped
|
|
// this to 512 — still 256 here as the documented default).
|
|
if cfg.Ingestd.MaxIngestBytes != 256<<20 {
|
|
t.Errorf("ingestd MaxIngestBytes = %d, want %d", cfg.Ingestd.MaxIngestBytes, 256<<20)
|
|
}
|
|
// embedd default model is the G2 nomic-embed-text default.
|
|
if cfg.Embedd.DefaultModel != "nomic-embed-text" {
|
|
t.Errorf("embedd DefaultModel = %q, want nomic-embed-text", cfg.Embedd.DefaultModel)
|
|
}
|
|
// queryd refresh ticker default — production value, not the proof
|
|
// harness's 500ms override.
|
|
if cfg.Queryd.RefreshEvery != "30s" {
|
|
t.Errorf("queryd RefreshEvery = %q, want 30s", cfg.Queryd.RefreshEvery)
|
|
}
|
|
}
|
|
|
|
func TestLoadConfig_EmptyPath_ReturnsDefaults(t *testing.T) {
|
|
cfg, err := LoadConfig("")
|
|
if err != nil {
|
|
t.Fatalf("empty path should not error, got %v", err)
|
|
}
|
|
if cfg.Gateway.Bind != "127.0.0.1:3110" {
|
|
t.Errorf("expected default gateway bind, got %q", cfg.Gateway.Bind)
|
|
}
|
|
}
|
|
|
|
func TestLoadConfig_MissingFile_FallsBackToDefaults(t *testing.T) {
|
|
// Per the comment in config.go: "non-empty + missing is suspicious"
|
|
// — but the contract is to log a warn and return defaults, not
|
|
// fail. We verify the contract; capturing the warn line is a
|
|
// stretch for a unit test (slog default sink is os.Stderr).
|
|
cfg, err := LoadConfig("/nonexistent/path/lakehouse.toml")
|
|
if err != nil {
|
|
t.Fatalf("missing file should not error, got %v", err)
|
|
}
|
|
if cfg.Storaged.Bind != "127.0.0.1:3211" {
|
|
t.Errorf("expected default storaged bind on missing file, got %q", cfg.Storaged.Bind)
|
|
}
|
|
}
|
|
|
|
func TestLoadConfig_ValidTOML_RoundTrip(t *testing.T) {
|
|
// Write a partial config; verify only the overridden sections
|
|
// land while the rest stay at defaults.
|
|
dir := t.TempDir()
|
|
cfgPath := filepath.Join(dir, "lakehouse.toml")
|
|
body := `[gateway]
|
|
bind = "0.0.0.0:8080"
|
|
|
|
[s3]
|
|
endpoint = "http://other-minio:9000"
|
|
bucket = "custom-bucket"
|
|
`
|
|
if err := os.WriteFile(cfgPath, []byte(body), 0o644); err != nil {
|
|
t.Fatalf("write config: %v", err)
|
|
}
|
|
|
|
cfg, err := LoadConfig(cfgPath)
|
|
if err != nil {
|
|
t.Fatalf("LoadConfig: %v", err)
|
|
}
|
|
|
|
if cfg.Gateway.Bind != "0.0.0.0:8080" {
|
|
t.Errorf("gateway.bind = %q, want 0.0.0.0:8080", cfg.Gateway.Bind)
|
|
}
|
|
if cfg.S3.Bucket != "custom-bucket" {
|
|
t.Errorf("s3.bucket = %q, want custom-bucket", cfg.S3.Bucket)
|
|
}
|
|
// Unspecified sections keep defaults (TOML decoder doesn't zero
|
|
// fields it didn't see).
|
|
if cfg.Storaged.Bind != "127.0.0.1:3211" {
|
|
t.Errorf("storaged.bind drifted to %q, want default 127.0.0.1:3211", cfg.Storaged.Bind)
|
|
}
|
|
}
|
|
|
|
func TestLoadConfig_InvalidTOML_ReturnsError(t *testing.T) {
|
|
dir := t.TempDir()
|
|
cfgPath := filepath.Join(dir, "bad.toml")
|
|
if err := os.WriteFile(cfgPath, []byte("this is = not [toml"), 0o644); err != nil {
|
|
t.Fatalf("write bad config: %v", err)
|
|
}
|
|
|
|
_, err := LoadConfig(cfgPath)
|
|
if err == nil {
|
|
t.Fatal("expected parse error on malformed TOML, got nil")
|
|
}
|
|
if !strings.Contains(err.Error(), "parse config") {
|
|
t.Errorf("error = %v, want 'parse config' wrapper", err)
|
|
}
|
|
}
|
|
|
|
func TestLoadConfig_FileButUnreadable(t *testing.T) {
|
|
// Skip on non-unix or when running as root (which can read
|
|
// 0000-permission files). We only need this case in CI/local-dev
|
|
// where test user isn't root. Per memory `feedback_pkill_scope.md`
|
|
// J's box runs many things as root; treat this as informational.
|
|
if os.Geteuid() == 0 {
|
|
t.Skip("root can read 0000 files; skipping unreadable-file case")
|
|
}
|
|
dir := t.TempDir()
|
|
cfgPath := filepath.Join(dir, "locked.toml")
|
|
if err := os.WriteFile(cfgPath, []byte("[gateway]\nbind=\":1\""), 0o000); err != nil {
|
|
t.Fatalf("write: %v", err)
|
|
}
|
|
|
|
_, err := LoadConfig(cfgPath)
|
|
if err == nil {
|
|
t.Fatal("expected read error on unreadable file, got nil")
|
|
}
|
|
if !strings.Contains(err.Error(), "read config") {
|
|
t.Errorf("error = %v, want 'read config' wrapper", err)
|
|
}
|
|
}
|