diff --git a/internal/queryd/db_test.go b/internal/queryd/db_test.go new file mode 100644 index 0000000..c50535f --- /dev/null +++ b/internal/queryd/db_test.go @@ -0,0 +1,186 @@ +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) + } +} diff --git a/internal/shared/config_test.go b/internal/shared/config_test.go new file mode 100644 index 0000000..f35311b --- /dev/null +++ b/internal/shared/config_test.go @@ -0,0 +1,150 @@ +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) + } +} diff --git a/internal/shared/server_test.go b/internal/shared/server_test.go new file mode 100644 index 0000000..0019149 --- /dev/null +++ b/internal/shared/server_test.go @@ -0,0 +1,206 @@ +package shared + +import ( + "encoding/json" + "errors" + "net" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/go-chi/chi/v5/middleware" +) + +// Closes R-002: internal/shared was load-bearing-but-untested per the +// audit. These tests cover the pieces server.go exposes that DON'T +// require running Run() under a signal — bind error surfacing, JSON +// shape of /health, and the register-callback contract. + +func TestNewListener_ValidAddr(t *testing.T) { + // Port 0 = "let the OS pick" — the listener should bind cleanly. + ln, err := newListener("127.0.0.1:0") + if err != nil { + t.Fatalf("expected success on :0, got %v", err) + } + defer ln.Close() + if _, _, err := net.SplitHostPort(ln.Addr().String()); err != nil { + t.Errorf("listener returned unparseable addr %q: %v", ln.Addr(), err) + } +} + +func TestNewListener_InvalidAddr(t *testing.T) { + cases := []struct { + name string + addr string + }{ + // Note: net.Listen("tcp", "") binds an OS-picked address — NOT + // an error — so empty string is excluded here. That quirk is + // captured in TestNewListener_EmptyAddrIsValid below. + {"non-numeric port", "127.0.0.1:notaport"}, + {"port out of range", "127.0.0.1:999999"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ln, err := newListener(tc.addr) + if err == nil { + ln.Close() + t.Fatalf("expected error on %q, got success", tc.addr) + } + }) + } +} + +// Documents the net.Listen empty-string quirk so a future reader +// doesn't waste time wondering whether it should be a hard error. +// stdlib treats "" as ":0" → bind to all addrs, OS-picked port. +func TestNewListener_EmptyAddrIsValid(t *testing.T) { + ln, err := newListener("") + if err != nil { + t.Fatalf("net.Listen quirk changed: empty addr now errors with %v", err) + } + defer ln.Close() +} + +func TestNewListener_PortAlreadyInUse(t *testing.T) { + // Bind first to occupy a real port. + first, err := newListener("127.0.0.1:0") + if err != nil { + t.Fatalf("setup listener: %v", err) + } + defer first.Close() + + // Second bind to the same address should fail synchronously — + // this is the contract Run depends on per the "race-safe startup" + // comment in server.go. + second, err := newListener(first.Addr().String()) + if err == nil { + second.Close() + t.Fatalf("expected EADDRINUSE-like error, got success") + } +} + +func TestHealthResponse_JSONShape(t *testing.T) { + hr := HealthResponse{Status: "ok", Service: "test-svc"} + out, err := json.Marshal(hr) + if err != nil { + t.Fatalf("marshal: %v", err) + } + expected := `{"status":"ok","service":"test-svc"}` + if string(out) != expected { + t.Errorf("got %q, want %q", string(out), expected) + } + + // And round-trip — important because /health consumers depend on + // the field names being stable; a struct rename would break them. + var back HealthResponse + if err := json.Unmarshal(out, &back); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if back != hr { + t.Errorf("round-trip got %#v, want %#v", back, hr) + } +} + +// TestHealthHandler_Behavior reconstructs the /health handler's +// behavior in isolation — same wiring as Run uses, exercised via +// httptest.Server. Confirms the JSON shape AND the Content-Type +// header AND the service-name echo are all stable. +func TestHealthHandler_Behavior(t *testing.T) { + r := chi.NewRouter() + r.Use(middleware.RequestID) + + const svcName = "probe-svc" + r.Get("/health", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(HealthResponse{Status: "ok", Service: svcName}) + }) + + srv := httptest.NewServer(r) + defer srv.Close() + + resp, err := http.Get(srv.URL + "/health") + if err != nil { + t.Fatalf("GET /health: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want 200", resp.StatusCode) + } + if ct := resp.Header.Get("Content-Type"); !strings.HasPrefix(ct, "application/json") { + t.Errorf("Content-Type = %q, want application/json prefix", ct) + } + + var got HealthResponse + if err := json.NewDecoder(resp.Body).Decode(&got); err != nil { + t.Fatalf("decode body: %v", err) + } + if got.Status != "ok" || got.Service != svcName { + t.Errorf("body = %+v, want {Status:ok Service:%s}", got, svcName) + } +} + +// TestRegisterRoutes_CallbackInvoked verifies that the per-service +// register callback receives a chi.Router we can mount routes on. +// This is the contract every cmd//main.go relies on. +func TestRegisterRoutes_CallbackInvoked(t *testing.T) { + called := false + var capturedRouter chi.Router + cb := RegisterRoutes(func(r chi.Router) { + called = true + capturedRouter = r + r.Get("/extra", func(w http.ResponseWriter, _ *http.Request) { + w.Write([]byte("extra-route")) + }) + }) + + r := chi.NewRouter() + cb(r) + + if !called { + t.Fatal("RegisterRoutes callback was not invoked") + } + if capturedRouter == nil { + t.Fatal("callback received nil router") + } + + // Verify the route mounted via the callback is reachable. + srv := httptest.NewServer(r) + defer srv.Close() + resp, err := http.Get(srv.URL + "/extra") + if err != nil { + t.Fatalf("GET /extra: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want 200", resp.StatusCode) + } +} + +// TestRun_BindFailureSurfacedSynchronously is the audit's deepest +// concern about server.go: bind errors must come back as Run's +// return value, not be swallowed by the goroutine. We verify by +// occupying a port first, then expect the second Run call (via the +// listener factory) to fail loudly. +func TestRun_BindFailureSurfacedSynchronously(t *testing.T) { + occupier, err := newListener("127.0.0.1:0") + if err != nil { + t.Fatalf("setup listener: %v", err) + } + defer occupier.Close() + + // We don't call Run() directly because it blocks on signal; we + // test the synchronous-error path by calling newListener with the + // same addr — which is exactly what Run does first thing. + _, err = newListener(occupier.Addr().String()) + if err == nil { + t.Fatal("expected bind error on occupied port, got nil") + } + // Smoke that this is a "real" net error, not e.g. nil pointer. + var opErr *net.OpError + if !errors.As(err, &opErr) { + t.Errorf("expected *net.OpError, got %T", err) + } +} diff --git a/internal/storeclient/client_test.go b/internal/storeclient/client_test.go new file mode 100644 index 0000000..0e1a2da --- /dev/null +++ b/internal/storeclient/client_test.go @@ -0,0 +1,270 @@ +package storeclient + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// Closes R-003: storeclient was used by catalogd + vectord with zero +// tests. Coverage strategy: table-driven safeKey for the URL-escape +// edge cases; httptest.Server-backed tests for Put/Get/Delete/List +// covering both happy paths and the documented error contracts +// (404 → ErrKeyNotFound, non-200 → wrapped error with body preview). + +func TestSafeKey(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"plain segments", "a/b/c", "a/b/c"}, + {"single slash", "/", "/"}, + {"empty string", "", ""}, + {"trailing slash preserved", "pre/fix/", "pre/fix/"}, + {"space gets escaped", "a/b c/d", "a/b%20c/d"}, + {"apostrophe gets escaped", "O'Reilly/key", "O%27Reilly/key"}, + {"plus sign escaped", "a+b/c", "a+b/c"}, // PathEscape leaves + alone + {"unicode encoded", "café/x", "caf%C3%A9/x"}, + {"deep nesting", "datasets/proof_workers/abc.parquet", + "datasets/proof_workers/abc.parquet"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := safeKey(tc.in) + if got != tc.want { + t.Errorf("safeKey(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +func TestNew_TrimsTrailingSlash(t *testing.T) { + c := New("http://127.0.0.1:3211/") + if c.baseURL != "http://127.0.0.1:3211" { + t.Errorf("baseURL = %q, want trailing-slash stripped", c.baseURL) + } +} + +// httptest server that records what the client sent + can be steered +// to return a specific status code per route. +type recordingServer struct { + t *testing.T + srv *httptest.Server + gotPath string + gotMethod string + gotBody []byte + respStatus int + respBody string +} + +func newRecordingServer(t *testing.T) *recordingServer { + rs := &recordingServer{t: t, respStatus: http.StatusOK} + rs.srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rs.gotPath = r.URL.Path + (func() string { + if r.URL.RawQuery != "" { + return "?" + r.URL.RawQuery + } + return "" + })() + rs.gotMethod = r.Method + rs.gotBody, _ = io.ReadAll(r.Body) + w.WriteHeader(rs.respStatus) + if rs.respBody != "" { + _, _ = w.Write([]byte(rs.respBody)) + } + })) + t.Cleanup(rs.srv.Close) + return rs +} + +func TestPut_HappyPath(t *testing.T) { + rs := newRecordingServer(t) + c := New(rs.srv.URL) + body := []byte("hello world") + + if err := c.Put(context.Background(), "datasets/x/y.parquet", body); err != nil { + t.Fatalf("Put: %v", err) + } + + if rs.gotMethod != http.MethodPut { + t.Errorf("method = %q, want PUT", rs.gotMethod) + } + if rs.gotPath != "/storage/put/datasets/x/y.parquet" { + t.Errorf("path = %q, want /storage/put/datasets/x/y.parquet", rs.gotPath) + } + if string(rs.gotBody) != "hello world" { + t.Errorf("body bytes mismatch: got %q want %q", rs.gotBody, body) + } +} + +func TestPut_NonOKStatusReturnsWrappedError(t *testing.T) { + rs := newRecordingServer(t) + rs.respStatus = http.StatusForbidden + rs.respBody = "denied" + c := New(rs.srv.URL) + + err := c.Put(context.Background(), "k", []byte{1}) + if err == nil { + t.Fatal("expected error on 403, got nil") + } + if !strings.Contains(err.Error(), "status 403") { + t.Errorf("error = %v, want status 403 in message", err) + } +} + +func TestGet_RoundTripsBody(t *testing.T) { + rs := newRecordingServer(t) + rs.respBody = "the bytes" + c := New(rs.srv.URL) + + got, err := c.Get(context.Background(), "datasets/foo") + if err != nil { + t.Fatalf("Get: %v", err) + } + if string(got) != "the bytes" { + t.Errorf("body = %q, want 'the bytes'", got) + } + if rs.gotMethod != http.MethodGet { + t.Errorf("method = %q, want GET", rs.gotMethod) + } +} + +func TestGet_404ReturnsErrKeyNotFound(t *testing.T) { + rs := newRecordingServer(t) + rs.respStatus = http.StatusNotFound + c := New(rs.srv.URL) + + _, err := c.Get(context.Background(), "missing") + if !errors.Is(err, ErrKeyNotFound) { + t.Errorf("error = %v, want ErrKeyNotFound", err) + } +} + +func TestGet_500WrapsBodyPreview(t *testing.T) { + rs := newRecordingServer(t) + rs.respStatus = http.StatusInternalServerError + rs.respBody = "boom" + c := New(rs.srv.URL) + + _, err := c.Get(context.Background(), "k") + if err == nil { + t.Fatal("expected wrapped error on 500") + } + if !strings.Contains(err.Error(), "status 500") { + t.Errorf("error = %v, want status 500 in message", err) + } +} + +func TestDelete_204IsSuccess(t *testing.T) { + rs := newRecordingServer(t) + rs.respStatus = http.StatusNoContent + c := New(rs.srv.URL) + + if err := c.Delete(context.Background(), "k"); err != nil { + t.Fatalf("Delete: %v", err) + } + if rs.gotMethod != http.MethodDelete { + t.Errorf("method = %q, want DELETE", rs.gotMethod) + } +} + +func TestDelete_200IsSuccess(t *testing.T) { + // S3 returns 204; some compatible stores return 200. Both should + // be acceptable per the comment in client.go. + rs := newRecordingServer(t) + rs.respStatus = http.StatusOK + c := New(rs.srv.URL) + + if err := c.Delete(context.Background(), "k"); err != nil { + t.Fatalf("Delete with 200: %v", err) + } +} + +func TestDelete_400IsError(t *testing.T) { + rs := newRecordingServer(t) + rs.respStatus = http.StatusBadRequest + rs.respBody = "bad key" + c := New(rs.srv.URL) + + err := c.Delete(context.Background(), "k") + if err == nil { + t.Fatal("expected error on 400") + } +} + +func TestList_ParsesObjects(t *testing.T) { + rs := newRecordingServer(t) + rs.respBody = `{"prefix":"datasets/","objects":[ + {"Key":"datasets/a.parquet","Size":100}, + {"Key":"datasets/b.parquet","Size":200}, + {"Key":"datasets/c.parquet","Size":300} + ]}` + c := New(rs.srv.URL) + + keys, err := c.List(context.Background(), "datasets/") + if err != nil { + t.Fatalf("List: %v", err) + } + want := []string{"datasets/a.parquet", "datasets/b.parquet", "datasets/c.parquet"} + if len(keys) != len(want) { + t.Fatalf("got %d keys, want %d", len(keys), len(want)) + } + for i, k := range keys { + if k != want[i] { + t.Errorf("keys[%d] = %q, want %q", i, k, want[i]) + } + } + // And the prefix query-param made it across the wire. + if !strings.Contains(rs.gotPath, "prefix=datasets") { + t.Errorf("query path = %q, want prefix=datasets", rs.gotPath) + } +} + +func TestList_EmptyPrefix(t *testing.T) { + rs := newRecordingServer(t) + rs.respBody = `{"prefix":"","objects":[]}` + c := New(rs.srv.URL) + + keys, err := c.List(context.Background(), "") + if err != nil { + t.Fatalf("List: %v", err) + } + if len(keys) != 0 { + t.Errorf("got %d keys, want 0", len(keys)) + } +} + +func TestList_BadJSON_ReturnsDecodeError(t *testing.T) { + rs := newRecordingServer(t) + rs.respBody = "not json" + c := New(rs.srv.URL) + + _, err := c.List(context.Background(), "p") + if err == nil { + t.Fatal("expected decode error on non-JSON body") + } + if !strings.Contains(err.Error(), "list decode") { + t.Errorf("error = %v, want 'list decode' wrapper", err) + } +} + +func TestPut_ContextCancellation(t *testing.T) { + rs := newRecordingServer(t) + c := New(rs.srv.URL) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() // pre-cancel — request should fail without hitting server + + err := c.Put(ctx, "k", []byte{1}) + if err == nil { + t.Fatal("expected error from canceled context") + } + if !errors.Is(err, context.Canceled) { + t.Errorf("error = %v, want context.Canceled-wrapped", err) + } +}