Vellum/internal/arch/arch_test.go
mithral 35442e4aff Phase 2: split the fat packages along real boundaries
Four extractions, each chosen because the two halves have different callers,
different failure modes or different testing needs. Not a line-count exercise.

internal/home out of internal/media
- media serves item RESOURCES (detail, file, cover, progress); home computes
  AGGREGATES (the five shelves). The 2328-line package was doing both.
- media keeps POST /items/{id}/view because it is item-scoped and must stay in
  the package that authorizes the item; it imports home.RecordView for that one
  thing. home does not import media.
- Verified live: all five shelves non-empty, and two consecutive /home calls
  return different explore ids, which is the non-determinism the Explore rewrite
  exists for.

internal/scanner out of internal/library
- The scanner walks the filesystem and reconciles; library CRUD is an admin HTTP
  surface. Different callers (a job vs a request), different failure modes.
- The registry deliberately STAYED in internal/library: three of its four callers
  are not the scanner (modules register types, handlers serve the list, metadata
  reads the group mode, subsonic calls CleanTitle). It is a property of the kind,
  not of the scan. The one genuinely scanner-owned helper became
  library.ExtensionsFor.
- Two tests moved with the code rather than being rewritten; they now exercise
  the boundary by calling library.ListMissing across it.
- Verified live: first scan updates, rescan reports unchanged=N, so change
  detection still short-circuits.

internal/providers out of internal/metadata
- Providers talk to third-party APIs; the enrich job owns the matching policy and
  the database writes. The split's whole point is that a provider is testable
  WITHOUT a database, so a DB-free test was added and OpenLibrary gained a
  BaseURL override to make it possible.
- providers imports zero internal packages — a true leaf.
- The matching policy (searchTitle, artistHint, pickBest) is untouched. It exists
  because real false matches were found.

dto packages out of internal/jellyfin and internal/subsonic
- The wire types and their mapping are now separate from the handler logic, so
  the contract with clients we do not control is reviewable on its own.
- Both dto packages are PURE: zero references to sql.DB or net/http, verified by
  grep. That is what makes them reviewable.
- No field name, tag or error code changed.

internal/arch did its job: it failed the build twice during this phase, once for
each new package that was not yet placed in a layer. That is the rule working,
not an obstacle.

Verified: gofmt clean, build exit 0, vet exit 0, 23 packages pass, svelte-check
0/0, and end to end against a running server — home shelves, a queued scan
reporting unchanged, Jellyfin /Items, and Subsonic ping.

Process note: one agent (dto) stopped mid-task and left the tree red — the dto
packages unlayered and its test still referencing constants that had moved. The
arch test caught it immediately. I finished it by hand: exported constant
references in the test, and both dto packages placed at layer 3.

Still open from Phase 1: internal/jellyfin has no test files, catalog is
item-centric while Subsonic is file-first, and subsonic loadAlbum now scopes to
the item's own library.
2026-10-11 16:03:10 -04:00

241 lines
6.9 KiB
Go

// Package arch holds no code. It exists so that the import rules this project
// depends on are enforced by the test suite instead of by good intentions.
//
// A layering rule that is only written down in a comment is a rule that will be
// broken within a month, and the breakage is invisible: an upward import does
// not fail to compile, it just quietly welds two layers together until nothing
// can be moved. These tests fail loudly instead.
package arch
import (
"go/parser"
"go/token"
"os"
"path/filepath"
"sort"
"strings"
"testing"
)
const modulePath = "github.com/mithral/vellum/"
// Layers, lowest first. A package may import from a LOWER layer or its own
// layer, never from a higher one. Peers are allowed because they genuinely
// collaborate (metadata enriches items that media serves); upward imports are
// not, because they invert the dependency and make the lower layer unusable on
// its own.
var layers = map[string]int{
// 0 — foundation. No knowledge of the application.
"internal/core": 0,
"internal/httpx": 0,
"internal/natsort": 0,
"internal/arch": 0,
"migrations": 0,
// 1 — primitives the application is built from.
"internal/auth": 1,
"internal/jobs": 1,
"internal/stream": 1,
"internal/progress": 1,
"internal/probe": 1,
"internal/engine": 1,
"internal/apitoken": 1,
// 2 — leaves that the application is built from: the data seam, and the
// external catalogues. providers sits here so it stays a leaf — a
// metadata client must be usable from a test with no database.
"internal/catalog": 2,
"internal/providers": 2,
// 3 — the application.
"internal/library": 3,
"internal/scanner": 3,
"internal/media": 3,
"internal/home": 3,
"internal/search": 3,
"internal/metadata": 3,
"internal/playback": 3,
"internal/modules/comics": 3,
"internal/modules/ebooks": 3,
"internal/modules/games": 3,
"internal/modules/photos": 3,
// 4 — adapters: HTTP surfaces and foreign protocols.
"internal/httpapi": 4,
"internal/jellyfin": 4,
"internal/jellyfin/dto": 3,
"internal/subsonic": 4,
"internal/subsonic/dto": 3,
"internal/cli": 4,
// 5 — the only place everything is wired together.
"cmd/vellum": 5,
}
// repoRoot walks up until it finds go.mod, so the test works regardless of
// where `go test` is invoked from.
func repoRoot(t *testing.T) string {
t.Helper()
dir, err := os.Getwd()
if err != nil {
t.Fatal(err)
}
for {
if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil {
return dir
}
parent := filepath.Dir(dir)
if parent == dir {
t.Fatal("could not find go.mod above the test directory")
}
dir = parent
}
}
// importsOf parses a directory and returns the internal packages it imports.
func importsOf(t *testing.T, dir string) []string {
t.Helper()
entries, err := os.ReadDir(dir)
if err != nil {
t.Fatalf("read %s: %v", dir, err)
}
seen := map[string]bool{}
for _, e := range entries {
if e.IsDir() || !strings.HasSuffix(e.Name(), ".go") || strings.HasSuffix(e.Name(), "_test.go") {
continue
}
fset := token.NewFileSet()
f, err := parser.ParseFile(fset, filepath.Join(dir, e.Name()), nil, parser.ImportsOnly)
if err != nil {
t.Fatalf("parse %s: %v", e.Name(), err)
}
for _, imp := range f.Imports {
p := strings.Trim(imp.Path.Value, `"`)
if strings.HasPrefix(p, modulePath) {
seen[strings.TrimPrefix(p, modulePath)] = true
}
}
}
out := make([]string, 0, len(seen))
for p := range seen {
out = append(out, p)
}
sort.Strings(out)
return out
}
// TestNoUpwardImports is the rule that keeps the layers separable.
func TestNoUpwardImports(t *testing.T) {
root := repoRoot(t)
for pkg, layer := range layers {
dir := filepath.Join(root, filepath.FromSlash(pkg))
if _, err := os.Stat(dir); err != nil {
continue // package not present (e.g. a module was removed)
}
for _, dep := range importsOf(t, dir) {
depLayer, known := layers[dep]
if !known {
continue // a package outside the map; TestEveryPackageIsLayered catches it
}
if depLayer > layer {
t.Errorf("%s (layer %d) imports %s (layer %d): upward dependency",
pkg, layer, dep, depLayer)
}
}
}
}
// TestCoreStaysPure is the module rule: core may not know about any feature, or
// it stops being the thing everything else can depend on.
func TestCoreStaysPure(t *testing.T) {
root := repoRoot(t)
for _, dep := range importsOf(t, filepath.Join(root, "internal", "core")) {
if dep != "migrations" {
t.Errorf("internal/core imports %s; core may depend only on migrations", dep)
}
}
}
// TestCatalogHasNoHTTP keeps the data seam a data seam. If it grows an
// http.ResponseWriter it can no longer be reused by a job, a CLI or a test
// without dragging a request in.
func TestCatalogHasNoHTTP(t *testing.T) {
root := repoRoot(t)
for _, dep := range importsOf(t, filepath.Join(root, "internal", "catalog")) {
if dep == "internal/httpx" || dep == "internal/httpapi" {
t.Errorf("internal/catalog imports %s; the data seam must not know about HTTP", dep)
}
}
}
// TestModulesDoNotImportEachOther keeps modules independently removable. A
// module that imports a sibling cannot be dropped without dropping both.
func TestModulesDoNotImportEachOther(t *testing.T) {
root := repoRoot(t)
modsDir := filepath.Join(root, "internal", "modules")
mods, err := os.ReadDir(modsDir)
if err != nil {
t.Skip("no modules directory")
}
for _, m := range mods {
if !m.IsDir() {
continue
}
self := "internal/modules/" + m.Name()
for _, dep := range importsOf(t, filepath.Join(modsDir, m.Name())) {
if strings.HasPrefix(dep, "internal/modules/") && dep != self {
t.Errorf("%s imports %s; modules must not depend on each other", self, dep)
}
}
}
}
// TestEveryPackageIsLayered stops the map above from silently going stale. A
// new package that nobody placed in a layer is a package with no rules, which
// is how the layering erodes.
func TestEveryPackageIsLayered(t *testing.T) {
root := repoRoot(t)
var found []string
var walk func(base string)
walk = func(base string) {
entries, err := os.ReadDir(base)
if err != nil {
return
}
for _, e := range entries {
if !e.IsDir() {
continue
}
full := filepath.Join(base, e.Name())
rel, _ := filepath.Rel(root, full)
rel = filepath.ToSlash(rel)
// Only directories that actually contain Go files are packages.
if hasGoFiles(full) {
found = append(found, rel)
}
walk(full)
}
}
walk(filepath.Join(root, "internal"))
found = append(found, "cmd/vellum")
for _, pkg := range found {
if _, ok := layers[pkg]; !ok {
t.Errorf("%s is not in the layers map; add it so the import rules apply to it", pkg)
}
}
}
func hasGoFiles(dir string) bool {
entries, err := os.ReadDir(dir)
if err != nil {
return false
}
for _, e := range entries {
if !e.IsDir() && strings.HasSuffix(e.Name(), ".go") {
return true
}
}
return false
}