thaloco/banger
1
Fork 0
Code Pull requests Releases Packages Activity Actions

cli + daemon: move test seams off package globals onto injected structs

Browse source 
CLI: introduce internal/cli.deps which owns every RPC/SSH/host-command
seam the tree used to reach through mutable package vars. Command
builders, orchestrators, and the completion helpers become methods on
*deps. Tests construct their own deps per case, so fakes no longer leak
across cases and tests are free to run in parallel.

Daemon: move workspaceInspectRepoFunc + workspaceImportFunc onto the
Daemon struct (workspaceInspectRepo / workspaceImport), mirroring the
existing guestWaitForSSH / guestDial pattern. Workspace-prepare tests
drop t.Parallel() guards now that they no longer mutate process-wide
state.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Thales Maciel 2026-04-19 19:03:55 -03:00
parent d38f580e00
commit c42fcbe012
No known key found for this signature in database
GPG key ID: 33112E6833C34679
19 changed files with 664 additions and 733 deletions
Download patch file Download diff file Expand all files Collapse all files

3
internal/daemon/daemon.go
View file

@ -18,6 +18,7 @@ import (
"banger/internal/buildinfo"
"banger/internal/config"
"banger/internal/daemon/opstate"
ws "banger/internal/daemon/workspace"
"banger/internal/imagecat"
"banger/internal/imagepull"
"banger/internal/model"
@ -66,6 +67,8 @@ type Daemon struct {
guestWaitForSSH func(context.Context, string, string, time.Duration) error
guestDial func(context.Context, string, string) (guestSSHClient, error)
waitForGuestSessionReady func(context.Context, model.VMRecord, model.GuestSession) (model.GuestSession, error)
workspaceInspectRepo func(ctx context.Context, sourcePath, branchName, fromRef string) (ws.RepoSpec, error)
workspaceImport func(ctx context.Context, client ws.GuestClient, spec ws.RepoSpec, guestPath string, mode model.WorkspacePrepareMode) error
}
func Open(ctx context.Context) (d *Daemon, err error) {

30
internal/daemon/workspace.go
View file

@ -15,13 +15,25 @@ import (
"banger/internal/model"
)
// Test seams. Tests swap these to observe or stall the guest-I/O
// phase without needing a real git repo or SSH server. Production
// callers see the real implementations from the workspace package.
var (
workspaceInspectRepoFunc = ws.InspectRepo
workspaceImportFunc = ws.ImportRepoToGuest
)
// workspaceInspectRepoHook + workspaceImportHook dispatch through the
// per-instance Daemon seams when set, falling back to the real
// workspace package implementations. Keeping the fallbacks here (as
// opposed to always requiring callers to populate d.workspaceInspectRepo
// in a constructor) lets tests selectively override one hook without
// having to wire both.
func (d *Daemon) workspaceInspectRepoHook(ctx context.Context, sourcePath, branchName, fromRef string) (ws.RepoSpec, error) {
if d != nil && d.workspaceInspectRepo != nil {
return d.workspaceInspectRepo(ctx, sourcePath, branchName, fromRef)
}
return ws.InspectRepo(ctx, sourcePath, branchName, fromRef)
}
func (d *Daemon) workspaceImportHook(ctx context.Context, client ws.GuestClient, spec ws.RepoSpec, guestPath string, mode model.WorkspacePrepareMode) error {
if d != nil && d.workspaceImport != nil {
return d.workspaceImport(ctx, client, spec, guestPath, mode)
}
return ws.ImportRepoToGuest(ctx, client, spec, guestPath, mode)
}
func (d *Daemon) ExportVMWorkspace(ctx context.Context, params api.WorkspaceExportParams) (api.WorkspaceExportResult, error) {
guestPath := strings.TrimSpace(params.GuestPath)
@ -156,7 +168,7 @@ func (d *Daemon) PrepareVMWorkspace(ctx context.Context, params api.VMWorkspaceP
// inspect the local repo, dial SSH, stream the tar, optionally chmod
// readonly. It is called without holding the VM mutex.
func (d *Daemon) prepareVMWorkspaceGuestIO(ctx context.Context, vm model.VMRecord, sourcePath, guestPath, branchName, fromRef string, mode model.WorkspacePrepareMode, readOnly bool) (model.WorkspacePrepareResult, error) {
spec, err := workspaceInspectRepoFunc(ctx, sourcePath, branchName, fromRef)
spec, err := d.workspaceInspectRepoHook(ctx, sourcePath, branchName, fromRef)
if err != nil {
return model.WorkspacePrepareResult{}, err
}
@ -172,7 +184,7 @@ func (d *Daemon) prepareVMWorkspaceGuestIO(ctx context.Context, vm model.VMRecor
return model.WorkspacePrepareResult{}, fmt.Errorf("dial guest ssh: %w", err)
}
defer client.Close()
if err := workspaceImportFunc(ctx, client, spec, guestPath, mode); err != nil {
if err := d.workspaceImportHook(ctx, client, spec, guestPath, mode); err != nil {
return model.WorkspacePrepareResult{}, err
}
if readOnly {

33
internal/daemon/workspace_test.go
View file

@ -370,9 +370,7 @@ func TestExportVMWorkspace_MultipleChangedFiles(t *testing.T) {
// inside the import step and then asserts the VM mutex is acquirable
// while the prepare is mid-flight.
func TestPrepareVMWorkspace_ReleasesVMLockDuringGuestIO(t *testing.T) {
// Not parallel: mutates package-level workspaceInspectRepoFunc /
// workspaceImportFunc seams, which the other prepare-concurrency
// test also swaps.
t.Parallel()
ctx := context.Background()
apiSock := filepath.Join(t.TempDir(), "fc.sock")
@ -395,21 +393,15 @@ func TestPrepareVMWorkspace_ReleasesVMLockDuringGuestIO(t *testing.T) {
upsertDaemonVM(t, ctx, d.store, vm)
d.setVMHandlesInMemory(vm.ID, model.VMHandles{PID: firecracker.Process.Pid})
// Replace the seams. InspectRepo returns a trivial spec so the
// real filesystem isn't touched; Import blocks until we say go.
origInspect := workspaceInspectRepoFunc
origImport := workspaceImportFunc
t.Cleanup(func() {
workspaceInspectRepoFunc = origInspect
workspaceImportFunc = origImport
})
// Install the workspace seams on this daemon instance. InspectRepo
// returns a trivial spec so the real filesystem isn't touched;
// Import blocks until we say go.
importStarted := make(chan struct{})
releaseImport := make(chan struct{})
workspaceInspectRepoFunc = func(context.Context, string, string, string) (workspace.RepoSpec, error) {
d.workspaceInspectRepo = func(context.Context, string, string, string) (workspace.RepoSpec, error) {
return workspace.RepoSpec{RepoName: "fake", RepoRoot: "/tmp/fake"}, nil
}
workspaceImportFunc = func(context.Context, workspace.GuestClient, workspace.RepoSpec, string, model.WorkspacePrepareMode) error {
d.workspaceImport = func(context.Context, workspace.GuestClient, workspace.RepoSpec, string, model.WorkspacePrepareMode) error {
close(importStarted)
<-releaseImport
return nil
@ -465,7 +457,7 @@ func TestPrepareVMWorkspace_ReleasesVMLockDuringGuestIO(t *testing.T) {
// the workspaceLocks scope: two concurrent prepares on the same VM do
// NOT interleave, even though they no longer take the core VM mutex.
func TestPrepareVMWorkspace_SerialisesConcurrentPreparesOnSameVM(t *testing.T) {
// Not parallel: see note on ReleasesVMLockDuringGuestIO.
t.Parallel()
ctx := context.Background()
apiSock := filepath.Join(t.TempDir(), "fc.sock")
@ -488,14 +480,7 @@ func TestPrepareVMWorkspace_SerialisesConcurrentPreparesOnSameVM(t *testing.T) {
upsertDaemonVM(t, ctx, d.store, vm)
d.setVMHandlesInMemory(vm.ID, model.VMHandles{PID: firecracker.Process.Pid})
origInspect := workspaceInspectRepoFunc
origImport := workspaceImportFunc
t.Cleanup(func() {
workspaceInspectRepoFunc = origInspect
workspaceImportFunc = origImport
})
workspaceInspectRepoFunc = func(context.Context, string, string, string) (workspace.RepoSpec, error) {
d.workspaceInspectRepo = func(context.Context, string, string, string) (workspace.RepoSpec, error) {
return workspace.RepoSpec{RepoName: "fake", RepoRoot: "/tmp/fake"}, nil
}
@ -503,7 +488,7 @@ func TestPrepareVMWorkspace_SerialisesConcurrentPreparesOnSameVM(t *testing.T) {
var active int32
var maxObserved int32
release := make(chan struct{})
workspaceImportFunc = func(context.Context, workspace.GuestClient, workspace.RepoSpec, string, model.WorkspacePrepareMode) error {
d.workspaceImport = func(context.Context, workspace.GuestClient, workspace.RepoSpec, string, model.WorkspacePrepareMode) error {
n := atomic.AddInt32(&active, 1)
for {
prev := atomic.LoadInt32(&maxObserved)