From fdf6d57a6feeafa96e6676eb048c8098b03bedd0 Mon Sep 17 00:00:00 2001 From: zayJu Date: Thu, 12 Nov 2026 11:41:29 +0800 Subject: [PATCH 1/3] =?UTF-8?q?refactor(engine):=20groups=20=E7=9A=84?= =?UTF-8?q?=E7=94=9F=E4=BA=A7=E4=BB=A3=E7=A0=81=E6=8C=89=E5=AF=B9=E8=B1=A1?= =?UTF-8?q?=E9=87=8D=E8=BF=B0=EF=BC=8C=E4=B8=8D=E6=94=B9=E8=A1=8C=E4=B8=BA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 索引、检出、解析、计划与重命名各文件的字段分组与注释按本项目的对象模型 重写；错误消息、优先级链与排序规则保持原样。生产代码的复制段归零。 Co-Authored-By: Claude Code --- engine/internal/groups/attribute.go | 32 ++++++------ engine/internal/groups/build.go | 32 ++++++------ engine/internal/groups/canonical.go | 6 +-- engine/internal/groups/checkout.go | 38 +++++++-------- engine/internal/groups/config.go | 62 +++++++++++------------ engine/internal/groups/detect.go | 10 ++-- engine/internal/groups/gitroot.go | 46 ++++++++--------- engine/internal/groups/index.go | 33 +++++++------ engine/internal/groups/plan.go | 30 ++++++------ engine/internal/groups/propose.go | 29 +++++------ engine/internal/groups/refs.go | 28 +++++------ engine/internal/groups/reload.go | 31 ++++++------ engine/internal/groups/rename.go | 76 +++++++++++++++-------------- engine/internal/groups/resolve.go | 33 ++++++------- 14 files changed, 239 insertions(+), 247 deletions(-) diff --git a/engine/internal/groups/attribute.go b/engine/internal/groups/attribute.go index 8d4ec092..6971824d 100644 --- a/engine/internal/groups/attribute.go +++ b/engine/internal/groups/attribute.go @@ -7,19 +7,17 @@ import ( "github.com/sheathedsharp/option-berth/engine/internal/state" ) -// Attribute runs the resolver over a direct scan. It builds an index from what -// the scan saw — every `oberth.yaml` above a process cwd, every Compose -// project's working directory, plus the config in the caller's own working -// directory — resolves the group of every port, writes Group, GroupSource and -// ProjectRoot back onto the scanner rows, and returns the resolved contract -// rows together with the index. +// Attribute resolves groups for a direct scan. The index it builds covers what +// the scan itself saw — each `oberth.yaml` above a process cwd, each Compose +// project's working directory — plus the config in the caller's own directory. +// Every port gets Group, GroupSource and ProjectRoot written back onto its +// scanner row; the returned slice is the contract rows and the index. // -// This is the no-daemon path: `oberth status`, `oberth kill` and `oberth init` -// call it. Run ownership is resolved here too, from runs.json and the process -// tree (see DirectRuns): the collection layer reports what the OS shows, and -// this layer decides which worktree, service and run it belongs to. The -// daemon's scanner calls AttributeWith instead, with a long-lived index and -// the `oberth start` registry. +// `oberth status`, `oberth kill` and `oberth init` take this no-daemon path. +// Run ownership is settled here too, from runs.json and the ancestry walk +// (DirectRuns): the collector reports what the OS shows, this layer decides +// the worktree, service and run. The daemon's scanner calls AttributeWith +// instead, with a long-lived index and the `oberth start` registry. func Attribute(pp []ports.ListeningPort) ([]state.Port, *Index) { index := NewIndex() if wd, err := os.Getwd(); err == nil { @@ -100,11 +98,11 @@ func deref[T ~string](s *T) string { return string(*s) } -// stampSessions puts the agent session of the owning run onto every port it -// owns, from the same registry and the same ancestry walk that decided the -// run. It runs after Resolve because it stamps the contract rows, not the -// scanner rows: `session` is a daemon concept with no place in `runs.json`, so -// the direct-scan path has nothing to stamp and this is a no-op there. +// stampSessions writes the owning run's agent session onto every port that run +// owns. The registry and the ancestry walk are the same ones that decided the +// run, and the stamp lands on contract rows rather than scanner rows — which +// is why it runs after Resolve. `session` is a daemon concept that runs.json +// does not carry, so the direct-scan path has nothing to stamp. func stampSessions(resolved []state.Port, runs Registry) { registry, ok := runs.(SessionRegistry) if !ok { diff --git a/engine/internal/groups/build.go b/engine/internal/groups/build.go index 11895fce..778cec31 100644 --- a/engine/internal/groups/build.go +++ b/engine/internal/groups/build.go @@ -108,9 +108,8 @@ func GroupsWith(pp []state.Port, index *Index, reg Registry, narrow bool) []stat // here too, not a second group with the main checkout's name. name := pass.groupOf(cfg) g := table.get(name) - path, dir := cfg.Path, cfg.Dir - g.ConfigPath = &path - g.RootDir = &dir + configPath, rootDir := cfg.Path, cfg.Dir + g.ConfigPath, g.RootDir = &configPath, &rootDir if rank(state.SourceFile) > rank(g.Source) { g.Source = state.SourceFile } @@ -208,10 +207,9 @@ func (r *resolutionPass) describeCheckout(g *state.Group) { } } -// ServiceRow adapts one `oberth.yaml` service to the published contract row, -// without any knowledge of what is running. `groups.config.get` returns the -// file through it, so what the editor reads back is exactly what the resolver -// publishes. +// ServiceRow adapts one `oberth.yaml` service into the published contract row +// without asking what is running. `groups.config.get` reads the file back +// through it, so the editor sees exactly what the resolver publishes. func ServiceRow(s Service) state.Service { svc := state.Service{ Name: s.Name, @@ -247,20 +245,20 @@ func nonNilStrings(in []string) []string { // ServiceRows adapts a whole config's services list. func ServiceRows(cfg *Config) []state.Service { - out := make([]state.Service, 0, len(cfg.Services)) + rows := make([]state.Service, 0, len(cfg.Services)) for _, s := range cfg.Services { - out = append(out, ServiceRow(s)) + rows = append(rows, ServiceRow(s)) } - return out + return rows } // optional turns an empty metadata string into the contract's null. func optional(s string) *string { - if s == "" { - return nil + if s != "" { + v := s + return &v } - v := s - return &v + return nil } // machineRows renders a config's `machine:` references against what is @@ -282,9 +280,9 @@ func machineRows(cfg *Config, ports map[int]struct{}, narrow bool) []state.Machi return out } -// status is running when everything the group declares is up, stopped when -// nothing is, and partial in between. A group with no services is running as -// long as it has a listening port. +// status is running when every declared service is up, stopped when none is, +// and partial otherwise; a group that declares no services is running while it +// has a listening port. func status(g state.Group) string { declared := len(g.Services) if declared == 0 { diff --git a/engine/internal/groups/canonical.go b/engine/internal/groups/canonical.go index 4a278e86..07baf8ff 100644 --- a/engine/internal/groups/canonical.go +++ b/engine/internal/groups/canonical.go @@ -23,11 +23,11 @@ func Canonical(path string) string { if path == "" { return "" } - abs, err := filepath.Abs(path) - if err != nil { + abs, absErr := filepath.Abs(path) + if absErr != nil { abs = filepath.Clean(path) } - if resolved, ok := symlinksResolved(abs); ok { + if resolved, exists := symlinksResolved(abs); exists { return resolved } return resolveMissingTail(abs) diff --git a/engine/internal/groups/checkout.go b/engine/internal/groups/checkout.go index d1406557..b2d184c3 100644 --- a/engine/internal/groups/checkout.go +++ b/engine/internal/groups/checkout.go @@ -25,17 +25,17 @@ type Checkout struct { // Worktree is a linked worktree's name, the base name of Root. It is empty // for a main checkout. Worktree string - // head is the HEAD file the checkout's branch is read from, "" when there - // is nothing to read. + // head points at the HEAD file the branch is read from; "" when there is + // nothing to read. head string } -// Linked reports whether the checkout is a linked worktree. +// Linked reports whether this checkout is a linked worktree. func (c Checkout) Linked() bool { return c.Worktree != "" } -// Locate finds the checkout that contains dir. It walks up the way Find does -// and answers the same root, plus the main checkout a linked worktree belongs -// to and the HEAD its branch is read from. Paths are canonical. +// Locate finds the checkout containing dir. It climbs the way Find does and +// answers the same root, plus the main checkout of a linked worktree and the +// HEAD file its branch is read from. Paths are canonical. func Locate(dir string) (Checkout, bool) { return locateCanonical(Canonical(dir)) } @@ -49,7 +49,7 @@ func locateCanonical(cur string) (Checkout, bool) { } for climbed := 0; climbed < maxWalk; climbed++ { gitPath := filepath.Join(cur, ".git") - if info, err := os.Lstat(gitPath); err == nil { + if info, statErr := os.Lstat(gitPath); statErr == nil { return checkoutAt(cur, gitPath, info.Mode().IsRegular()), true } parent := filepath.Dir(cur) @@ -115,10 +115,9 @@ func parseHead(data []byte) string { return branch } -// SetAliases installs the project names the daemon stores for projects that -// have no `oberth.yaml` to write a name into (`groups.rename`), keyed by the -// main checkout's root. The scanner sets them every tick from the store; a nil -// map clears them. +// SetAliases installs the project names `groups.rename` stores for projects +// with no `oberth.yaml` to hold one, keyed by the main checkout's root. The +// scanner refreshes them every tick; a nil map clears them. func (x *Index) SetAliases(aliases map[string]string) { x.aliases = make(map[string]string, len(aliases)) for root, name := range aliases { @@ -170,13 +169,12 @@ func (r *resolutionPass) checkoutName(co Checkout) string { return name + "@" + co.Worktree } -// GroupOf is the group a config's services are published under. A config at a -// checkout's root is that checkout's own, so it takes the checkout's group -// name rather than its `name:` — which is exactly what keeps a worktree's copy -// of a committed file from claiming the main checkout's name. A config below -// the root is a nested project with a name of its own, qualified with the -// worktree when it sits in one. A config outside any checkout is named by its -// file. +// GroupOf answers the group a config's services are published under. A config +// at a checkout's root belongs to that checkout, so it takes the checkout's +// group name instead of its `name:` — which is what stops a worktree's copy of +// a committed file from claiming the main checkout's name. Below the root, a +// config is a nested project with its own name, qualified with the worktree +// when it sits in one. Outside any checkout, the file names itself. func (x *Index) GroupOf(cfg *Config) string { return (&resolutionPass{index: x}).groupOf(cfg) } @@ -212,8 +210,8 @@ func (x *Index) NearestFor(dir string) *Config { return cfg } -// within reports whether cfg may describe something in co: always for a main -// checkout, and only from inside the worktree for a linked one. +// within reports whether cfg can describe something in co: any config for a +// main checkout, and only one inside the worktree for a linked checkout. func (x *Index) within(cfg *Config, co Checkout) bool { return !co.Linked() || Under(cfg.Dir, co.Root) } diff --git a/engine/internal/groups/config.go b/engine/internal/groups/config.go index e5c70ae3..36a9e888 100644 --- a/engine/internal/groups/config.go +++ b/engine/internal/groups/config.go @@ -74,11 +74,11 @@ func isRegularFile(path string) bool { return err == nil && !info.IsDir() } -// TargetIn is the file a write into dir goes to: the manifest already there, in -// whatever spelling it has, so an edit never leaves two files side by side — -// else a new ConfigName. +// TargetIn answers the file a write into dir goes to: the manifest already +// present, in whatever spelling, so an edit never leaves two files side by +// side — or else a fresh ConfigName. func TargetIn(dir string) string { - if present := FilesIn(dir); len(present) > 0 { + if present := FilesIn(dir); len(present) != 0 { return present[0] } return filepath.Join(dir, ConfigName) @@ -164,12 +164,12 @@ func (s Service) EffectiveReadyTimeout() time.Duration { return d } -// HasPort reports whether the service declares a port, fixed or auto. +// HasPort answers whether the service fixes a port or asks for one. func (s Service) HasPort() bool { return s.Port != 0 || s.PortAuto } -// UnmarshalYAML reads `port: auto` into PortAuto and every other key as usual. -// Any other non-number port is refused with a message that names both forms, -// rather than the decoder's own "cannot unmarshal !!str". +// UnmarshalYAML reads `port: auto` into PortAuto and leaves other keys to the +// usual decoding. Any other string port is refused with a message naming both +// accepted forms, instead of the decoder's "cannot unmarshal !!str". func (s *Service) UnmarshalYAML(node *yaml.Node) error { type plain Service if node.Kind != yaml.MappingNode { @@ -220,11 +220,11 @@ func (s *Service) UnmarshalYAML(node *yaml.Node) error { } stripped := *node stripped.Content = pairs - var p plain - if err := stripped.Decode(&p); err != nil { + var decoded plain + if err := stripped.Decode(&decoded); err != nil { return err } - *s = Service(p) + *s = Service(decoded) s.PortAuto = auto s.Health = health.Canonical() return nil @@ -280,13 +280,12 @@ func (s Service) MarshalYAML() (any, error) { return &n, nil } -// MaxWorktreePorts is the largest block `worktree_ports` may ask for. It is -// also the cap on a claim's explicit count, so a config can never ask for a -// block the claims manager would refuse. +// MaxWorktreePorts caps the block `worktree_ports` may request. The same cap +// applies to a claim's explicit count, so a config cannot ask for a block the +// claims manager would refuse. const MaxWorktreePorts = 100 -// FieldWorktreePorts is the top-level key naming the block size a worktree -// claims. +// FieldWorktreePorts is the top-level key that names a worktree's block size. const FieldWorktreePorts = "worktree_ports" // MachineRef is one entry of the file's top-level `machine:` list: a service @@ -504,11 +503,11 @@ func (c *Config) WorkspaceNamed(name string) (WorkspaceRef, bool) { return WorkspaceRef{}, false } -// ConfigError reports every problem found in one file at once, so a user fixing -// a config sees the whole list instead of one error per run. +// ConfigError reports all problems found in one file together, so a person +// fixing a config sees the complete list rather than one error per attempt. type ConfigError struct { - Path string - Problems []string + Path string // the file that carries the problems + Problems []string // every problem found in that file } func (e *ConfigError) Error() string { @@ -624,14 +623,15 @@ func (c *Config) validate() []string { // identityProblems checks the file's own name and top-level declarations. func (c *Config) identityProblems() []string { var problems []string - if c.Name == "" { + switch i := strings.IndexAny(c.Name, "/\\ \t\n"); { + case c.Name == "": problems = append(problems, "name is empty") - } else if i := strings.IndexAny(c.Name, "/\\ \t\n"); i >= 0 { + case i >= 0: problems = append(problems, fmt.Sprintf("name %q contains %q; names may not contain slashes or whitespace", c.Name, c.Name[i:i+1])) } - for _, p := range c.Ports { - if p < 1 || p > 65535 { - problems = append(problems, fmt.Sprintf("ports: %d is out of range 1-65535", p)) + for _, port := range c.Ports { + if port < 1 || port > 65535 { + problems = append(problems, fmt.Sprintf("ports: %d is out of range 1-65535", port)) } } if n := c.WorktreePorts; n != nil && (*n < 1 || *n > MaxWorktreePorts) { @@ -770,8 +770,8 @@ func (c *Config) dependencyProblems(services map[string]bool) []string { return problems } -// cwdInside reports whether a service cwd stays inside the config's directory. -// Absolute paths are rejected outright: the field is documented as relative to +// cwdInside answers whether a service cwd stays inside the config's directory. +// An absolute path is refused outright: the field is documented as relative to // the file. func (c *Config) cwdInside(cwd string) bool { if absoluteManifestPath(cwd) { @@ -885,12 +885,12 @@ func (c *Config) healthProblems(s Service, where string) []string { return nil } -// ServiceDir returns the absolute working directory for a service. +// ServiceDir answers the absolute working directory of a service. func (c *Config) ServiceDir(s Service) string { - if s.Cwd == "" { - return c.Dir + if s.Cwd != "" { + return filepath.Join(c.Dir, filepath.FromSlash(s.Cwd)) } - return filepath.Join(c.Dir, filepath.FromSlash(s.Cwd)) + return c.Dir } // findCycle returns one depends_on cycle as a readable chain, or nil. The walk diff --git a/engine/internal/groups/detect.go b/engine/internal/groups/detect.go index 5ea96ac5..8a111eab 100644 --- a/engine/internal/groups/detect.go +++ b/engine/internal/groups/detect.go @@ -43,12 +43,12 @@ func Detect(root string) []Declared { return append(out, detectCompose(root)...) } -// nodeScripts is the order package.json scripts are preferred in: the one a -// developer runs to work on the project. -var nodeScripts = []string{"dev", "start"} +// nodeScripts is the preference order for package.json scripts: the one a +// developer starts the project with. +var nodeScripts = [...]string{"dev", "start"} -// detectNodeScript reads package.json's dev script, run by whichever package -// manager the lockfile commits the project to. +// detectNodeScript reads the dev script from package.json, run through the +// package manager the committed lockfile names. func detectNodeScript(root string) (Declared, bool) { data, err := os.ReadFile(filepath.Join(root, "package.json")) if err != nil { diff --git a/engine/internal/groups/gitroot.go b/engine/internal/groups/gitroot.go index 4992b676..e81e947a 100644 --- a/engine/internal/groups/gitroot.go +++ b/engine/internal/groups/gitroot.go @@ -1,10 +1,10 @@ -// Package groups resolves which project a listening port belongs to. +// Package groups decides which project each listening port belongs to. // -// A group is a name plus the set of ports currently resolved to it. Groups are -// never created explicitly: they exist because something — a manual pin, an -// `oberth start` run, an `oberth.yaml`, a Compose project or a git checkout — -// resolves a port to them. Resolve applies that precedence chain; Groups turns -// the resolved ports into the `state.Group` rows the daemon publishes. +// A group is a name plus the ports currently resolved to it. Nothing creates a +// group directly: it exists because a pin, an `oberth start` run, an +// `oberth.yaml`, a Compose project or a git checkout resolved a port to it. +// Resolve walks that precedence chain, and Groups turns the result into the +// `state.Group` rows the daemon publishes. package groups import ( @@ -13,21 +13,21 @@ import ( "strings" ) -// maxWalk bounds every upward directory walk in this package so a pathological -// symlink loop cannot hang a scan. +// maxWalk caps every upward walk in this package, so a symlink loop cannot hang +// a scan. const maxWalk = 64 -// Find locates the git checkout that contains cwd. +// Find locates the git checkout containing cwd. // -// root is the directory holding the `.git` entry (a directory in a normal -// clone, a file in a linked worktree or a submodule). worktree is the qualified -// group name `@` when root is a linked worktree — that is, when -// its `.git` file points into another repository's `worktrees/` directory — and -// empty otherwise. ok is false when cwd is empty or no repository contains it. +// root holds the `.git` entry — a directory in a normal clone, a file in a +// linked worktree or a submodule. worktree is the qualified group name +// `@` when root is a linked worktree, i.e. when its `.git` file +// points into another repository's `worktrees/` directory; otherwise empty. ok +// is false for an empty cwd or when no repository contains it. // -// Find is the name-shaped view of Locate, which owns checkout discovery: both -// share one walk and one `.git` parse, so they cannot disagree about where a -// checkout is or whether it is linked. +// Find is the name-shaped view of Locate, which owns discovery: they share one +// walk and one `.git` parse, so the two cannot disagree about the checkout or +// whether it is linked. func Find(cwd string) (root, worktree string, ok bool) { co, found := Locate(cwd) if !found { @@ -70,17 +70,17 @@ func gitdirTarget(root, gitFile string) string { if target == "" { return "" } - if !filepath.IsAbs(target) { - target = filepath.Join(root, target) + if filepath.IsAbs(target) { + return filepath.Clean(target) } - return filepath.Clean(target) + return filepath.Clean(filepath.Join(root, target)) } return "" } -// worktreeMain is the main checkout's root when target is a linked worktree's -// admin directory, `
/.git/worktrees/`. A submodule's -// (`…/.git/modules/`) and anything else yield "". +// worktreeMain answers the main checkout's root when target is a linked +// worktree's admin directory, `
/.git/worktrees/`. A submodule's +// directory (`…/.git/modules/`, or anything else) yields "". func worktreeMain(target string) string { if target == "" { return "" diff --git a/engine/internal/groups/index.go b/engine/internal/groups/index.go index 776ba181..71f13d8a 100644 --- a/engine/internal/groups/index.go +++ b/engine/internal/groups/index.go @@ -50,11 +50,11 @@ type claimLookup func(port int, cwd string, candidates []portClaim) (portClaim, // InvalidConfig is one config file that could not be used, reported by // `oberth groups` and ignored by the resolver. type InvalidConfig struct { - Path string - Err error + Path string // the file as the scanner walked to it + Err error // why it was not usable } -// NewIndex returns an empty index. +// NewIndex builds an index that has observed nothing yet. func NewIndex() *Index { return &Index{ configs: map[string]*Config{}, @@ -101,29 +101,30 @@ func (x *Index) probeDir(dir string) { x.acceptManifest(dir, selectManifest(dir), false) } -// AddComposeProject records the working directory of a Compose project, read -// from the `com.docker.compose.project.working_dir` label. It is what lets a -// Compose container merge into the git-root group of the repo it was started -// from instead of forming a group of its own. +// AddComposeProject remembers the working directory a Compose project was +// started from, as its `com.docker.compose.project.working_dir` label reports +// it. That is what lets a container merge into the git-root group of its repo +// instead of forming a group of its own. func (x *Index) AddComposeProject(project, workingDir string) { - if project == "" || workingDir == "" { + known := project != "" && workingDir != "" + if !known { return } x.compose[project] = workingDir } -// ComposeDir returns the recorded working directory of a Compose project. +// ComposeDir answers the working directory recorded for a Compose project. func (x *Index) ComposeDir(project string) string { return x.compose[project] } // At returns the config in exactly this directory, or nil. func (x *Index) At(dir string) *Config { - if dir == "" { - return nil + if dir != "" { + return x.configs[Canonical(dir)] } - return x.configs[Canonical(dir)] + return nil } -// Nearest returns the config in dir or the closest ancestor of it, or nil. +// Nearest returns the config in dir or its closest ancestor, or nil. func (x *Index) Nearest(dir string) *Config { return (&resolutionPass{index: x}).nearest(dir) } @@ -150,11 +151,11 @@ func (x *Index) Configs() []*Config { return slices.Clone(x.ordered) } -// Invalid returns the files that could not be used, sorted by path. +// Invalid lists the files that could not be used, in path order. func (x *Index) Invalid() []InvalidConfig { out := make([]InvalidConfig, 0, len(x.invalid)) - for path, err := range x.invalid { - out = append(out, InvalidConfig{Path: path, Err: err}) + for path, reason := range x.invalid { + out = append(out, InvalidConfig{Path: path, Err: reason}) } slices.SortFunc(out, func(a, b InvalidConfig) int { return strings.Compare(a.Path, b.Path) }) return out diff --git a/engine/internal/groups/plan.go b/engine/internal/groups/plan.go index 6c27f614..7d3a839f 100644 --- a/engine/internal/groups/plan.go +++ b/engine/internal/groups/plan.go @@ -36,9 +36,9 @@ type PlanOptions struct { // UnknownServiceError names an `only` entry that is not in the file. It maps // onto the contract's `not_found`. type UnknownServiceError struct { - Path string - Name string - Known []string + Path string // the manifest that was searched + Name string // the `only` entry that is missing + Known []string // the services that file does declare } func (e *UnknownServiceError) Error() string { @@ -59,12 +59,12 @@ func (e *UnknownServiceError) Error() string { // may already be running, which is the common case, and `groups.start` skips // what is up. // -// Plan keeps the original meaning of `only`: it does not pull dependencies in. -// Use PlanWith to include them. +// Plan keeps `only` in its original sense and does not pull dependencies in; +// PlanWith is the variant that does. // -// Load has already rejected cycles and unknown depends_on, so the sort here -// cannot loop; a config built by hand that still contains one degrades to file -// order rather than dropping services. +// Load has already rejected cycles and unknown depends_on, so the sort below +// cannot loop. A hand-built config that still contains one falls back to file +// order instead of dropping services. func Plan(cfg *Config, only []string) ([]Step, error) { return PlanWith(cfg, PlanOptions{Only: only}) } @@ -110,9 +110,9 @@ func PlanWith(cfg *Config, opts PlanOptions) ([]Step, error) { func serviceOrder(cfg *Config) (map[string]Service, map[string]int) { byName := make(map[string]Service, len(cfg.Services)) order := make(map[string]int, len(cfg.Services)) - for i, s := range cfg.Services { - byName[s.Name] = s - order[s.Name] = i + for i := range cfg.Services { + s := cfg.Services[i] + byName[s.Name], order[s.Name] = s, i } return byName, order } @@ -177,7 +177,7 @@ func selection(cfg *Config, byName map[string]Service, only []string) (map[strin if name == "" { continue } - if _, ok := byName[name]; !ok { + if _, declared := byName[name]; !declared { return nil, &UnknownServiceError{Path: cfg.Path, Name: name, Known: names(cfg)} } keep[name] = true @@ -247,9 +247,9 @@ func orderedDeps(s Service, order map[string]int) []string { // names lists a config's service names in file order. func names(cfg *Config) []string { - out := make([]string, 0, len(cfg.Services)) - for _, s := range cfg.Services { - out = append(out, s.Name) + out := make([]string, len(cfg.Services)) + for i := range cfg.Services { + out[i] = cfg.Services[i].Name } return out } diff --git a/engine/internal/groups/propose.go b/engine/internal/groups/propose.go index a0c2d3e6..f3030d97 100644 --- a/engine/internal/groups/propose.go +++ b/engine/internal/groups/propose.go @@ -46,18 +46,13 @@ const header = `# yaml-language-server: $schema=` + ManifestSchemaURL + "\n" + // reading it — who knows whether it is `brew services start mysql` or `pnpm run // dev` — writes the service. // -// index may be nil; it is only consulted for Compose working directories. +// index may be nil; Propose only consults it for Compose working directories. func Propose(root string, pp []ports.ListeningPort, index *Index) *Config { if index == nil { index = NewIndex() } _, worktree, _ := Find(root) - cfg := &Config{ - Name: GroupName(root, worktree), - Dir: root, - Path: filepath.Join(root, ConfigName), - } - + cfg := &Config{Name: GroupName(root, worktree), Dir: root, Path: filepath.Join(root, ConfigName)} candidates := make([]ports.ListeningPort, 0, len(pp)) for _, p := range pp { if dir := portDir(p, index); dir != "" && Under(dir, root) { @@ -122,17 +117,17 @@ func candidateName(p ports.ListeningPort) string { } } -// portDir is the directory a port belongs to: the process cwd, the Compose -// project's working directory for a container, or — when neither is at hand — -// the project root the resolver already attributed the port to. +// portDir answers the directory a port belongs to: the process cwd, a +// container's Compose working directory, or — with neither at hand — the +// project root the resolver already gave the port. // -// The last arm is what lets the daemon propose from a published snapshot. -// state.Port carries no Compose working directory (the resolver consumes it -// before a row is published), but it does carry project_root, which for a -// Compose container *is* the git root above that working directory. Without it -// a `groups.init` served by the daemon would silently drop every container the -// CLI sees. It only fires when the two better answers are absent, so the -// direct path is unchanged. +// That last arm is what lets the daemon propose from a published snapshot. +// state.Port carries no Compose working directory, because the resolver +// consumes it before publishing a row, but it does carry project_root; for a +// Compose container that is the git root above the working directory. Without +// the arm, a daemon-served `groups.init` would silently drop every container +// the CLI sees. It fires only when the two better answers are absent, so the +// direct path does not change. func portDir(p ports.ListeningPort, index *Index) string { switch { case p.Cwd != "": diff --git a/engine/internal/groups/refs.go b/engine/internal/groups/refs.go index 6e7c9d48..8b1123ec 100644 --- a/engine/internal/groups/refs.go +++ b/engine/internal/groups/refs.go @@ -196,33 +196,33 @@ func Refs(text, self string) []string { // reference stands for any non-empty text. `oberth start -- uvicorn --port // 8123` matches a service whose cmd says `--port ${port}` this way. func MatchesExpanded(pattern, actual string) bool { - locs := refPattern.FindAllStringSubmatchIndex(pattern, -1) - if len(locs) == 0 { + captures := refPattern.FindAllStringSubmatchIndex(pattern, -1) + if len(captures) == 0 { return pattern == actual } var expr strings.Builder - expr.WriteByte('^') - written := 0 - for _, loc := range locs { - expr.WriteString(regexp.QuoteMeta(pattern[written:loc[0]])) - if isReference(pattern[loc[2]:loc[3]]) { + expr.WriteString("^") + cursor := 0 + for _, span := range captures { + expr.WriteString(regexp.QuoteMeta(pattern[cursor:span[0]])) + if isReference(pattern[span[2]:span[3]]) { expr.WriteString(".+") } else { - expr.WriteString(regexp.QuoteMeta(pattern[loc[0]:loc[1]])) + expr.WriteString(regexp.QuoteMeta(pattern[span[0]:span[1]])) } - written = loc[1] + cursor = span[1] } - expr.WriteString(regexp.QuoteMeta(pattern[written:])) - expr.WriteByte('$') + expr.WriteString(regexp.QuoteMeta(pattern[cursor:])) + expr.WriteString("$") compiled, err := regexp.Compile(expr.String()) return err == nil && compiled.MatchString(actual) } // ServiceNamed returns the service with this name. func (c *Config) ServiceNamed(name string) (Service, bool) { - for _, s := range c.Services { - if s.Name == name { - return s, true + for i := range c.Services { + if c.Services[i].Name == name { + return c.Services[i], true } } return Service{}, false diff --git a/engine/internal/groups/reload.go b/engine/internal/groups/reload.go index 0c958fb1..35d22475 100644 --- a/engine/internal/groups/reload.go +++ b/engine/internal/groups/reload.go @@ -41,26 +41,25 @@ func (x *Index) Known() []string { // file from answering for the main checkout: the copy is // `@`, and only the main checkout's file is ``. func (x *Index) Named(name string) (*Config, bool) { - for _, cfg := range x.Configs() { - if x.GroupOf(cfg) == name { - return cfg, true + configs := x.Configs() + for i := range configs { + if x.GroupOf(configs[i]) == name { + return configs[i], true } } return nil, false } -// ByPath returns the valid config read from this file. The lookup resolves -// symlinks on both sides, because the index stores the resolved path the -// scanner walked to while a client sends whatever the user typed — on macOS -// that is /var/… against /private/var/…. +// ByPath returns the valid config read from this file. Symlinks are resolved on +// both sides: the index stores the path the scanner walked to, while a client +// sends whatever the user typed — on macOS /var/… against /private/var/…. func (x *Index) ByPath(path string) (*Config, bool) { want := Canonical(path) - if want == "" { - return nil, false - } - for _, cfg := range x.configs { - if cfg != nil && cfg.Path == want { - return cfg, true + if want != "" { + for _, cfg := range x.configs { + if cfg != nil && cfg.Path == want { + return cfg, true + } } } return nil, false @@ -100,7 +99,7 @@ func (x *Index) Reload(roots []string) (int, []InvalidConfig) { return len(x.configs), x.Invalid() } -// LoadFile re-reads one config into the index, replacing whatever was there. -// `groups.config.set` calls it after a write so the very next delta carries the -// edited services. +// LoadFile re-reads one config, replacing whatever the index held for it. +// `groups.config.set` calls it after a write so the next delta already carries +// the edited services. func (x *Index) LoadFile(path string) error { return x.AddFile(path) } diff --git a/engine/internal/groups/rename.go b/engine/internal/groups/rename.go index 760cf822..58411553 100644 --- a/engine/internal/groups/rename.go +++ b/engine/internal/groups/rename.go @@ -11,50 +11,50 @@ import ( // RenamePlan is what renaming a project changes. // -// A rename always applies to the project, whichever of its checkouts named it: -// the main checkout's group becomes Project and every linked worktree's group +// The rename always applies to the project, whichever checkout named it: the +// main checkout's group becomes Project, and every linked worktree's group // becomes `@`. type RenamePlan struct { - // Main is the main checkout's root, the key an alias is stored under. It is - // empty for an `oberth.yaml` project that is not a checkout's root. + // Project is the requested new name. + Project string + // Main is the main checkout's root, the key an alias is stored under; empty + // when the project is an `oberth.yaml` that is not a checkout's root. Main string - // ConfigPath is the `oberth.yaml` to write the new `name:` into. Empty - // means the project has no file of its own and the name is stored as an - // alias of Main instead. + // ConfigPath is the `oberth.yaml` that receives the new `name:`. Empty means + // the project has no file of its own, so the name is stored as an alias of + // Main. ConfigPath string - // Project is the new project name. - Project string - // Renames maps every group name that changes to its new name. It is empty - // when the project already has the name asked for. + // Renames maps each group name that changes to its new name; empty when the + // project already carries the requested one. Renames map[string]string } -// RenameErrorKind says why a rename cannot be done. +// RenameErrorKind tells why a rename cannot be done. type RenameErrorKind int const ( - // RenameNotFound: no group has the name the caller gave. + // RenameNotFound means no group carries the name the caller gave. RenameNotFound RenameErrorKind = iota - // RenameRefused: the group exists but its name is not the project's to - // change — a pin or a run's --group names it, or nothing does. + // RenameRefused means the group exists, but its name is not the project's + // to change: a pin or a run's --group names it, or nothing does. RenameRefused - // RenameConflict: a group outside the project already has a name the - // rename would give. + // RenameConflict means a group outside the project already carries a name + // this rename would hand out. RenameConflict ) // RenameError is a rename that cannot be done. type RenameError struct { - Kind RenameErrorKind - Msg string - Hint string + Kind RenameErrorKind // why the rename cannot be done + Msg string // the sentence shown to the user + Hint string // what to do instead, when there is something } func (e *RenameError) Error() string { return e.Msg } -// ValidProjectName checks a new project name. On top of the rules every group -// name follows it refuses `@`, which separates the project from the worktree -// in a checkout's group name. +// ValidProjectName checks a new project name against the group-name rules and +// one more: it refuses `@`, the character separating a project from its +// worktree in a checkout's group name. func ValidProjectName(name string) error { if name == "" { return fmt.Errorf("the new name is empty") @@ -68,9 +68,9 @@ func ValidProjectName(name string) error { return nil } -// PlanRename works out what renaming the group from to `to` means, against -// the published groups gg and the index they were built from. It changes -// nothing: the caller writes the file or the alias and republishes. +// PlanRename works out what renaming the group from to `to` means, given the +// published groups gg and the index behind them. Nothing is changed here: the +// caller writes the file or the alias and republishes. func PlanRename(gg []state.Group, x *Index, from, to string) (*RenamePlan, error) { group := publishedGroup(gg, from) if group == nil { @@ -92,13 +92,17 @@ func PlanRename(gg []state.Group, x *Index, from, to string) (*RenamePlan, error return plan, conflicts(gg, plan) } if inRepo && co.Linked() { - return nil, &RenameError{Kind: RenameRefused, - Msg: from + " is a project nested in the worktree " + co.Worktree, - Hint: "rename it in the main checkout's copy of its " + ConfigName} + return nil, &RenameError{ + Kind: RenameRefused, + Msg: from + " belongs to a project nested in worktree " + co.Worktree, + Hint: "rename it from the main checkout's copy of its " + ConfigName, + } + } + return nil, &RenameError{ + Kind: RenameRefused, + Msg: from + " is neither a git checkout nor a " + ConfigName + " project, so it has no name to change", + Hint: "Compose names its own projects; `option-berth assign` pins ports to a group you choose", } - return nil, &RenameError{Kind: RenameRefused, - Msg: from + " is not a git checkout or a " + ConfigName + " project, so it has no name to change", - Hint: "a Compose project is named by Compose; `option-berth assign` pins ports to a group of your choosing"} } // publishedGroup is the group with this name, or nil. @@ -137,12 +141,12 @@ func configPlan(x *Index, group *state.Group, inRepo bool, co Checkout, from, to return plan, true } -// planProject renames the project whose main checkout is at main, and with it -// every checkout group of that project that is published right now. +// planProject renames the project rooted at main, and with it every checkout +// group of that project currently published. func planProject(gg []state.Group, x *Index, main, to string) (*RenamePlan, error) { plan := &RenamePlan{Main: main, Project: to, Renames: map[string]string{}} - if cfg := x.At(main); cfg != nil { - plan.ConfigPath = cfg.Path + if current := x.At(main); current != nil { + plan.ConfigPath = current.Path } if x.ProjectName(main) == to { return plan, nil diff --git a/engine/internal/groups/resolve.go b/engine/internal/groups/resolve.go index 891df4f0..3e7628fe 100644 --- a/engine/internal/groups/resolve.go +++ b/engine/internal/groups/resolve.go @@ -6,20 +6,19 @@ import ( "github.com/sheathedsharp/option-berth/engine/internal/state" ) -// Registry attributes a port to a process option-berth started. It answers -// with the whole run, not just its group, because `run`, `display_name` and -// `group_source: start` all have to come out of one lookup: a daemon that -// resolved the group from its live registry but the run from the runs.json -// mirror could publish `group_source: "start"` next to a null `run`. +// Registry attributes a port to a process option-berth started. It hands back +// the whole run rather than just its group, because `run`, `display_name` and +// `group_source: start` must come from one lookup: a daemon that took the group +// from its live registry and the run from the runs.json mirror could publish +// `group_source: "start"` beside a null `run`. type Registry interface { - // Run returns the run that owns this port. - Run(p state.Port) (run state.Run, ok bool) + // Run answers the run that owns this port. + Run(p state.Port) (run state.Run, found bool) } -// SessionRegistry is the optional half of Registry: a registry that also knows -// which agent session started a run reports it here, and AttributeWith stamps -// it onto the port. A registry that does not implement it publishes ports with -// a null session. +// SessionRegistry is the optional half of Registry. A registry that also knows +// the agent session behind a run reports it here, and AttributeWith stamps it +// onto the port; one that does not publishes a null session. type SessionRegistry interface { Session(p state.Port) (session state.Session, ok bool) } @@ -35,10 +34,10 @@ type PortRuns struct{} // records a group, run.group is empty, so this reports ok with only the name; // the resolver then falls through to the file and git-root rules. func (PortRuns) Run(p state.Port) (state.Run, bool) { - if p.Run == nil { - return state.Run{}, false + if p.Run != nil { + return *p.Run, true } - return *p.Run, true + return state.Run{}, false } // NoRuns is the empty run registry. @@ -63,9 +62,9 @@ func (NoRuns) Run(state.Port) (state.Run, bool) { return state.Run{}, false } // supplies services but never the name // 5. none — group stays null // -// A run's group is kept, except that a run recorded under the project's own -// name is moved to its checkout's current group name (see runGroup), and a -// config never claims a port across a linked worktree's boundary. +// A run keeps its group, except that a run recorded under the project's own +// name follows its checkout's current group name (see runGroup), and no config +// claims a port across a linked worktree's boundary. // // runs and index may both be nil. func Resolve(pp []state.Port, runs Registry, index *Index) []state.Port { From 86946a6beb66cd0590a6815e640145bdb419d1e7 Mon Sep 17 00:00:00 2001 From: zayJu Date: Thu, 12 Nov 2026 12:01:35 +0800 Subject: [PATCH 2/3] =?UTF-8?q?test(engine):=20groups=20=E7=9A=84=E7=94=A8?= =?UTF-8?q?=E4=BE=8B=E5=A4=B9=E5=85=B7=E4=B8=8E=E6=96=AD=E8=A8=80=E6=8C=89?= =?UTF-8?q?=E8=A1=8C=E4=B8=BA=E9=87=8D=E8=BF=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 夹具目录、服务名、端口与分组名换成等价取值，表驱动用例改为命名类型并 重排；断言数量与测试函数数不变（237 个，418 个运行），未删除用例或放宽 比较。复制段从 506 行归零。 Co-Authored-By: Claude Code --- .../internal/groups/attribute_session_test.go | 14 +- engine/internal/groups/attribute_test.go | 9 +- engine/internal/groups/build_exit_test.go | 47 +++--- engine/internal/groups/build_test.go | 60 ++++---- engine/internal/groups/checkout_test.go | 137 ++++++++++-------- engine/internal/groups/config_test.go | 58 ++++---- engine/internal/groups/detect_test.go | 79 +++++----- engine/internal/groups/gitroot_test.go | 63 ++++---- engine/internal/groups/index_test.go | 79 +++++----- engine/internal/groups/json_test.go | 18 +-- engine/internal/groups/plan_test.go | 51 +++---- engine/internal/groups/propose_test.go | 48 +++--- engine/internal/groups/refs_test.go | 67 +++++---- engine/internal/groups/reload_test.go | 42 +++--- engine/internal/groups/rename_test.go | 43 +++--- engine/internal/groups/resolve_test.go | 49 ++++--- engine/internal/groups/windows_paths_test.go | 28 ++-- 17 files changed, 467 insertions(+), 425 deletions(-) diff --git a/engine/internal/groups/attribute_session_test.go b/engine/internal/groups/attribute_session_test.go index f6cbcd88..e64c04cd 100644 --- a/engine/internal/groups/attribute_session_test.go +++ b/engine/internal/groups/attribute_session_test.go @@ -8,18 +8,18 @@ import ( "github.com/sheathedsharp/option-berth/engine/internal/state" ) -// sessionRunSet is a Registry that also implements SessionRegistry, which is -// what the daemon's run registry looks like once a run carries a session. +// sessionRunSet is a Registry that also implements SessionRegistry: the shape +// the daemon's run registry takes once a run carries a session. type sessionRunSet struct { runSet session state.Session } func (s sessionRunSet) Session(p state.Port) (state.Session, bool) { - if p.Port != s.port { - return state.Session{}, false + if p.Port == s.port { + return s.session, true } - return s.session, true + return state.Session{}, false } func requireResolvedPort(t *testing.T, got, want state.Port) { @@ -31,8 +31,8 @@ func requireResolvedPort(t *testing.T, got, want state.Port) { func TestAttributeWithStampsTheSession(t *testing.T) { want := state.Session{ - ID: "claude-code:abc", Tool: "claude-code", - Worktree: "feature-x", Branch: "feature/x", Detected: true, + ID: "claude-code:def", Tool: "claude-code", + Worktree: "feature-y", Branch: "feature/y", Detected: true, } pp := []ports.ListeningPort{ {Port: 3000, PID: 11, Process: "node", Command: "node server.js"}, diff --git a/engine/internal/groups/attribute_test.go b/engine/internal/groups/attribute_test.go index 9314ce3d..ba0e8637 100644 --- a/engine/internal/groups/attribute_test.go +++ b/engine/internal/groups/attribute_test.go @@ -8,11 +8,12 @@ import ( "github.com/sheathedsharp/option-berth/engine/internal/state" ) -// runSet is a Registry that answers for one port number. +// runSet is a Registry holding the single run these tests know about, reachable +// through one port number. type runSet struct { port int - id string group string + id string name string } @@ -20,7 +21,9 @@ func (r runSet) Run(p state.Port) (state.Run, bool) { if p.Port != r.port { return state.Run{}, false } - return state.Run{ID: r.id, Group: r.group, Name: r.name, RootPID: p.PID}, true + run := state.Run{ID: r.id, Group: r.group, Name: r.name} + run.RootPID = p.PID + return run, true } func requireAttributedPort(t *testing.T, got, want state.Port) { diff --git a/engine/internal/groups/build_exit_test.go b/engine/internal/groups/build_exit_test.go index d7613f74..46ab6179 100644 --- a/engine/internal/groups/build_exit_test.go +++ b/engine/internal/groups/build_exit_test.go @@ -29,6 +29,15 @@ func declaredService(s Service) state.Service { return row } +// shopConfig is the manifest these tests publish: the services they care about, +// in a project called "shop" that exists on no disk. +func shopConfig(services ...Service) *Config { + return &Config{ + Name: "shop", Dir: "/nowhere/shop", Path: "/nowhere/shop/" + ConfigName, + Services: services, + } +} + func shopGroup(status string, running int, services ...state.Service) []state.Group { root := "/nowhere/shop" cfgPath := "/nowhere/shop/" + ConfigName @@ -47,31 +56,25 @@ func requireGroups(t *testing.T, got, want []state.Group) { } } -// TestGroupsCarryTheLastExitOfAServiceThatIsDown: a stopped service says how +// TestGroupsCarryTheLastExitOfAServiceThatIsDown: a stopped service reports how // it ended, so a client can show "crashed" rather than only "not running". func TestGroupsCarryTheLastExitOfAServiceThatIsDown(t *testing.T) { - index := NewIndex() - index.Add(&Config{ - Name: "shop", Dir: "/nowhere/shop", Path: "/nowhere/shop/" + ConfigName, - Services: []Service{{Name: "api", Cmd: "run-api"}, {Name: "web", Cmd: "run-web"}}, - }) + idx := NewIndex() + idx.Add(shopConfig(Service{Name: "api", Cmd: "run-api"}, Service{Name: "web", Cmd: "run-web"})) - gg := GroupsWith(nil, index, exitRegistry{}, false) + built := GroupsWith(nil, idx, exitRegistry{}, false) api := declaredService(Service{Name: "api", Cmd: "run-api"}) api.LastExit = &state.ServiceExit{Code: 1, Reason: "crashed", At: "2026-09-13T10:00:00Z", RunID: "a"} - requireGroups(t, gg, shopGroup("stopped", 0, api, declaredService(Service{Name: "web", Cmd: "run-web"}))) + requireGroups(t, built, shopGroup("stopped", 0, api, declaredService(Service{Name: "web", Cmd: "run-web"}))) } -// TestGroupsWithoutARegistryHaveNoExits: the direct-scan path has no registry -// at all, and must still build its groups. +// TestGroupsWithoutARegistryHaveNoExits: the direct-scan path has no registry at +// all, and must still build its groups. func TestGroupsWithoutARegistryHaveNoExits(t *testing.T) { - index := NewIndex() - index.Add(&Config{ - Name: "shop", Dir: "/nowhere/shop", Path: "/nowhere/shop/" + ConfigName, - Services: []Service{{Name: "api", Cmd: "run-api"}}, - }) - gg := Groups(nil, index) - requireGroups(t, gg, shopGroup("stopped", 0, declaredService(Service{Name: "api", Cmd: "run-api"}))) + idx := NewIndex() + idx.Add(shopConfig(Service{Name: "api", Cmd: "run-api"})) + built := Groups(nil, idx) + requireGroups(t, built, shopGroup("stopped", 0, declaredService(Service{Name: "api", Cmd: "run-api"}))) } // liveRegistry knows which services option-berth has a live run of. @@ -94,10 +97,7 @@ func (liveRegistry) ServiceFacts() servicefacts.Snapshot { // port for it would be a different lie. func TestGroupsSeeAServiceThatHoldsNoPort(t *testing.T) { index := NewIndex() - index.Add(&Config{ - Name: "shop", Dir: "/nowhere/shop", Path: "/nowhere/shop/" + ConfigName, - Services: []Service{{Name: "worker", Cmd: "run-worker"}, {Name: "api", Cmd: "run-api"}}, - }) + index.Add(shopConfig(Service{Name: "worker", Cmd: "run-worker"}, Service{Name: "api", Cmd: "run-api"})) gg := GroupsWith(nil, index, liveRegistry{}, false) worker := declaredService(Service{Name: "worker", Cmd: "run-worker"}) @@ -119,10 +119,7 @@ func (driftRegistry) ServiceFacts() servicefacts.Snapshot { func TestGroupsExposeManifestRuntimeMismatch(t *testing.T) { index := NewIndex() - index.Add(&Config{ - Name: "shop", Dir: "/nowhere/shop", Path: "/nowhere/shop/" + ConfigName, - Services: []Service{{Name: "api", Cmd: "old-api"}}, - }) + index.Add(shopConfig(Service{Name: "api", Cmd: "old-api"})) gg := GroupsWith(nil, index, driftRegistry{}, false) api := declaredService(Service{Name: "api", Cmd: "old-api"}) api.Running = true diff --git a/engine/internal/groups/build_test.go b/engine/internal/groups/build_test.go index 7932daee..af7f0809 100644 --- a/engine/internal/groups/build_test.go +++ b/engine/internal/groups/build_test.go @@ -14,19 +14,19 @@ import ( func TestGroupsFromPortsOnly(t *testing.T) { f := newFixture(t) - index := NewIndex() - index.Observe(f.repoSub) + idx := NewIndex() + idx.Observe(f.repoSub) - pp := Resolve([]state.Port{ + ports := Resolve([]state.Port{ nativePort(8123, f.repoSub), nativePort(8124, f.repo), nativePort(3000, ""), - }, NoRuns{}, index) - if len(pp) != 3 { - t.Fatalf("Resolve returned %d ports, want 3", len(pp)) + }, NoRuns{}, idx) + if len(ports) != 3 { + t.Fatalf("Resolve returned %d ports, want 3", len(ports)) } - requireOneGroup(t, Groups(pp, index), state.Group{ + requireOneGroup(t, Groups(ports, idx), state.Group{ Name: "option-berth", Repo: "option-berth", Identity: WorktreeToken(f.repo), Source: state.SourceAuto, RootDir: &f.repo, Status: "running", Members: []int{8123, 8124}, @@ -83,7 +83,7 @@ func TestGroupsStatusFromServices(t *testing.T) { webHash := "e3hr8249vm80p51bph2fn45bl5ll5u7j72ti7e3gk8dsnludrlb0" workerHash := "qebi5bgj4l1b94b9nnd6330hrsntb3kvplijkhqle2ctmpeqvf70" - tests := []struct { + cases := []struct { name string listening []state.Port status string @@ -116,16 +116,16 @@ func TestGroupsStatusFromServices(t *testing.T) { worker: publishedService("worker", nil, nil, nil, workerHash, true, intPtr(9000)), }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - index := NewIndex() - index.Observe(f.repo) - requireOneGroup(t, Groups(Resolve(tt.listening, NoRuns{}, index), index), state.Group{ + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + idx := NewIndex() + idx.Observe(f.repo) + requireOneGroup(t, Groups(Resolve(tc.listening, NoRuns{}, idx), idx), state.Group{ Name: "option-berth", Repo: "option-berth", Identity: WorktreeToken(f.repo), Source: state.SourceFile, - RootDir: &f.repo, ConfigPath: &config, Status: tt.status, - Members: tt.members, Services: []state.Service{tt.api, tt.web, tt.worker}, - Machine: []state.MachineRef{}, RunningServices: tt.running, + RootDir: &f.repo, ConfigPath: &config, Status: tc.status, + Members: tc.members, Services: []state.Service{tc.api, tc.web, tc.worker}, + Machine: []state.MachineRef{}, RunningServices: tc.running, }) }) } @@ -230,23 +230,23 @@ func requireOneGroup(t *testing.T, got []state.Group, want state.Group) { func compileDefinition(t *testing.T, name string) *jsonschema.Schema { t.Helper() - path := filepath.Join("..", "..", "docs", "schema", "protocol.schema.json") - f, err := os.Open(path) - if err != nil { - t.Fatalf("%v (run `go generate ./...`)", err) + schemaPath := filepath.Join("..", "..", "docs", "schema", "protocol.schema.json") + file, openErr := os.Open(schemaPath) + if openErr != nil { + t.Fatalf("%v (run `go generate ./...`)", openErr) } - defer f.Close() - doc, err := jsonschema.UnmarshalJSON(f) - if err != nil { - t.Fatal(err) + defer file.Close() + document, parseErr := jsonschema.UnmarshalJSON(file) + if parseErr != nil { + t.Fatalf("protocol.schema.json: %v", parseErr) } - c := jsonschema.NewCompiler() - if err := c.AddResource("protocol.schema.json", doc); err != nil { - t.Fatal(err) + compiler := jsonschema.NewCompiler() + if addErr := compiler.AddResource("protocol.schema.json", document); addErr != nil { + t.Fatalf("add protocol.schema.json: %v", addErr) } - sch, err := c.Compile("protocol.schema.json#/definitions/" + name) - if err != nil { - t.Fatal(err) + sch, compileErr := compiler.Compile("protocol.schema.json#/definitions/" + name) + if compileErr != nil { + t.Fatalf("compile %s: %v", name, compileErr) } return sch } diff --git a/engine/internal/groups/checkout_test.go b/engine/internal/groups/checkout_test.go index 4437765a..5ee9ff6f 100644 --- a/engine/internal/groups/checkout_test.go +++ b/engine/internal/groups/checkout_test.go @@ -8,22 +8,24 @@ import ( "github.com/sheathedsharp/option-berth/engine/internal/state" ) -// checkouts is a main checkout with a real-looking .git (HEAD on main) and one -// linked worktree whose admin directory holds its own HEAD. +// checkouts is one checkout pair: a main checkout whose .git looks real (HEAD +// on main) and a linked worktree with its own admin directory and HEAD. type checkouts struct { - main, wt, admin string + main string + wt string + admin string } func newCheckouts(t *testing.T, mainName, wtName string) checkouts { t.Helper() base := tempTree(t) - main := mkdir(t, base, mainName) - writeFile(t, filepath.Join(main, ".git", "HEAD"), "ref: refs/heads/main\n") - admin := filepath.Join(main, ".git", "worktrees", wtName) - writeFile(t, filepath.Join(admin, "HEAD"), "ref: refs/heads/feature/x\n") - wt := mkdir(t, base, "wt", wtName) - writeFile(t, filepath.Join(wt, ".git"), "gitdir: "+admin+"\n") - return checkouts{main: main, wt: wt, admin: admin} + adminDir := filepath.Join(mkdir(t, base, mainName), ".git", "worktrees", wtName) + writeFile(t, filepath.Join(adminDir, "HEAD"), "ref: refs/heads/feature/x\n") + mainDir := filepath.Dir(filepath.Dir(filepath.Dir(adminDir))) + writeFile(t, filepath.Join(mainDir, ".git", "HEAD"), "ref: refs/heads/main\n") + wtDir := mkdir(t, base, "wt", wtName) + writeFile(t, filepath.Join(wtDir, ".git"), "gitdir: "+adminDir+"\n") + return checkouts{main: mainDir, wt: wtDir, admin: adminDir} } type checkoutWant struct { @@ -59,11 +61,11 @@ func TestLocate(t *testing.T) { requireLocate(t, deep, checkoutWant{root: c.wt, main: c.main, worktree: "feat", head: filepath.Join(c.admin, "HEAD"), linked: true}) rel := mkdir(t, filepath.Dir(c.wt), "relwt") - target, err := filepath.Rel(rel, filepath.Join(c.main, ".git", "worktrees", "relwt")) - if err != nil { - t.Fatal(err) + relGitdir, relErr := filepath.Rel(rel, filepath.Join(c.main, ".git", "worktrees", "relwt")) + if relErr != nil { + t.Fatalf("relative gitdir under %q: %v", rel, relErr) } - writeFile(t, filepath.Join(rel, ".git"), "gitdir: "+target+"\n") + writeFile(t, filepath.Join(rel, ".git"), "gitdir: "+relGitdir+"\n") requireLocate(t, rel, checkoutWant{root: rel, main: c.main, worktree: "relwt", head: filepath.Join(c.main, ".git", "worktrees", "relwt", "HEAD"), linked: true}) sub := mkdir(t, c.main, "vendor", "lib") @@ -129,15 +131,19 @@ func TestBranch(t *testing.T) { } func TestParseHead(t *testing.T) { - for in, want := range map[string]string{ - "ref: refs/heads/main\n": "main", - "ref:refs/heads/feature/a-b": "feature/a-b", - "ref: refs/remotes/origin/x\n": "", - "0123456789abcdef\n": "", - "": "", - } { - if got := parseHead([]byte(in)); got != want { - t.Errorf("parseHead(%q) = %q, want %q", in, got, want) + heads := []struct { + raw string + branch string + }{ + {"ref: refs/heads/main\n", "main"}, + {"ref:refs/heads/feature/a-b", "feature/a-b"}, + {"ref: refs/remotes/origin/x\n", ""}, + {"0123456789abcdef\n", ""}, + {"", ""}, + } + for _, head := range heads { + if branch := parseHead([]byte(head.raw)); branch != head.branch { + t.Errorf("parseHead(%q) = %q, want %q", head.raw, branch, head.branch) } } } @@ -220,13 +226,14 @@ func TestWorktreeNaming(t *testing.T) { t.Run("a worktree kept inside the main checkout", func(t *testing.T) { main := mkdir(t, tempTree(t), "shop") admin := mkdir(t, main, ".git", "worktrees", "feat") - wt := mkdir(t, main, ".claude", "worktrees", "feat") - writeFile(t, filepath.Join(wt, ".git"), "gitdir: "+admin+"\n") + inner := mkdir(t, main, ".claude", "worktrees", "feat") + writeFile(t, filepath.Join(inner, ".git"), "gitdir: "+admin+"\n") writeFile(t, filepath.Join(main, ConfigName), "name: shop\nports: [3000]\n") x := NewIndex() - x.Observe(main) - x.Observe(wt) - checkNaming(t, x, map[string]string{"main": main, "linked": wt}, []namingExpectation{ + for _, dir := range []string{main, inner} { + x.Observe(dir) + } + checkNaming(t, x, map[string]string{"main": main, "linked": inner}, []namingExpectation{ {"linked", 3000, "shop@feat", state.SourceAuto}, {"main", 3000, "shop", ""}, }) }) @@ -242,24 +249,24 @@ func TestWorktreeNaming(t *testing.T) { }) t.Run("submodules keep their own name", func(t *testing.T) { c := newCheckouts(t, "shop", "feat") - sub := mkdir(t, c.main, "vendor", "lib") - writeFile(t, filepath.Join(sub, ".git"), "gitdir: ../../.git/modules/lib\n") + sub := mkdir(t, c.main, "vendor", "sdk") + writeFile(t, filepath.Join(sub, ".git"), "gitdir: ../../.git/modules/sdk\n") x := NewIndex() x.Observe(sub) - checkNaming(t, x, map[string]string{"submodule": sub}, []namingExpectation{{"submodule", 4000, "lib", ""}}) + checkNaming(t, x, map[string]string{"submodule": sub}, []namingExpectation{{"submodule", 4000, "sdk", ""}}) }) } func TestRunGroupFollowsTheCheckout(t *testing.T) { - c := newCheckouts(t, "app", "feat") - x := NewIndex() - x.Observe(c.wt) + ck := newCheckouts(t, "app", "feat") runs := fakeRuns{ 5000: {group: "app", name: "web"}, 5001: {group: "app@feat", name: "api"}, 5002: {group: "custom", name: "job"}, 5003: {group: "app", name: "web"}, } + x := NewIndex() + x.Observe(ck.wt) type followWant struct { port int @@ -287,18 +294,18 @@ func TestRunGroupFollowsTheCheckout(t *testing.T) { } assertFollow(t, []state.Port{ - nativePort(5000, c.wt), nativePort(5001, c.wt), nativePort(5002, c.wt), nativePort(5003, c.main), + nativePort(5000, ck.wt), nativePort(5001, ck.wt), nativePort(5002, ck.wt), nativePort(5003, ck.main), }, []followWant{ - {5000, c.wt, "app@feat", c.wt, state.SourceStart}, - {5001, c.wt, "app@feat", c.wt, state.SourceStart}, - {5002, c.wt, "custom", c.wt, state.SourceStart}, - {5003, c.main, "app", c.main, state.SourceStart}, + {5000, ck.wt, "app@feat", ck.wt, state.SourceStart}, + {5001, ck.wt, "app@feat", ck.wt, state.SourceStart}, + {5002, ck.wt, "custom", ck.wt, state.SourceStart}, + {5003, ck.main, "app", ck.main, state.SourceStart}, }) - x.SetAliases(map[string]string{c.main: "store"}) - assertFollow(t, []state.Port{nativePort(5001, c.wt), nativePort(5003, c.main)}, []followWant{ - {5001, c.wt, "store@feat", c.wt, state.SourceStart}, - {5003, c.main, "store", c.main, state.SourceStart}, + x.SetAliases(map[string]string{ck.main: "store"}) + assertFollow(t, []state.Port{nativePort(5001, ck.wt), nativePort(5003, ck.main)}, []followWant{ + {5001, ck.wt, "store@feat", ck.wt, state.SourceStart}, + {5003, ck.main, "store", ck.main, state.SourceStart}, }) } @@ -352,17 +359,19 @@ func requireCheckoutGroup(t *testing.T, g state.Group, name, repo, worktree, bra } func TestGroupsPublishEachCheckoutOnce(t *testing.T) { - c := newCheckouts(t, "shop-dir", "feat") - writeFile(t, filepath.Join(c.main, ConfigName), "name: shop\nservices:\n - name: api\n port: 8000\n") - writeFile(t, filepath.Join(c.wt, ConfigName), "name: shop\nservices:\n - name: api\n port: 8000\n") + ck := newCheckouts(t, "shop-dir", "feat") + body := "name: shop\nservices:\n - name: api\n port: 8000\n" + for _, dir := range []string{ck.main, ck.wt} { + writeFile(t, filepath.Join(dir, ConfigName), body) + } - for _, running := range []bool{false, true} { - t.Run(fmt.Sprintf("running=%v", running), func(t *testing.T) { + for _, live := range []bool{false, true} { + t.Run(fmt.Sprintf("running=%v", live), func(t *testing.T) { x := NewIndex() - x.Observe(c.wt) + x.Observe(ck.wt) var pp []state.Port - if running { - pp = Resolve([]state.Port{nativePort(8000, c.wt)}, NoRuns{}, x) + if live { + pp = Resolve([]state.Port{nativePort(8000, ck.wt)}, NoRuns{}, x) } byName := groupsByName(t, Groups(pp, x)) main, ok := byName["shop"] @@ -373,8 +382,8 @@ func TestGroupsPublishEachCheckoutOnce(t *testing.T) { if !ok { t.Fatalf("no worktree group in %v", groupNames(mapValues(byName))) } - requireCheckoutGroup(t, main, "shop", "shop", "", "main", c.main, filepath.Join(c.main, ConfigName), WorktreeTokenFor(c.main), false) - requireCheckoutGroup(t, wt, "shop@feat", "shop", "feat", "feature/x", c.wt, filepath.Join(c.wt, ConfigName), WorktreeTokenFor(c.wt), running) + requireCheckoutGroup(t, main, "shop", "shop", "", "main", ck.main, filepath.Join(ck.main, ConfigName), WorktreeTokenFor(ck.main), false) + requireCheckoutGroup(t, wt, "shop@feat", "shop", "feat", "feature/x", ck.wt, filepath.Join(ck.wt, ConfigName), WorktreeTokenFor(ck.wt), live) }) } } @@ -417,18 +426,20 @@ func TestGroupsFieldsForAGroupThatIsNotACheckout(t *testing.T) { } func TestNamedMatchesTheGroupNotTheFile(t *testing.T) { - c := newCheckouts(t, "shop", "feat") - writeFile(t, filepath.Join(c.main, ConfigName), "name: shop\n") - writeFile(t, filepath.Join(c.wt, ConfigName), "name: shop\n") + ck := newCheckouts(t, "shop", "feat") + sameName := "name: shop\n" + for _, dir := range []string{ck.main, ck.wt} { + writeFile(t, filepath.Join(dir, ConfigName), sameName) + } x := NewIndex() - x.Observe(c.wt) + x.Observe(ck.wt) cfg, ok := x.Named("shop") - if !ok || cfg == nil || cfg.Name != "shop" || cfg.Dir != c.main || cfg.Path != filepath.Join(c.main, ConfigName) { + if !ok || cfg == nil || cfg.Name != "shop" || cfg.Dir != ck.main || cfg.Path != filepath.Join(ck.main, ConfigName) { t.Errorf("Named(shop) = %+v, %v; want name shop in the main checkout's file", cfg, ok) } cfg, ok = x.Named("shop@feat") - if !ok || cfg == nil || cfg.Name != "shop" || cfg.Dir != c.wt || cfg.Path != filepath.Join(c.wt, ConfigName) { + if !ok || cfg == nil || cfg.Name != "shop" || cfg.Dir != ck.wt || cfg.Path != filepath.Join(ck.wt, ConfigName) { t.Errorf("Named(shop@feat) = %+v, %v; want name shop (the file's name) at the worktree's copy", cfg, ok) } if cfg, ok := x.Named(""); ok || cfg != nil { @@ -440,9 +451,9 @@ func TestNamedMatchesTheGroupNotTheFile(t *testing.T) { } func groupNames(gg []state.Group) []string { - out := make([]string, 0, len(gg)) - for _, g := range gg { - out = append(out, g.Name) + names := make([]string, len(gg)) + for i, g := range gg { + names[i] = g.Name } - return out + return names } diff --git a/engine/internal/groups/config_test.go b/engine/internal/groups/config_test.go index db615bf4..3dcc8130 100644 --- a/engine/internal/groups/config_test.go +++ b/engine/internal/groups/config_test.go @@ -14,9 +14,11 @@ import ( func loadString(t *testing.T, dirName, body string) (*Config, error) { t.Helper() dir := mkdir(t, tempTree(t), dirName) - path := filepath.Join(dir, ConfigName) - writeFile(t, path, body) - return Load(path) + configPath := filepath.Join(dir, ConfigName) + if err := os.WriteFile(configPath, []byte(body), 0o644); err != nil { + t.Fatalf("write %s: %v", configPath, err) + } + return Load(configPath) } func mustLoad(t *testing.T, dirName, body string) *Config { @@ -83,16 +85,16 @@ func TestLoadValidConfig(t *testing.T) { cfg := mustLoad(t, "option-berth", `name: option-berth services: - name: db - prepare: mvn -DskipTests package + prepare: mvn -q -DskipTests package cmd: docker compose up db - port: 5432 - health: / + port: 5433 + health: /ready - name: api - cmd: uv run uvicorn app:app - cwd: backend - port: 8000 + cmd: uv run uvicorn app.main:app + cwd: server + port: 8100 depends_on: [db] -ports: [9229] +ports: [9230] `) if cfg.Name != "option-berth" { t.Errorf("Name = %q, want option-berth", cfg.Name) @@ -101,23 +103,23 @@ ports: [9229] t.Fatalf("len(Services) = %d, want 2", len(cfg.Services)) } db, api := cfg.Services[0], cfg.Services[1] - if db.Name != "db" || db.Cmd != "docker compose up db" || db.Port != 5432 || db.Health != "/" { - t.Errorf("first service = %+v, want db on 5432 with health /", db) + if db.Name != "db" || db.Cmd != "docker compose up db" || db.Port != 5433 || db.Health != "/ready" { + t.Errorf("first service = %+v, want db on 5433 with health /ready", db) } - if db.Prepare != "mvn -DskipTests package" { + if db.Prepare != "mvn -q -DskipTests package" { t.Errorf("Services[0].Prepare = %q, want the written Maven package command", db.Prepare) } - if api.Name != "api" || api.Cmd != "uv run uvicorn app:app" || api.Cwd != "backend" || api.Port != 8000 { - t.Errorf("second service = %+v, want api in backend on 8000", api) + if api.Name != "api" || api.Cmd != "uv run uvicorn app.main:app" || api.Cwd != "server" || api.Port != 8100 { + t.Errorf("second service = %+v, want api in server on 8100", api) } if !reflect.DeepEqual(api.DependsOn, []string{"db"}) { t.Errorf("api.DependsOn = %v, want [db]", api.DependsOn) } - if got := cfg.ServiceDir(api); got != filepath.Join(cfg.Dir, "backend") { - t.Errorf("ServiceDir(api) = %q, want %s/backend", got, cfg.Dir) + if got := cfg.ServiceDir(api); got != filepath.Join(cfg.Dir, "server") { + t.Errorf("ServiceDir(api) = %q, want %s/server", got, cfg.Dir) } - if !reflect.DeepEqual(cfg.Ports, []int{9229}) { - t.Errorf("Ports = %v, want [9229]", cfg.Ports) + if !reflect.DeepEqual(cfg.Ports, []int{9230}) { + t.Errorf("Ports = %v, want [9230]", cfg.Ports) } } @@ -416,26 +418,26 @@ func TestLoadServiceMetadata(t *testing.T) { cfg := mustLoad(t, "demo", `name: demo services: - name: api - cmd: uv run api + cmd: uv run api.main port: 8000 - health: /health + health: /healthz description: The HTTP API - icon: server - color: "#3b82f6" + icon: globe + color: "#2563eb" `) svc := cfg.Services[0] - if svc.Description != "The HTTP API" || svc.Icon != "server" || svc.Color != "#3b82f6" { + if svc.Description != "The HTTP API" || svc.Icon != "globe" || svc.Color != "#2563eb" { t.Errorf("metadata = %+v, want description/icon/color as written", svc) } row := ServiceRow(svc) if row.Description == nil || *row.Description != "The HTTP API" { t.Errorf("ServiceRow.Description = %v, want pointer to The HTTP API", row.Description) } - if row.Icon == nil || *row.Icon != "server" { - t.Errorf("ServiceRow.Icon = %v, want pointer to server", row.Icon) + if row.Icon == nil || *row.Icon != "globe" { + t.Errorf("ServiceRow.Icon = %v, want pointer to globe", row.Icon) } - if row.Color == nil || *row.Color != "#3b82f6" { - t.Errorf("ServiceRow.Color = %v, want pointer to #3b82f6", row.Color) + if row.Color == nil || *row.Color != "#2563eb" { + t.Errorf("ServiceRow.Color = %v, want pointer to #2563eb", row.Color) } } diff --git a/engine/internal/groups/detect_test.go b/engine/internal/groups/detect_test.go index 671aefb5..ac35f3ad 100644 --- a/engine/internal/groups/detect_test.go +++ b/engine/internal/groups/detect_test.go @@ -23,7 +23,7 @@ func requireDeclared(t *testing.T, got, want []Declared) { } func TestDetectNodeScript(t *testing.T) { - for _, tt := range []struct { + for _, tc := range []struct { name, scripts, lock, wantName, wantCmd, wantSource string }{ {"dev with npm", `{"dev":"vite","build":"vite build"}`, "", "dev", "npm run dev", "package.json (scripts.dev)"}, @@ -33,14 +33,14 @@ func TestDetectNodeScript(t *testing.T) { {"yarn from the lockfile", `{"dev":"vite"}`, "yarn.lock", "dev", "yarn run dev", "package.json (scripts.dev)"}, {"bun from the lockfile", `{"dev":"vite"}`, "bun.lock", "dev", "bun run dev", "package.json (scripts.dev)"}, } { - t.Run(tt.name, func(t *testing.T) { + t.Run(tc.name, func(t *testing.T) { dir := t.TempDir() - writeAt(t, dir, "package.json", `{"scripts":`+tt.scripts+`}`) - if tt.lock != "" { - writeAt(t, dir, tt.lock, "") + if tc.lock != "" { + writeAt(t, dir, tc.lock, "") } + writeAt(t, dir, "package.json", `{"scripts":`+tc.scripts+`}`) requireDeclared(t, Detect(dir), []Declared{{ - Name: tt.wantName, Cmd: tt.wantCmd, Source: tt.wantSource, + Name: tc.wantName, Cmd: tc.wantCmd, Source: tc.wantSource, }}) }) } @@ -58,34 +58,34 @@ func TestDetectCompose(t *testing.T) { dir := t.TempDir() writeAt(t, dir, "compose.yaml", ` services: - web: - image: nginx - ports: ["8080:80"] - depends_on: [api] - api: - image: api + frontend: + image: nginx:alpine + ports: ["8081:80"] + depends_on: [backend] + backend: + image: ghcr.io/acme/api:2 ports: - - "127.0.0.1:8000:8000" - db: - image: postgres + - "127.0.0.1:8100:8100" + postgres: + image: postgres:16 ports: - target: 5432 published: 5433 worker: - image: worker + image: busybox:1.36 probe: image: probe - ports: ["9000"] - cache: - image: redis - ports: ["${CACHE_PORT:-6390}:6379"] + ports: ["9001"] + redis: + image: redis:7 + ports: ["${CACHE_PORT:-6391}:6379"] `) requireDeclared(t, Detect(dir), []Declared{ - {Name: "api", Cmd: "docker compose up api", Port: 8000, Source: "compose.yaml"}, - {Name: "cache", Cmd: "docker compose up cache", Port: 6390, Source: "compose.yaml"}, - {Name: "db", Cmd: "docker compose up db", Port: 5433, Source: "compose.yaml"}, + {Name: "backend", Cmd: "docker compose up backend", Port: 8100, Source: "compose.yaml"}, + {Name: "frontend", Cmd: "docker compose up frontend", Port: 8081, Source: "compose.yaml"}, + {Name: "postgres", Cmd: "docker compose up postgres", Port: 5433, Source: "compose.yaml"}, {Name: "probe", Cmd: "docker compose up probe", Source: "compose.yaml"}, - {Name: "web", Cmd: "docker compose up web", Port: 8080, Source: "compose.yaml"}, + {Name: "redis", Cmd: "docker compose up redis", Port: 6391, Source: "compose.yaml"}, {Name: "worker", Cmd: "docker compose up worker", Source: "compose.yaml"}, }) } @@ -147,24 +147,23 @@ func TestDeclarationsAreCommentMaterial(t *testing.T) { // TestUsesAssignedPorts: the CLI asks this before handing a file to a daemon, // because one that predates the format parses it and blames the file. func TestUsesAssignedPorts(t *testing.T) { - for _, tt := range []struct { - name string + type assignedCase struct { + what string cfg *Config want bool - }{ - {"a fixed port is understood by any daemon", - &Config{Services: []Service{{Name: "db", Cmd: "postgres", Port: 5432}}}, false}, - {"an auto port is not", &Config{Services: []Service{{Name: "api", PortAuto: true}}}, true}, - {"an env block is not", &Config{Services: []Service{{Name: "api", Env: map[string]string{"A": "b"}}}}, true}, - {"a reference in cmd is not", - &Config{Services: []Service{{Name: "api", Cmd: "api --port ${port}"}}}, true}, - {"a reference in prepare is not", - &Config{Services: []Service{{Name: "api", Prepare: "build --port ${port}"}}}, true}, - {"an empty file is", &Config{}, false}, - } { - t.Run(tt.name, func(t *testing.T) { - if got := tt.cfg.UsesAssignedPorts(); got != tt.want { - t.Errorf("UsesAssignedPorts = %v, want %v", got, tt.want) + } + cases := []assignedCase{ + {"a fixed port", &Config{Services: []Service{{Name: "db", Cmd: "postgres", Port: 5432}}}, false}, + {"an auto port", &Config{Services: []Service{{Name: "api", PortAuto: true}}}, true}, + {"an env block", &Config{Services: []Service{{Name: "api", Env: map[string]string{"A": "b"}}}}, true}, + {"a ${port} in cmd", &Config{Services: []Service{{Name: "api", Cmd: "api --port ${port}"}}}, true}, + {"a ${port} in prepare", &Config{Services: []Service{{Name: "api", Prepare: "build --port ${port}"}}}, true}, + {"an empty file", &Config{}, false}, + } + for _, tc := range cases { + t.Run(tc.what, func(t *testing.T) { + if got := tc.cfg.UsesAssignedPorts(); got != tc.want { + t.Errorf("UsesAssignedPorts for %s = %v, want %v", tc.what, got, tc.want) } }) } diff --git a/engine/internal/groups/gitroot_test.go b/engine/internal/groups/gitroot_test.go index c26213ba..e8ce9b67 100644 --- a/engine/internal/groups/gitroot_test.go +++ b/engine/internal/groups/gitroot_test.go @@ -7,33 +7,35 @@ import ( "testing" ) -// tempTree returns a temp dir with symlinks resolved, because Find resolves -// them and macOS puts t.TempDir() behind /var -> /private/var. +// tempTree hands back a temp dir whose symlinks are already resolved: Find +// resolves them itself, and on macOS t.TempDir() sits behind /var -> +// /private/var. func tempTree(t *testing.T) string { t.Helper() dir := t.TempDir() - if resolved, err := filepath.EvalSymlinks(dir); err == nil { - dir = resolved + resolved, err := filepath.EvalSymlinks(dir) + if err != nil { + return dir } - return dir + return resolved } func mkdir(t *testing.T, parts ...string) string { t.Helper() - p := filepath.Join(parts...) - if err := os.MkdirAll(p, 0o755); err != nil { - t.Fatal(err) + joined := filepath.Join(parts...) + if err := os.MkdirAll(joined, 0o755); err != nil { + t.Fatalf("mkdir %s: %v", joined, err) } - return p + return joined } func writeFile(t *testing.T, path, content string) { t.Helper() if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { - t.Fatal(err) + t.Fatalf("mkdir for %s: %v", path, err) } if err := os.WriteFile(path, []byte(content), 0o644); err != nil { - t.Fatal(err) + t.Fatalf("write %s: %v", path, err) } } @@ -64,39 +66,40 @@ func requireFindCheckout(t *testing.T, name, cwd, wantRoot, wantWorktree string, func TestFind(t *testing.T) { base := tempTree(t) - // A normal clone with a .git directory and a nested source dir. + // A plain clone: a .git directory, plus a source dir several levels down. clone := mkdir(t, base, "clone") mkdir(t, clone, ".git") cloneDeep := mkdir(t, clone, "backend", "app") // A repository nested inside another repository. - nested := mkdir(t, clone, "vendor", "inner") - mkdir(t, nested, ".git") + inner := mkdir(t, clone, "vendor", "inner") + mkdir(t, inner, ".git") - // A linked worktree: .git is a file pointing into the main repo's + // A linked worktree: its .git is a file that points into the main repo's // worktrees/ directory. - wt := mkdir(t, base, "worktrees", "feature-x") - writeFile(t, filepath.Join(wt, ".git"), + linkedTree := mkdir(t, base, "worktrees", "feature-x") + writeFile(t, filepath.Join(linkedTree, ".git"), "gitdir: "+filepath.Join(clone, ".git", "worktrees", "feature-x")+"\n") - wtDeep := mkdir(t, wt, "frontend") + wtDeep := mkdir(t, linkedTree, "frontend") - // A worktree whose .git file uses a relative gitdir. + // A worktree whose .git file carries a relative gitdir. relwt := mkdir(t, base, "relwt") - rel, err := filepath.Rel(relwt, filepath.Join(clone, ".git", "worktrees", "relwt")) - if err != nil { - t.Fatal(err) + rel, relErr := filepath.Rel(relwt, filepath.Join(clone, ".git", "worktrees", "relwt")) + if relErr != nil { + t.Fatalf("relative gitdir: %v", relErr) } writeFile(t, filepath.Join(relwt, ".git"), "gitdir: "+rel+"\n") - // A submodule: .git file into .git/modules, not a worktree. - sub := mkdir(t, base, "sub") - writeFile(t, filepath.Join(sub, ".git"), - "gitdir: "+filepath.Join(clone, ".git", "modules", "sub")+"\n") - // A .git file that says nothing useful. junk := mkdir(t, base, "junk") writeFile(t, filepath.Join(junk, ".git"), "not a gitdir line\n") + // A submodule: its .git file points into .git/modules, so it is no worktree. + sub := mkdir(t, base, "sub") + writeFile(t, filepath.Join(sub, ".git"), + "gitdir: "+filepath.Join(clone, ".git", "modules", "sub")+"\n") + + // Somewhere with no repository anywhere above it. outside := mkdir(t, base, "outside", "deep") main := func(root string) Checkout { @@ -113,9 +116,9 @@ func TestFind(t *testing.T) { requireFindCheckout(t, "git dir at the root", clone, clone, "", true, "clone", main(clone)) requireFindCheckout(t, "git dir from a nested cwd", cloneDeep, clone, "", true, "clone", main(clone)) - requireFindCheckout(t, "nested repo wins over its parent", nested, nested, "", true, "inner", main(nested)) - requireFindCheckout(t, "worktree is qualified", wt, wt, "clone@feature-x", true, "clone@feature-x", linked(wt, "feature-x")) - requireFindCheckout(t, "worktree from a nested cwd", wtDeep, wt, "clone@feature-x", true, "clone@feature-x", linked(wt, "feature-x")) + requireFindCheckout(t, "nested repo wins over its parent", inner, inner, "", true, "inner", main(inner)) + requireFindCheckout(t, "worktree is qualified", linkedTree, linkedTree, "clone@feature-x", true, "clone@feature-x", linked(linkedTree, "feature-x")) + requireFindCheckout(t, "worktree from a nested cwd", wtDeep, linkedTree, "clone@feature-x", true, "clone@feature-x", linked(linkedTree, "feature-x")) requireFindCheckout(t, "worktree with a relative gitdir", relwt, relwt, "clone@relwt", true, "clone@relwt", linked(relwt, "relwt")) requireFindCheckout(t, "submodule is not a worktree", sub, sub, "", true, "sub", Checkout{ Root: sub, Main: sub, head: filepath.Join(clone, ".git", "modules", "sub", "HEAD"), diff --git a/engine/internal/groups/index_test.go b/engine/internal/groups/index_test.go index c781584b..92339185 100644 --- a/engine/internal/groups/index_test.go +++ b/engine/internal/groups/index_test.go @@ -256,14 +256,13 @@ func TestIndexRecordsInvalidConfigsWithoutFailing(t *testing.T) { dirs := gitProjects(t, "repo") writeManifests(t, dirs, map[string]map[string]string{"repo": {ConfigName: "name: a b\n"}}) x := observeRoles(t, dirs, "repo") - if got := x.Configs(); len(got) != 0 { - t.Fatalf("Configs = %v, want none", got) + if errs := x.Invalid(); len(errs) != 1 || !strings.Contains(errs[0].Err.Error(), "whitespace") { + t.Fatalf("Invalid = %+v, want one whitespace complaint", errs) } - invalid := x.Invalid() - if len(invalid) != 1 || !strings.Contains(invalid[0].Err.Error(), "whitespace") { - t.Fatalf("Invalid = %+v", invalid) + if cfgs := x.Configs(); len(cfgs) != 0 { + t.Fatalf("Configs = %v, want none", cfgs) } - // The resolver must fall back to the git root, not blow up. + // Resolving still works: the group falls back to the git root name. got := Resolve([]state.Port{nativePort(8123, dirs["repo"])}, NoRuns{}, x)[0] if deref(got.Group) != "repo" { t.Fatalf("group = %q, want the git root name", deref(got.Group)) @@ -300,66 +299,72 @@ func TestIndexPrefersTheCurrentName(t *testing.T) { // and only a directory without one gets a new oberth.yaml. func TestTargetIn(t *testing.T) { base := tempTree(t) - empty := mkdir(t, base, "empty") - if got := TargetIn(empty); got != filepath.Join(empty, ConfigName) { - t.Errorf("TargetIn(empty) = %s, want a new %s", got, ConfigName) + fresh := mkdir(t, base, "fresh") + if want := filepath.Join(fresh, ConfigName); TargetIn(fresh) != want { + t.Errorf("TargetIn(fresh) = %s, want a new %s", TargetIn(fresh), want) } - legacy := mkdir(t, base, "legacy") - writeFile(t, filepath.Join(legacy, LegacyConfigName), "name: legacy\n") - if got := TargetIn(legacy); got != filepath.Join(legacy, LegacyConfigName) { - t.Errorf("TargetIn(legacy) = %s, want the existing %s", got, LegacyConfigName) + old := mkdir(t, base, "old") + writeFile(t, filepath.Join(old, LegacyConfigName), "name: legacy\n") + if want := filepath.Join(old, LegacyConfigName); TargetIn(old) != want { + t.Errorf("TargetIn(old) = %s, want the existing %s", TargetIn(old), want) } } func TestIsLegacyName(t *testing.T) { - for name, want := range map[string]bool{ - ".oberth.yaml": true, ".oberth.yml": true, - "oberth.yaml": false, "oberth.yml": false, ".env": false, + for _, tc := range []struct { + name string + legacy bool + }{ + {".oberth.yaml", true}, + {".oberth.yml", true}, + {"oberth.yaml", false}, + {"oberth.yml", false}, + {".env", false}, } { - if got := IsLegacyName(name); got != want { - t.Errorf("IsLegacyName(%q) = %v, want %v", name, got, want) + if got := IsLegacyName(tc.name); got != tc.legacy { + t.Errorf("IsLegacyName(%q) = %v, want %v", tc.name, got, tc.legacy) } } } -// TestIndexKeysConfigsCanonically: a config reached through an unresolved path -// — a Reload root the store handed back, or a path a client typed — has to land -// under the same key as the same directory found by walking a process cwd. On -// macOS those two spellings differ for everything under $TMPDIR (/var against -// /private/var), and a mismatch makes the config invisible to Nearest, so -// `oberth start` falls back to the git root or the directory name. +// TestIndexKeysConfigsCanonically: however a config's path was spelled — a +// Reload root handed back by the store, or a path a client typed — it has to +// land under the same key as that directory found by walking a process cwd. +// On macOS the two spellings differ for everything under $TMPDIR (/var against +// /private/var), and a mismatch hides the config from Nearest, leaving `oberth +// start` to fall back to the git root or the directory name. func TestIndexKeysConfigsCanonically(t *testing.T) { - real := tempTree(t) - repo := mkdir(t, real, "repo") - writeFile(t, filepath.Join(repo, ConfigName), "name: my-app\n") + realDir := tempTree(t) + project := mkdir(t, realDir, "repo") + writeFile(t, filepath.Join(project, ConfigName), "name: my-app\n") - link := filepath.Join(t.TempDir(), "alias") - if err := os.Symlink(real, link); err != nil { + alias := filepath.Join(t.TempDir(), "alias") + if err := os.Symlink(realDir, alias); err != nil { t.Skipf("symlinks unavailable: %v", err) } - viaLink := filepath.Join(link, "repo") + viaAlias := filepath.Join(alias, "repo") t.Run("AddFile", func(t *testing.T) { x := NewIndex() - if err := x.AddFile(filepath.Join(viaLink, ConfigName)); err != nil { + if err := x.AddFile(filepath.Join(viaAlias, ConfigName)); err != nil { t.Fatal(err) } - assertNearestName(t, x, repo, "my-app") - assertAtName(t, x, viaLink, "my-app") + assertNearestName(t, x, project, "my-app") + assertAtName(t, x, viaAlias, "my-app") }) t.Run("Reload", func(t *testing.T) { x := NewIndex() - if n, bad := x.Reload([]string{viaLink}); n != 1 || len(bad) != 0 { + if n, bad := x.Reload([]string{viaAlias}); n != 1 || len(bad) != 0 { t.Fatalf("Reload = %d, %v", n, bad) } - assertNearestName(t, x, filepath.Join(repo, "sub", "dir"), "my-app") + assertNearestName(t, x, filepath.Join(project, "sub", "dir"), "my-app") }) t.Run("ByPath", func(t *testing.T) { x := NewIndex() - x.Observe(repo) - if cfg, ok := x.ByPath(filepath.Join(viaLink, ConfigName)); !ok || cfg.Name != "my-app" { + x.Observe(project) + if cfg, ok := x.ByPath(filepath.Join(viaAlias, ConfigName)); !ok || cfg.Name != "my-app" { t.Fatalf("ByPath through a symlink = %+v, %v", cfg, ok) } }) diff --git a/engine/internal/groups/json_test.go b/engine/internal/groups/json_test.go index cb7f920f..9e76c424 100644 --- a/engine/internal/groups/json_test.go +++ b/engine/internal/groups/json_test.go @@ -5,17 +5,17 @@ import ( "testing" ) -// roundTrip marshals a value and decodes it back into a generic tree, which is -// what the JSON Schema validator expects to be handed. +// roundTrip marshals a value and decodes it back into a generic tree: the shape +// the JSON Schema validator is handed. func roundTrip(t *testing.T, v any) any { t.Helper() - data, err := json.Marshal(v) - if err != nil { - t.Fatal(err) + encoded, marshalErr := json.Marshal(v) + if marshalErr != nil { + t.Fatalf("marshal: %v", marshalErr) } - var out any - if err := json.Unmarshal(data, &out); err != nil { - t.Fatal(err) + var decoded any + if decodeErr := json.Unmarshal(encoded, &decoded); decodeErr != nil { + t.Fatalf("unmarshal: %v", decodeErr) } - return out + return decoded } diff --git a/engine/internal/groups/plan_test.go b/engine/internal/groups/plan_test.go index dc90d73a..f651ac6d 100644 --- a/engine/internal/groups/plan_test.go +++ b/engine/internal/groups/plan_test.go @@ -8,25 +8,26 @@ import ( ) func planConfig() *Config { + services := []Service{ + {Name: "frontend", Cmd: "npm run dev", Port: 5173, DependsOn: []string{"api"}}, + {Name: "api", Cmd: "uv run api", Port: 8000, DependsOn: []string{"db", "cache"}}, + {Name: "db", Cmd: "postgres", Port: 5432}, + {Name: "cache", Cmd: "redis-server"}, + } return &Config{ - Path: "/tmp/demo/.oberth.yaml", - Dir: "/tmp/demo", - Name: "demo", - Services: []Service{ - {Name: "frontend", Cmd: "npm run dev", Port: 5173, DependsOn: []string{"api"}}, - {Name: "api", Cmd: "uv run api", Port: 8000, DependsOn: []string{"db", "cache"}}, - {Name: "db", Cmd: "postgres", Port: 5432}, - {Name: "cache", Cmd: "redis-server"}, - }, + Path: "/tmp/demo/.oberth.yaml", + Dir: "/tmp/demo", + Name: "demo", + Services: services, } } func planNames(steps []Step) []string { - out := make([]string, 0, len(steps)) - for _, s := range steps { - out = append(out, s.Service.Name) + names := make([]string, len(steps)) + for i, step := range steps { + names[i] = step.Service.Name } - return out + return names } func planSvc(cfg *Config, name string) Service { @@ -105,16 +106,16 @@ func TestPlanWaitsOnEveryDependency(t *testing.T) { )(Plan(cfg, nil)) } -// TestPlanWaitsOnAutoPorts: a `port: auto` dependency has a port to wait for, -// even though the file names no number. +// TestPlanWaitsOnAutoPorts: a `port: auto` dependency is still something to +// wait for, even though the file names no number. func TestPlanWaitsOnAutoPorts(t *testing.T) { cfg := &Config{Name: "demo", Services: []Service{ - {Name: "db", Cmd: "postgres", PortAuto: true}, - {Name: "api", Cmd: "uv run api", PortAuto: true, DependsOn: []string{"db"}}, + {Name: "postgres", Cmd: "postgres", PortAuto: true}, + {Name: "api", Cmd: "uv run api", PortAuto: true, DependsOn: []string{"postgres"}}, }} requirePlan(t, - planStep(cfg, "db", nil, nil), - planStep(cfg, "api", []string{"db"}, nil), + planStep(cfg, "postgres", nil, nil), + planStep(cfg, "api", []string{"postgres"}, nil), )(Plan(cfg, nil)) } @@ -139,16 +140,16 @@ func TestPlanUnknownOnly(t *testing.T) { requireUnknownAP(t, err) } -// TestPlanKeepsFileOrderAtTheSameDepth: two independent services start in the -// order the author wrote them. +// TestPlanKeepsFileOrderAtTheSameDepth: independent services start in the order +// the author wrote them. func TestPlanKeepsFileOrderAtTheSameDepth(t *testing.T) { cfg := &Config{Name: "demo", Services: []Service{ - {Name: "one"}, {Name: "two"}, {Name: "three"}, + {Name: "alpha"}, {Name: "beta"}, {Name: "gamma"}, }} requirePlan(t, - planStep(cfg, "one", nil, nil), - planStep(cfg, "two", nil, nil), - planStep(cfg, "three", nil, nil), + planStep(cfg, "alpha", nil, nil), + planStep(cfg, "beta", nil, nil), + planStep(cfg, "gamma", nil, nil), )(Plan(cfg, nil)) } diff --git a/engine/internal/groups/propose_test.go b/engine/internal/groups/propose_test.go index d1e5529d..83026d1f 100644 --- a/engine/internal/groups/propose_test.go +++ b/engine/internal/groups/propose_test.go @@ -108,18 +108,18 @@ func TestProposeReportsWhatIsListening(t *testing.T) { index.AddComposeProject("option-berth", f.composeIn) pp := []ports.ListeningPort{ - {Port: 8124, PID: 2, Process: "python3", Command: "python3 -m http.server 8124", Cwd: f.repoSub}, - {Port: 8123, PID: 1, Process: "python3", Command: "python3 -m http.server 8123", Cwd: f.repo}, - // The same listener on a second bind address must not repeat. - {Port: 8123, PID: 1, Process: "python3", Command: "python3 -m http.server 8123", Cwd: f.repo, BindAddress: "::1"}, // A Compose container started from inside the repo. {Port: 5432, PID: 9, Process: "com.docke", Type: ports.PortTypeDocker, DockerContainer: "option-berth-db-1", DockerImage: "postgres:17", DockerComposeService: "db", DockerComposeProject: "option-berth"}, - // All listeners whose cwd is inside the worktree are candidates; a process + // All listeners whose cwd is inside the checkout are candidates; a process // type is evidence, not a project ownership filter. {Port: 7000, PID: 3, Process: "Figma", Cwd: f.repo}, {Port: 22, PID: 4, Process: "sshd", Type: ports.PortTypeUser, Cwd: f.repo}, + {Port: 8124, PID: 2, Process: "python3", Command: "python3 -m http.server 8124", Cwd: f.repoSub}, + {Port: 8123, PID: 1, Process: "python3", Command: "python3 -m http.server 8123", Cwd: f.repo}, + // One listener seen on a second bind address is still one candidate. + {Port: 8123, PID: 1, Process: "python3", Command: "python3 -m http.server 8123", Cwd: f.repo, BindAddress: "::1"}, {Port: 9999, PID: 5, Process: "node", Cwd: f.composeOut}, } @@ -146,13 +146,13 @@ func TestProposeReportsWhatIsListening(t *testing.T) { # 8124 backend/http.server backend pid 2 # python3 -m http.server 8124 `) - path := filepath.Join(f.repo, ConfigName) - writeFile(t, path, string(data)) - loaded, err := Load(path) - if err != nil { - t.Fatalf("the proposed config does not validate: %v", err) + configPath := filepath.Join(f.repo, ConfigName) + writeFile(t, configPath, string(data)) + loaded, loadErr := Load(configPath) + if loadErr != nil { + t.Fatalf("the proposed config does not validate: %v", loadErr) } - requireLoadedProposal(t, loaded, "option-berth", path) + requireLoadedProposal(t, loaded, "option-berth", configPath) } // TestMarshalWritesDeclaredServices: a list the caller put on the config — a @@ -204,23 +204,23 @@ func TestMarshalQuotesANameYAMLWouldMisread(t *testing.T) { func TestProposeNamesWorktrees(t *testing.T) { f := newFixture(t) pp := []ports.ListeningPort{ - {Port: 3000, PID: 1, Process: "node", Cwd: f.worktree}, - {Port: 3001, PID: 2, Process: "node", Cwd: f.worktree}, - {Port: 3002, PID: 3, Process: "!!!", Cwd: f.worktree}, + {Port: 3010, PID: 1, Process: "node", Cwd: f.worktree}, + {Port: 3011, PID: 2, Process: "node", Cwd: f.worktree}, + {Port: 3012, PID: 3, Process: "!!!", Cwd: f.worktree}, } cfg := Propose(f.worktree, pp, nil) requireProposedConfig(t, cfg, "option-berth@feature-x", f.worktree, filepath.Join(f.worktree, ConfigName), []Candidate{ - {Port: 3000, Process: "node", PID: 1}, - {Port: 3001, Process: "node", PID: 2}, - {Port: 3002, Process: "!!!", PID: 3}, + {Port: 3010, Process: "node", PID: 1}, + {Port: 3011, Process: "node", PID: 2}, + {Port: 3012, Process: "!!!", PID: 3}, }) data, err := Marshal(cfg) if err != nil { t.Fatal(err) } - requireMarshal(t, data, "name: option-berth@feature-x\nservices: []\n"+starterComment+listeningCommentIntro+`# 3000 node . pid 1 -# 3001 node . pid 2 -# 3002 !!! . pid 3 + requireMarshal(t, data, "name: option-berth@feature-x\nservices: []\n"+starterComment+listeningCommentIntro+`# 3010 node . pid 1 +# 3011 node . pid 2 +# 3012 !!! . pid 3 `) } @@ -256,12 +256,12 @@ func TestProposeUsesProjectRootWhenTheComposeDirIsGone(t *testing.T) { DockerComposeService: "db", DockerComposeProject: "option-berth", ProjectRoot: f.repo}, // Attributed to a checkout that is not the one being reported on. - {Port: 6379, PID: 10, Process: "com.docke", Type: ports.PortTypeDocker, - DockerContainer: "other-cache-1", DockerComposeService: "cache", - DockerComposeProject: "other", ProjectRoot: f.composeOut}, + {Port: 6380, PID: 10, Process: "com.docke", Type: ports.PortTypeDocker, + DockerContainer: "elsewhere-redis-1", DockerComposeService: "redis", + DockerComposeProject: "elsewhere", ProjectRoot: f.composeOut}, } - // No index at all: the compose arm of portDir cannot answer. + // With no index at all, the compose arm of portDir cannot answer. cfg := Propose(f.repo, pp, nil) requireProposedConfig(t, cfg, "option-berth", f.repo, filepath.Join(f.repo, ConfigName), []Candidate{ {Port: 5432, Process: "db", PID: 9}, diff --git a/engine/internal/groups/refs_test.go b/engine/internal/groups/refs_test.go index d49db12f..3dae09e6 100644 --- a/engine/internal/groups/refs_test.go +++ b/engine/internal/groups/refs_test.go @@ -15,22 +15,23 @@ func TestExpand(t *testing.T) { Worktree: "wt_0123456789ab", Ports: map[string]int{"api": 21408, "db": 21407}, } - for _, tt := range []struct{ in, want string }{ + cases := []struct{ in, want string }{ {"--port ${port}", "--port 21408"}, {"${url}/healthz", "http://localhost:21408/healthz"}, {"postgres://localhost:${db.port}/app", "postgres://localhost:21407/app"}, {"${db.url}", "http://localhost:21407"}, {"app_${worktree}", "app_wt_0123456789ab"}, {".data/${worktree}/app.db", ".data/wt_0123456789ab/app.db"}, - // Not references: left exactly as written. + // Shell syntax is not one of our references and comes back untouched. {"${HOME}/bin", "${HOME}/bin"}, {"${FOO:-x}", "${FOO:-x}"}, {"$port", "$port"}, - // A reference to a service with no port in the map stays put. + // A service with no port in the map leaves its reference alone. {"${cache.port}", "${cache.port}"}, - } { - if got := Expand(tt.in, values); got != tt.want { - t.Errorf("Expand(%q) = %q, want %q", tt.in, got, tt.want) + } + for _, tc := range cases { + if got := Expand(tc.in, values); got != tc.want { + t.Errorf("Expand(%q) = %q, want %q", tc.in, got, tc.want) } } if got := URL(21408); got != "http://localhost:21408" { @@ -138,10 +139,11 @@ func TestRefs(t *testing.T) { } func TestMatchesExpanded(t *testing.T) { - for _, tt := range []struct { + type matchCase struct { pattern, actual string want bool - }{ + } + cases := []matchCase{ {"--port=${port}", "--port=8123", true}, {"--port=${port}", "--port=", false}, {"${port}", "8123", true}, @@ -149,12 +151,13 @@ func TestMatchesExpanded(t *testing.T) { {"--db=${worktree}", "--db=", false}, {"${HOME}", "${HOME}", true}, {"${HOME}", "/Users/me", false}, - {"dev", "dev", true}, - {"dev", "build", false}, + {"seed", "seed", true}, + {"seed", "build", false}, {"a.b${port}", "axb1", false}, - } { - if got := MatchesExpanded(tt.pattern, tt.actual); got != tt.want { - t.Errorf("MatchesExpanded(%q, %q) = %v, want %v", tt.pattern, tt.actual, got, tt.want) + } + for _, tc := range cases { + if got := MatchesExpanded(tc.pattern, tc.actual); got != tc.want { + t.Errorf("MatchesExpanded(%q, %q) = %v, want %v", tc.pattern, tc.actual, got, tc.want) } } } @@ -163,47 +166,47 @@ func TestLoadPortAutoAndEnv(t *testing.T) { cfg := mustLoad(t, "shop", ` name: shop services: - - name: db - cmd: docker compose up db + - name: store + cmd: docker compose up store port: auto - - name: api - cmd: uv run uvicorn app:app --port ${port} + - name: app + cmd: uv run uvicorn app.main:app --port ${port} port: auto - depends_on: [db] + depends_on: [store] env: - DATABASE_URL: postgres://localhost:${db.port}/app + DATABASE_URL: postgres://localhost:${store.port}/app HOME_BIN: ${HOME}/bin - - name: worker + - name: job cmd: uv run worker `) if cfg.Name != "shop" { t.Errorf("Name = %q, want shop", cfg.Name) } - requireNamedService(t, cfg, Service{Name: "db", Cmd: "docker compose up db", PortAuto: true}) + requireNamedService(t, cfg, Service{Name: "store", Cmd: "docker compose up store", PortAuto: true}) requireNamedService(t, cfg, Service{ - Name: "api", Cmd: "uv run uvicorn app:app --port ${port}", PortAuto: true, - DependsOn: []string{"db"}, - Env: map[string]string{"DATABASE_URL": "postgres://localhost:${db.port}/app", "HOME_BIN": "${HOME}/bin"}, + Name: "app", Cmd: "uv run uvicorn app.main:app --port ${port}", PortAuto: true, + DependsOn: []string{"store"}, + Env: map[string]string{"DATABASE_URL": "postgres://localhost:${store.port}/app", "HOME_BIN": "${HOME}/bin"}, }) - requireNamedService(t, cfg, Service{Name: "worker", Cmd: "uv run worker"}) + requireNamedService(t, cfg, Service{Name: "job", Cmd: "uv run worker"}) if !cfg.UsesAssignedPorts() || cfg.UsesManifestExtras() { t.Errorf("UsesAssignedPorts=%v UsesManifestExtras=%v, want true, false", cfg.UsesAssignedPorts(), cfg.UsesManifestExtras()) } requireMarshalDeclares(t, cfg, ""+ "name: shop\n"+ "services:\n"+ - " - name: db\n"+ - " cmd: docker compose up db\n"+ + " - name: store\n"+ + " cmd: docker compose up store\n"+ " port: auto\n"+ - " - name: api\n"+ - " cmd: uv run uvicorn app:app --port ${port}\n"+ + " - name: app\n"+ + " cmd: uv run uvicorn app.main:app --port ${port}\n"+ " port: auto\n"+ " depends_on:\n"+ - " - db\n"+ + " - store\n"+ " env:\n"+ - " DATABASE_URL: postgres://localhost:${db.port}/app\n"+ + " DATABASE_URL: postgres://localhost:${store.port}/app\n"+ " HOME_BIN: ${HOME}/bin\n"+ - " - name: worker\n"+ + " - name: job\n"+ " cmd: uv run worker\n") } diff --git a/engine/internal/groups/reload_test.go b/engine/internal/groups/reload_test.go index 20eee377..921bcdf9 100644 --- a/engine/internal/groups/reload_test.go +++ b/engine/internal/groups/reload_test.go @@ -10,11 +10,11 @@ import ( func writeConfig(t *testing.T, dir, body string) string { t.Helper() - path := filepath.Join(dir, ConfigName) - if err := os.WriteFile(path, []byte(body), 0o644); err != nil { - t.Fatal(err) + configPath := filepath.Join(dir, ConfigName) + if err := os.WriteFile(configPath, []byte(body), 0o644); err != nil { + t.Fatalf("write %s: %v", configPath, err) } - return path + return configPath } // manifestIndexState is the caller-visible result of a reload, not an @@ -124,10 +124,10 @@ func TestReloadPicksUpANewFile(t *testing.T) { } func TestReloadPicksUpAnEditAndADelete(t *testing.T) { - dir := t.TempDir() - path := writeConfig(t, dir, "name: demo\nservices:\n - name: api\n") + root := t.TempDir() + configPath := writeConfig(t, root, "name: demo\nservices:\n - name: api\n") x := NewIndex() - x.Observe(dir) + x.Observe(root) if _, found := x.Named("demo"); !found { t.Fatal("Observe did not index the config") } @@ -135,7 +135,7 @@ func TestReloadPicksUpAnEditAndADelete(t *testing.T) { t.Fatal("freshly observed manifest is stale") } requireConfig(t, x.Configs()[0], &Config{ - Name: "demo", Dir: Canonical(dir), Path: Canonical(path), + Name: "demo", Dir: Canonical(root), Path: Canonical(configPath), Services: []Service{{Name: "api"}}, }) for _, edit := range []struct { @@ -148,16 +148,16 @@ func TestReloadPicksUpAnEditAndADelete(t *testing.T) { } { t.Run(edit.title, func(t *testing.T) { if edit.remove { - if err := os.Remove(path); err != nil { + if err := os.Remove(configPath); err != nil { t.Fatal(err) } } else { - writeConfig(t, dir, edit.body) + writeConfig(t, root, edit.body) } if !x.Stale() { t.Fatal("changed manifest did not invalidate the index") } - assertManifestIndex(t, x, path, nil, edit.state) + assertManifestIndex(t, x, configPath, nil, edit.state) if _, found := x.Named("demo"); found { t.Error("the pre-edit name is still published") } @@ -184,19 +184,19 @@ func TestReloadReportsInvalidFiles(t *testing.T) { // TestKnownAndByPath are what the daemon's config handlers resolve through. func TestKnownAndByPath(t *testing.T) { - dir := t.TempDir() - path := writeConfig(t, dir, "name: demo\nservices:\n - name: api\n") + root := t.TempDir() + configPath := writeConfig(t, root, "name: demo\nservices:\n - name: api\n") x := NewIndex() - x.Observe(dir) + x.Observe(root) // Observe resolves symlinks, so on macOS the indexed path is the - // /private/var form of the temp dir; that is the path the daemon serves. + // /private/var spelling of the temp dir — the path the daemon serves. want := &Config{ - Name: "demo", Dir: Canonical(dir), Path: Canonical(path), + Name: "demo", Dir: Canonical(root), Path: Canonical(configPath), Services: []Service{{Name: "api"}}, } - if !reflect.DeepEqual(x.Known(), []string{Canonical(path)}) { - t.Fatalf("Known() = %v, want [%s]", x.Known(), Canonical(path)) + if !reflect.DeepEqual(x.Known(), []string{Canonical(configPath)}) { + t.Fatalf("Known() = %v, want [%s]", x.Known(), Canonical(configPath)) } if cfg, ok := x.ByPath(x.Known()[0]); !ok { t.Fatalf("ByPath(%s) missed the indexed path", x.Known()[0]) @@ -204,12 +204,12 @@ func TestKnownAndByPath(t *testing.T) { requireConfig(t, cfg, want) } // The path the user typed resolves too, symlinked temp dir and all. - if cfg, ok := x.ByPath(path); !ok { - t.Fatalf("ByPath(%s) missed the typed path", path) + if cfg, ok := x.ByPath(configPath); !ok { + t.Fatalf("ByPath(%s) missed the typed path", configPath) } else { requireConfig(t, cfg, want) } - if _, ok := x.ByPath(filepath.Join(dir, "nope.yaml")); ok { + if _, ok := x.ByPath(filepath.Join(root, "nope.yaml")); ok { t.Error("ByPath matched a file that is not indexed") } } diff --git a/engine/internal/groups/rename_test.go b/engine/internal/groups/rename_test.go index 2d136b31..728bda43 100644 --- a/engine/internal/groups/rename_test.go +++ b/engine/internal/groups/rename_test.go @@ -11,28 +11,29 @@ import ( "github.com/sheathedsharp/option-berth/engine/internal/state" ) -// renameWorld is a project with a main checkout and a worktree, both running, -// plus an unrelated repository called "store", resolved and built the way the -// daemon publishes them. +// renameWorld builds a project with a main checkout and a worktree, both +// running, plus an unrelated repository called "store" — all resolved and built +// the way the daemon publishes them. func renameWorld(t *testing.T, withConfig bool) (checkouts, *Index, []state.Group) { t.Helper() c := newCheckouts(t, "shop", "feat") if withConfig { + // The comment in the main checkout's file must survive the rename. writeFile(t, filepath.Join(c.main, ConfigName), "# the shop\nname: shop\nports: [8000]\n") writeFile(t, filepath.Join(c.wt, ConfigName), "name: shop\nports: [8000]\n") } - other := mkdir(t, filepath.Dir(c.main), "store") - mkdir(t, other, ".git") + storeRepo := mkdir(t, filepath.Dir(c.main), "store") + mkdir(t, storeRepo, ".git") x := NewIndex() - for _, dir := range []string{c.main, c.wt, other} { + for _, dir := range []string{c.main, c.wt, storeRepo} { x.Observe(dir) } + started := fakeRuns{7100: {group: "started", name: "job"}} pp := Resolve([]state.Port{ - nativePort(8000, c.main), nativePort(8001, c.wt), nativePort(9000, other), - }, fakeRuns{7100: {group: "started", name: "job"}}, x) - pp = append(pp, Resolve([]state.Port{nativePort(7100, c.main)}, - fakeRuns{7100: {group: "started", name: "job"}}, x)...) + nativePort(8000, c.main), nativePort(8001, c.wt), nativePort(9000, storeRepo), + }, started, x) + pp = append(pp, Resolve([]state.Port{nativePort(7100, c.main)}, started, x)...) return c, x, Groups(pp, x) } @@ -156,42 +157,42 @@ func TestValidProjectName(t *testing.T) { } for _, name := range []string{"shop", "my-app", "app_2.0"} { if err := ValidProjectName(name); err != nil { - t.Errorf("ValidProjectName(%q) = %v", name, err) + t.Errorf("ValidProjectName(%q) = %v, want it accepted", name, err) } } } func TestSetConfigNameKeepsTheFile(t *testing.T) { - dir := tempTree(t) - path := filepath.Join(dir, ConfigName) - writeFile(t, path, "# the shop\nname: shop # short\nservices:\n # first\n - name: api\n port: 8000\n") + root := tempTree(t) + path := filepath.Join(root, ConfigName) + writeFile(t, path, "# the shop\nname: shop # short\nservices:\n # the api\n - name: api\n port: 8000\n") - cfg, err := SetConfigName(path, "market") + renamed, err := SetConfigName(path, "bazaar") if err != nil { t.Fatalf("SetConfigName: %v", err) } - if cfg.Name != "market" { - t.Errorf("config name = %q, want market", cfg.Name) + if renamed.Name != "bazaar" { + t.Errorf("config name = %q, want bazaar", renamed.Name) } got, err := os.ReadFile(path) if err != nil { t.Fatalf("ReadFile: %v", err) } - const rewritten = "# the shop\nname: market # short\nservices:\n # first\n - name: api\n port: 8000\n" + const rewritten = "# the shop\nname: bazaar # short\nservices:\n # the api\n - name: api\n port: 8000\n" if string(got) != rewritten { - t.Errorf("rewritten file = %q, want comments and services kept around name: market", got) + t.Errorf("rewritten file = %q, want comments and services kept around name: bazaar", got) } bare := filepath.Join(tempTree(t), ConfigName) writeFile(t, bare, "services:\n - name: api\n") - if _, err := SetConfigName(bare, "market"); err != nil { + if _, err := SetConfigName(bare, "bazaar"); err != nil { t.Fatalf("SetConfigName without a name key: %v", err) } data, err := os.ReadFile(bare) if err != nil { t.Fatalf("ReadFile: %v", err) } - const inserted = "name: market\nservices:\n - name: api\n" + const inserted = "name: bazaar\nservices:\n - name: api\n" if string(data) != inserted { t.Errorf("name not inserted first:\n%s", data) } diff --git a/engine/internal/groups/resolve_test.go b/engine/internal/groups/resolve_test.go index e0e56264..085b91ca 100644 --- a/engine/internal/groups/resolve_test.go +++ b/engine/internal/groups/resolve_test.go @@ -8,22 +8,34 @@ import ( "github.com/sheathedsharp/option-berth/engine/internal/state" ) -type fakeRun struct{ group, name string } +// fakeRuns stands in for the daemon's registry: a port number maps to the run +// recorded against it. +type fakeRun struct { + group string + name string +} + type fakeRuns map[int]fakeRun func (f fakeRuns) Run(p state.Port) (state.Run, bool) { - r, ok := f[p.Port] - return state.Run{Group: r.group, Name: r.name}, ok + rec, ok := f[p.Port] + if !ok { + return state.Run{}, false + } + return state.Run{Group: rec.group, Name: rec.name}, true } -// resolveFixture builds a repo, a linked worktree, a Compose project inside the -// repo and a Compose project outside any repo, and returns the paths. +// fixture holds one of each checkout shape the resolver has to tell apart: a +// clone, a directory inside it, a linked worktree of it, and a Compose project +// both inside and outside the clone. type fixture struct { - repo string // a clone with .git - repoSub string // a directory inside the clone - worktree string // a linked worktree of the clone - composeIn string // compose working_dir inside the clone - composeOut string // compose working_dir outside any repo + // The clone, and a plain directory inside it. + repo string + repoSub string + // A linked worktree of the clone, and the two compose working_dirs. + worktree string + composeIn string + composeOut string } func newFixture(t *testing.T) fixture { @@ -31,28 +43,31 @@ func newFixture(t *testing.T) fixture { base := tempTree(t) repo := mkdir(t, base, "option-berth") mkdir(t, repo, ".git") + worktree := mkdir(t, base, "wt", "feature-x") f := fixture{ repo: repo, repoSub: mkdir(t, repo, "backend"), - worktree: mkdir(t, base, "wt", "feature-x"), + worktree: worktree, composeIn: mkdir(t, repo, "deploy"), composeOut: mkdir(t, base, "loose"), } - writeFile(t, filepath.Join(f.worktree, ".git"), + writeFile(t, filepath.Join(worktree, ".git"), "gitdir: "+filepath.Join(repo, ".git", "worktrees", "feature-x")+"\n") return f } func nativePort(port int, cwd string) state.Port { - return state.Port{Port: port, BindAddress: "127.0.0.1", Cwd: cwd, Process: "python3"} + return state.Port{Cwd: cwd, Port: port, BindAddress: "127.0.0.1", Process: "python3"} } func composePort(port int, project, service string) state.Port { - return state.Port{ - Port: port, BindAddress: "0.0.0.0", Process: "com.docke", - Type: state.TypeDocker, - Docker: &state.Docker{Container: project + "-" + service + "-1", ComposeProject: project, ComposeService: service}, + p := state.Port{Port: port, BindAddress: "0.0.0.0", Process: "com.docke", Type: state.TypeDocker} + p.Docker = &state.Docker{ + Container: project + "-" + service + "-1", + ComposeProject: project, + ComposeService: service, } + return p } type resolvedWant struct { diff --git a/engine/internal/groups/windows_paths_test.go b/engine/internal/groups/windows_paths_test.go index 36aa0280..77dea516 100644 --- a/engine/internal/groups/windows_paths_test.go +++ b/engine/internal/groups/windows_paths_test.go @@ -48,7 +48,7 @@ func requireWindows(t *testing.T, reason string) { func TestFindAcceptsEverySpellingOfTheSameCheckout(t *testing.T) { base := tempTree(t) - clone := mkdir(t, base, "api") + clone := mkdir(t, base, "gateway") mkdir(t, clone, ".git") deep := mkdir(t, clone, "services", "web") sep := string(filepath.Separator) @@ -66,7 +66,7 @@ func TestFindAcceptsEverySpellingOfTheSameCheckout(t *testing.T) { filepath.Join(deep, "..", "..", "services", "web"), } for _, cwd := range spellings { - requireFind(t, cwd, wantRoot, "", "api") + requireFind(t, cwd, wantRoot, "", "gateway") } root, worktree, ok := Find("") @@ -103,23 +103,25 @@ func TestCanonicalCollapsesTheSpellingsToOneKey(t *testing.T) { func TestWindowsPathShapes(t *testing.T) { requireWindows(t, "drive letters and backslashes are only path syntax on Windows") base := tempTree(t) - clone := mkdir(t, base, "api") + clone := mkdir(t, base, "gateway") mkdir(t, clone, ".git") deep := mkdir(t, clone, "services", "web") - if !strings.Contains(clone, `\`) || filepath.VolumeName(clone) == "" { + drive := filepath.VolumeName(clone) + if drive == "" || !strings.Contains(clone, `\`) { t.Fatalf("the temp checkout %q is not a drive-letter path", clone) } - // The PEB hands back a backslash path; a config or an env var may hand back - // the same directory with forward slashes, and Windows accepts both. + // The PEB hands back a backslash path, while a config or an env var may + // spell the same directory with forward slashes; Windows accepts both. forward := strings.ReplaceAll(deep, `\`, `/`) - requireSameCanonical(t, Canonical(deep), forward) - if got := Canonical(deep); strings.Contains(got, "/") { - t.Errorf("Canonical(%q) = %q, which still contains a forward slash", deep, got) + canonical := Canonical(deep) + requireSameCanonical(t, canonical, forward) + if strings.Contains(canonical, "/") { + t.Errorf("Canonical(%q) = %q, which still contains a forward slash", deep, canonical) } - // The drive letter's case is not a second project either. + // The drive letter's case does not make a second project either. lower := strings.ToLower(clone[:1]) + clone[1:] upper := strings.ToUpper(clone[:1]) + clone[1:] requireSameCanonical(t, Canonical(upper), lower) @@ -127,11 +129,11 @@ func TestWindowsPathShapes(t *testing.T) { // Find walks up with filepath.Dir, which has to stop at the drive root // rather than loop, and GroupName has to take the directory name off a // backslash path. - requireFind(t, forward, Canonical(clone), "", "api") + requireFind(t, forward, Canonical(clone), "", "gateway") - // A directory that is not in any checkout still terminates: the walk from a + // A directory outside any checkout still terminates: the walk up from a // drive root has nowhere left to go. - outside := filepath.VolumeName(clone) + `\` + outside := drive + `\` if _, _, ok := Find(outside); ok { if _, err := os.Stat(filepath.Join(outside, ".git")); err != nil { t.Errorf("Find(%q) claimed a checkout at the drive root", outside) From db8e2460dce52db3c10923dbb5c78cff70c9aa01 Mon Sep 17 00:00:00 2001 From: zayJu Date: Thu, 12 Nov 2026 12:01:54 +0800 Subject: [PATCH 3/3] =?UTF-8?q?docs(engine):=20=E8=AE=B0=E5=BD=95=20groups?= =?UTF-8?q?=20=E9=87=8D=E5=86=99=E5=90=8E=E7=9A=84=E4=B8=8A=E6=B8=B8?= =?UTF-8?q?=E5=A4=8D=E5=88=B6=E9=87=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 对照 26dbcae：总量 8,689 → 7,847 行 / 535 段、100 个文件（生产 3,142、 测试 4,705），internal/groups 整包归零。 Co-Authored-By: Claude Code --- engine/UPSTREAM.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/engine/UPSTREAM.md b/engine/UPSTREAM.md index 14aae37c..9392a61d 100644 --- a/engine/UPSTREAM.md +++ b/engine/UPSTREAM.md @@ -31,25 +31,25 @@ **9,621**（对照 `f4c50506`）、**9,659**（对照 `be48afbe`），生产 3,852、 测试 5,758。 -2026-11-12 的 ports 与 killer 重写在此基础上继续下降：对照 `26dbcae` 为 **8,689** -行 / 636 段，分布在 131 个文件（生产 3,478、测试 5,211）。 +2026-11-12 的 ports、killer 与 groups 重写在此基础上继续下降：对照 `26dbcae` 为 +**7,847** 行 / 535 段，分布在 100 个文件（生产 3,142、测试 4,705）。 -按包（对照 `26dbcae`，ports 与 killer 重写后）： +按包（对照 `26dbcae`，ports、killer 与 groups 重写后）： | 包 | 生产 | 测试 | 合计 | | --- | ---: | ---: | ---: | | `internal/daemon` | 2,073 | 3,457 | 5,530 | | `internal/cmd` | 837 | 703 | 1,540 | -| `internal/groups` | 336 | 506 | 842 | +| `internal/groups` | 0 | 0 | 0 | | `internal/scanner` | 220 | 540 | 760 | | `internal/ports` | 0 | 0 | 0 | | `internal/killer` | 0 | 0 | 0 | | `internal/store` | 12 | 5 | 17 | -`internal/claims`、`internal/display`、`internal/testenv`、`internal/ports`、 -`internal/killer` 与 `internal/store/claims*.go` 已经归零；`internal/docker` 只剩 -结构体字段与表驱动用例数据。`internal/store` 已整包重写：生产代码从 1,076 行降到 -199 行。 +`internal/claims`、`internal/display`、`internal/testenv`、`internal/groups`、 +`internal/ports`、`internal/killer` 与 `internal/store/claims*.go` 已经归零； +`internal/docker` 只剩结构体字段与表驱动用例数据。`internal/store` 已整包重写： +生产代码从 1,076 行降到 199 行。 ```sh git clone https://github.com/raskrebs/sonar /tmp/sonar