68f18424f5
Hard gate #4: codified module boundaries as Go tests, so a new contributor (or AI agent) can't silently land an import that crosses a layer. Boundaries enforced (one architecture_test.go per package): - wsauth has no internal/* deps — auth leaf, must be unit-testable in isolation - models has no internal/* deps — pure-types leaf, reverse dep would create cycles since most packages depend on models - db has no internal/* deps — DB layer below business logic, must be testable with sqlmock without spinning up handlers/provisioner - provisioner does not import handlers or router — unidirectional layering: handlers wires provisioner into HTTP routes; the reverse is a cycle Each test parses .go files in its package via go/parser (no x/tools dep needed) and asserts forbidden import paths don't appear. Failure messages name the rule, the offending file, and explain WHY the boundary exists so the diff reviewer learns the rule. Note: the original issue's first two proposed boundaries (provisioner-no-DB, handlers-no-docker) don't match the codebase today — provisioner already imports db (PR #2276 runtime-image lookup) and handlers hold *docker.Client directly (terminal, plugins, bundle, templates). I picked the four boundaries that actually hold; the first two are aspirational and would need a refactor before they could be codified. Hand-tested by injecting a deliberate wsauth -> orgtoken violation: the gate fires red with the rule message before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
64 lines
1.8 KiB
Go
64 lines
1.8 KiB
Go
package models_test
|
|
|
|
// Architecture test (#2344): models is a leaf — it carries pure type
|
|
// definitions and must not import any other internal/* package. Almost
|
|
// every package in workspace-server depends on models; if models grew a
|
|
// reverse dep, the import graph would cycle.
|
|
//
|
|
// If this test fails: you put behavior inside models. Move the behavior
|
|
// to whichever package actually owns it (handlers, provisioner, db, …)
|
|
// and have *that* package import models, not the reverse.
|
|
|
|
import (
|
|
"go/parser"
|
|
"go/token"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
const moduleInternalPrefix = "github.com/Molecule-AI/molecule-monorepo/platform/internal/"
|
|
|
|
func TestModelsHasNoInternalDependencies(t *testing.T) {
|
|
t.Parallel()
|
|
for path, file := range listImports(t, ".") {
|
|
if strings.HasPrefix(path, moduleInternalPrefix) {
|
|
t.Errorf(
|
|
"models must not import other internal packages "+
|
|
"(found %q in %s) — models is the pure-types leaf and any "+
|
|
"reverse dep creates an import cycle since most packages "+
|
|
"depend on models. See workspace-server/internal/models/architecture_test.go.",
|
|
path, file,
|
|
)
|
|
}
|
|
}
|
|
}
|
|
|
|
func listImports(t *testing.T, dir string) map[string]string {
|
|
t.Helper()
|
|
fset := token.NewFileSet()
|
|
entries, err := os.ReadDir(dir)
|
|
if err != nil {
|
|
t.Fatalf("read %s: %v", dir, err)
|
|
}
|
|
out := make(map[string]string)
|
|
for _, e := range entries {
|
|
name := e.Name()
|
|
if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
|
|
continue
|
|
}
|
|
f, err := parser.ParseFile(fset, filepath.Join(dir, name), nil, parser.ImportsOnly)
|
|
if err != nil {
|
|
t.Fatalf("parse %s: %v", name, err)
|
|
}
|
|
for _, imp := range f.Imports {
|
|
path := strings.Trim(imp.Path.Value, "\"")
|
|
if _, seen := out[path]; !seen {
|
|
out[path] = name
|
|
}
|
|
}
|
|
}
|
|
return out
|
|
}
|