Add storage tests for URL parsing and every in-process backend (closes #66)

internal/storage had no tests. This adds table-driven ParseStorageURL
coverage and a shared Storer conformance suite, defined once in
conformance_test.go and run against every backend that can run
in-process: file:// over a temp dir and s3:// over the same in-memory
S3 harness (gofakes3 + s3mem) that internal/s3 and the not-found test
use, so a new backend inherits the contract by passing its constructor.
The rclone backend gets construction and argument-shaping tests via
rclone's in-process ":local" backend, plus the missing-remote error
path; a comment records that its data-plane operations need a configured
remote and so cannot run in a unit test.

Tests only; no production code changed and no defect surfaced.

Model: opus-4-8

internal/storage/conformance_test.go

@@ -0,0 +1,198 @@
+package storage_test
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"io"
+	"reflect"
+	"sort"
+	"testing"
+
+	"sneak.berlin/go/vaultik/internal/storage"
+)
+
+// runStorerConformance is the shared Storer contract. Every backend that
+// can run in-process is expected to pass it: TestFileStorer runs it against
+// file://, TestS3Storer against s3://. A new backend inherits this coverage
+// by passing its own constructor, so the contract is defined once.
+//
+// It exercises the public Storer interface: round-trip, stat, list with
+// prefix filtering, overwrite, delete, delete-of-missing, and not-found on
+// Get and Stat. Each section takes its own fresh backend instance, so the
+// order of sections never matters and no section sees another's objects.
+func runStorerConformance(t *testing.T, newStorer func(*testing.T) storage.Storer) {
+	t.Helper()
+
+	conformanceRoundTrip(t, newStorer(t))
+	conformanceOverwrite(t, newStorer(t))
+	conformanceList(t, newStorer(t))
+	conformanceDelete(t, newStorer(t))
+	conformanceNotFound(t, newStorer(t))
+}
+
+// conformanceRoundTrip stores a nested key, then reads it back and stats it.
+func conformanceRoundTrip(t *testing.T, s storage.Storer) {
+	t.Helper()
+
+	ctx := context.Background()
+	key := "blobs/aa/bb/object.bin"
+	want := []byte("round-trip payload")
+
+	err := s.Put(ctx, key, bytes.NewReader(want))
+	if err != nil {
+		t.Fatalf("Put: %v", err)
+	}
+
+	got := getBytes(t, s, key)
+	if !bytes.Equal(got, want) {
+		t.Errorf("Get returned %q, want %q", got, want)
+	}
+
+	info, err := s.Stat(ctx, key)
+	if err != nil {
+		t.Fatalf("Stat: %v", err)
+	}
+
+	if info.Key != key {
+		t.Errorf("Stat key = %q, want %q", info.Key, key)
+	}
+
+	if info.Size != int64(len(want)) {
+		t.Errorf("Stat size = %d, want %d", info.Size, len(want))
+	}
+}
+
+// conformanceOverwrite checks that a second Put replaces the first.
+func conformanceOverwrite(t *testing.T, s storage.Storer) {
+	t.Helper()
+
+	ctx := context.Background()
+	key := "meta/snapshot.json"
+
+	err := s.Put(ctx, key, bytes.NewReader([]byte("first")))
+	if err != nil {
+		t.Fatalf("first Put: %v", err)
+	}
+
+	want := []byte("second and longer payload")
+
+	err = s.Put(ctx, key, bytes.NewReader(want))
+	if err != nil {
+		t.Fatalf("second Put: %v", err)
+	}
+
+	got := getBytes(t, s, key)
+	if !bytes.Equal(got, want) {
+		t.Errorf("after overwrite Get returned %q, want %q", got, want)
+	}
+}
+
+// conformanceList checks prefix filtering and the empty result for a
+// prefix that matches nothing.
+func conformanceList(t *testing.T, s storage.Storer) {
+	t.Helper()
+
+	ctx := context.Background()
+	keys := []string{"blobs/aa/one", "blobs/bb/two", "meta/three"}
+
+	for _, k := range keys {
+		err := s.Put(ctx, k, bytes.NewReader([]byte("data")))
+		if err != nil {
+			t.Fatalf("Put %q: %v", k, err)
+		}
+	}
+
+	if got := listSorted(t, s, ""); !reflect.DeepEqual(got, keys) {
+		t.Errorf("List(\"\") = %v, want %v", got, keys)
+	}
+
+	wantBlobs := []string{"blobs/aa/one", "blobs/bb/two"}
+	if got := listSorted(t, s, "blobs/"); !reflect.DeepEqual(got, wantBlobs) {
+		t.Errorf("List(\"blobs/\") = %v, want %v", got, wantBlobs)
+	}
+
+	if got := listSorted(t, s, "absent/"); len(got) != 0 {
+		t.Errorf("List(\"absent/\") = %v, want empty", got)
+	}
+}
+
+// conformanceDelete checks that Delete removes an object and that deleting
+// a missing key is not an error.
+func conformanceDelete(t *testing.T, s storage.Storer) {
+	t.Helper()
+
+	ctx := context.Background()
+	key := "blobs/cc/gone.bin"
+
+	err := s.Put(ctx, key, bytes.NewReader([]byte("temporary")))
+	if err != nil {
+		t.Fatalf("Put: %v", err)
+	}
+
+	err = s.Delete(ctx, key)
+	if err != nil {
+		t.Fatalf("Delete: %v", err)
+	}
+
+	_, err = s.Get(ctx, key)
+	if !errors.Is(err, storage.ErrNotFound) {
+		t.Errorf("Get after Delete error = %v, want ErrNotFound", err)
+	}
+
+	err = s.Delete(ctx, key)
+	if err != nil {
+		t.Errorf("Delete of missing key = %v, want nil", err)
+	}
+}
+
+// conformanceNotFound checks Get and Stat on an absent key.
+func conformanceNotFound(t *testing.T, s storage.Storer) {
+	t.Helper()
+
+	ctx := context.Background()
+	key := "never/written"
+
+	_, err := s.Get(ctx, key)
+	if !errors.Is(err, storage.ErrNotFound) {
+		t.Errorf("Get error = %v, want ErrNotFound", err)
+	}
+
+	_, err = s.Stat(ctx, key)
+	if !errors.Is(err, storage.ErrNotFound) {
+		t.Errorf("Stat error = %v, want ErrNotFound", err)
+	}
+}
+
+// getBytes reads a key fully and closes the reader.
+func getBytes(t *testing.T, s storage.Storer, key string) []byte {
+	t.Helper()
+
+	rc, err := s.Get(context.Background(), key)
+	if err != nil {
+		t.Fatalf("Get %q: %v", key, err)
+	}
+
+	defer func() { _ = rc.Close() }()
+
+	data, err := io.ReadAll(rc)
+	if err != nil {
+		t.Fatalf("read %q: %v", key, err)
+	}
+
+	return data
+}
+
+// listSorted returns the keys under a prefix in a stable order.
+func listSorted(t *testing.T, s storage.Storer, prefix string) []string {
+	t.Helper()
+
+	keys, err := s.List(context.Background(), prefix)
+	if err != nil {
+		t.Fatalf("List %q: %v", prefix, err)
+	}
+
+	sort.Strings(keys)
+
+	return keys
+}

internal/storage/file_backend_test.go

@@ -0,0 +1,27 @@
+package storage_test
+
+import (
+	"testing"
+
+	"sneak.berlin/go/vaultik/internal/storage"
+)
+
+// newFileStorer builds a file:// backend rooted at a fresh temp directory.
+//
+//nolint:ireturn // conformance runs against the Storer interface by design
+func newFileStorer(t *testing.T) storage.Storer {
+	t.Helper()
+
+	s, err := storage.NewFileStorer(t.TempDir())
+	if err != nil {
+		t.Fatalf("NewFileStorer: %v", err)
+	}
+
+	return s
+}
+
+// TestFileStorer runs the shared Storer contract against the file:// backend.
+func TestFileStorer(t *testing.T) {
+	t.Parallel()
+	runStorerConformance(t, newFileStorer)
+}

internal/storage/rclone_test.go

@@ -0,0 +1,58 @@
+package storage_test
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"sneak.berlin/go/vaultik/internal/storage"
+)
+
+// The rclone backend is a thin adapter over the rclone library: it turns a
+// (remote, path) pair into rclone's "remote:path" string, hands it to
+// rclone, and maps rclone's own results back to the Storer interface. What
+// can be tested in-process, without a configured remote or network, is that
+// adapter layer — how the arguments are shaped and how construction errors
+// are reported. The data-plane operations (Put/Get/List/Delete) are rclone's
+// own, exercised against a real provider (drive, s3-via-rclone, ...), which
+// needs a configured remote with credentials and network access and so is
+// out of reach of a unit test. The shared Storer conformance suite therefore
+// runs against the in-process file and s3 backends; the rclone backend
+// inherits that contract once a remote is configured.
+//
+// These tests use rclone's ":local:" on-the-fly backend, which addresses the
+// local filesystem directly without any configured remote, so construction
+// runs entirely in-process.
+
+// TestNewRcloneStorerConstruction checks that a valid remote constructs a
+// backend and that Info() reports the shaped "remote:path" location.
+//
+//nolint:paralleltest // NewRcloneStorer installs the process-global rclone config
+func TestNewRcloneStorerConstruction(t *testing.T) {
+	dir := t.TempDir()
+
+	s, err := storage.NewRcloneStorer(context.Background(), ":local", dir)
+	if err != nil {
+		t.Fatalf("NewRcloneStorer: %v", err)
+	}
+
+	// Info().Location is the "remote:path" string the adapter builds from
+	// its two arguments, so asserting it confirms the argument shaping.
+	want := ":local:" + dir
+	if got := s.Info().Location; got != want {
+		t.Errorf("Info().Location = %q, want %q", got, want)
+	}
+}
+
+// TestNewRcloneStorerUnknownRemote checks that a remote that is not in the
+// rclone config fails construction with the ErrRemoteNotFound sentinel,
+// rather than silently returning a backend pointed nowhere.
+//
+//nolint:paralleltest // NewRcloneStorer installs the process-global rclone config
+func TestNewRcloneStorerUnknownRemote(t *testing.T) {
+	_, err := storage.NewRcloneStorer(
+		context.Background(), "vaultik-no-such-remote", "path")
+	if !errors.Is(err, storage.ErrRemoteNotFound) {
+		t.Errorf("NewRcloneStorer error = %v, want ErrRemoteNotFound", err)
+	}
+}

internal/storage/s3_test.go

@@ -13,18 +13,23 @@ import (
 	"sneak.berlin/go/vaultik/internal/storage"
 )
 
-// TestS3StorerMissingKeyMapsToErrNotFound verifies that the s3 backend reports
-// a missing object as storage.ErrNotFound, matching the file and rclone
-// backends and the Storer contract. Without the mapping, Get and Stat leak the
-// raw SDK error and errors.Is(err, storage.ErrNotFound) is false.
+// s3TestBucket is the bucket created for each in-process S3 server.
+const s3TestBucket = "test-bucket"
+
+// newS3Storer builds an s3:// backend backed by a fresh in-process
+// S3 server. It reuses the same in-memory S3 harness (gofakes3 + s3mem
+// over httptest) that internal/s3 and the not-found regression test use,
+// so no new mock or dependency is introduced. Each call gets its own
+// server, bucket, and client, so the conformance suite's per-section
+// instances stay isolated.
 //
-//nolint:paralleltest // shares an in-process S3 server via t.Cleanup
-func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
-	const bucket = "test-bucket"
+//nolint:ireturn // conformance runs against the Storer interface by design
+func newS3Storer(t *testing.T) storage.Storer {
+	t.Helper()
 
 	backend := s3mem.New()
 
-	err := backend.CreateBucket(bucket)
+	err := backend.CreateBucket(s3TestBucket)
 	if err != nil {
 		t.Fatalf("create bucket: %v", err)
 	}
@@ -32,11 +37,9 @@ func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
 	srv := httptest.NewServer(gofakes3.New(backend).Server())
 	t.Cleanup(srv.Close)
 
-	ctx := context.Background()
-
-	client, err := s3.NewClient(ctx, s3.Config{
+	client, err := s3.NewClient(context.Background(), s3.Config{
 		Endpoint:        srv.URL,
-		Bucket:          bucket,
+		Bucket:          s3TestBucket,
 		AccessKeyID:     "test",
 		SecretAccessKey: "test",
 		Region:          "us-east-1",
@@ -45,9 +48,28 @@ func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
 		t.Fatalf("new client: %v", err)
 	}
 
-	storer := storage.NewS3Storer(client)
+	return storage.NewS3Storer(client)
+}
 
-	_, err = storer.Get(ctx, "does-not-exist")
+// TestS3Storer runs the shared Storer contract against the s3:// backend,
+// so it is held to the same round-trip, list, delete, and not-found
+// behaviour as the file:// backend.
+func TestS3Storer(t *testing.T) {
+	t.Parallel()
+	runStorerConformance(t, newS3Storer)
+}
+
+// TestS3StorerMissingKeyMapsToErrNotFound pins the specific contract that a
+// missing object surfaces as storage.ErrNotFound rather than the raw AWS SDK
+// error. Without the mapping, errors.Is(err, storage.ErrNotFound) is false on
+// s3 and callers would branch differently per backend.
+func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
+	t.Parallel()
+
+	storer := newS3Storer(t)
+	ctx := context.Background()
+
+	_, err := storer.Get(ctx, "does-not-exist")
 	if !errors.Is(err, storage.ErrNotFound) {
 		t.Errorf("Get on missing key: got %v, want ErrNotFound", err)
 	}

internal/storage/url_parse_test.go

@@ -0,0 +1,110 @@
+package storage_test
+
+import (
+	"errors"
+	"reflect"
+	"testing"
+
+	"sneak.berlin/go/vaultik/internal/storage"
+)
+
+// TestParseStorageURLValid checks that each supported scheme parses into
+// the expected fields, since those fields decide which backend is built.
+func TestParseStorageURLValid(t *testing.T) {
+	t.Parallel()
+
+	const bucket = "mybucket"
+
+	cases := []struct {
+		name string
+		raw  string
+		want *storage.URL
+	}{
+		{
+			name: "file absolute path",
+			raw:  "file:///var/backups/vaultik",
+			want: &storage.URL{Scheme: "file", Prefix: "/var/backups/vaultik"},
+		},
+		{
+			name: "s3 bucket and prefix, ssl defaults on",
+			raw:  "s3://mybucket/backups/host",
+			want: &storage.URL{
+				Scheme: "s3", Bucket: bucket,
+				Prefix: "backups/host", UseSSL: true,
+			},
+		},
+		{
+			name: "s3 bucket only",
+			raw:  "s3://mybucket",
+			want: &storage.URL{Scheme: "s3", Bucket: bucket, UseSSL: true},
+		},
+		{
+			name: "s3 with endpoint, region, ssl off",
+			raw:  "s3://mybucket?endpoint=minio.example.com&region=us-west-2&ssl=false",
+			want: &storage.URL{
+				Scheme: "s3", Bucket: bucket,
+				Endpoint: "minio.example.com", Region: "us-west-2", UseSSL: false,
+			},
+		},
+		{
+			name: "rclone remote and path",
+			raw:  "rclone://gdrive/backups/host",
+			want: &storage.URL{
+				Scheme: "rclone", RcloneRemote: "gdrive", Prefix: "backups/host",
+			},
+		},
+		{
+			name: "rclone remote only",
+			raw:  "rclone://gdrive",
+			want: &storage.URL{Scheme: "rclone", RcloneRemote: "gdrive"},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			got, err := storage.ParseStorageURL(tc.raw)
+			if err != nil {
+				t.Fatalf("ParseStorageURL(%q) returned error: %v", tc.raw, err)
+			}
+
+			if !reflect.DeepEqual(got, tc.want) {
+				t.Errorf("ParseStorageURL(%q) = %+v, want %+v", tc.raw, got, tc.want)
+			}
+		})
+	}
+}
+
+// TestParseStorageURLErrors checks that empty, missing, and unknown-scheme
+// inputs fail with the documented sentinel errors instead of parsing to a
+// wrong destination.
+func TestParseStorageURLErrors(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name    string
+		raw     string
+		wantErr error
+	}{
+		{"empty url", "", storage.ErrEmptyStorageURL},
+		{"file empty path", "file://", storage.ErrEmptyFilePath},
+		{"s3 missing bucket", "s3://", storage.ErrMissingBucket},
+		{"s3 missing bucket with path", "s3:///justprefix", storage.ErrMissingBucket},
+		{"rclone missing remote", "rclone://", storage.ErrMissingRemote},
+		{"unknown scheme", "gs://bucket/x", storage.ErrUnsupportedScheme},
+		{"no scheme", "/local/path", storage.ErrUnsupportedScheme},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			_, err := storage.ParseStorageURL(tc.raw)
+			if !errors.Is(err, tc.wantErr) {
+				t.Errorf("ParseStorageURL(%q) error = %v, want %v",
+					tc.raw, err, tc.wantErr)
+			}
+		})
+	}
+}