From b2756f5e7ea3515b1714b221ea690a52769005ad Mon Sep 17 00:00:00 2001 From: Thales Maciel Date: Wed, 22 Apr 2026 17:45:43 -0300 Subject: [PATCH] test: add newTestDaemon harness + options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No Daemon test in this package has a shared constructor. Every file re-derives the same pattern — &Daemon{...}, wireServices(d), maybe override a field — which means new lifecycle / integration tests spend half their length standing up infrastructure instead of exercising behaviour. Consolidate into internal/daemon/daemon_testing_test.go: func newTestDaemon(t *testing.T, opts ...testDaemonOption) *Daemon Defaults: tempdir layout (distinct StateDir/ConfigDir/SSHDir/...), fresh store.Store with migrations auto-run, permissiveRunner, io.Discard logger, empty vmCaps (so default workDisk/dns/nat capabilities don't fire real side effects in tests that just want to exercise VMService plumbing). Options so far: - withRunner(system.CommandRunner) - withConfig(model.DaemonConfig) - withStore(*store.Store) - withLogger(*slog.Logger) - withLayout(paths.Layout) - withVMCaps(caps ...vmCapability) - withVsockHostDevice(string) withVMCaps tracks a vmCapsSet flag so tests that explicitly pass no caps (i.e. the default) still get the empty-slice behaviour — the reset after wireServices only fires when the caller didn't opt in. That keeps wireServices's production semantics unchanged: if you construct a real Daemon without pre-populating vmCaps, you still get the default three. Two smoke tests pin: - zero-option call wires every service, gives an empty-vmCaps daemon with the default vsock device, store non-nil - each option actually lands on the resulting Daemon (guards against silent rename) Existing tests unchanged — this is purely additive. Later slices (Firecracker error-path tests, store migration edges, lifecycle flow harness) will adopt the helper. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/daemon/daemon_testing_test.go | 241 +++++++++++++++++++++++++ 1 file changed, 241 insertions(+) create mode 100644 internal/daemon/daemon_testing_test.go diff --git a/internal/daemon/daemon_testing_test.go b/internal/daemon/daemon_testing_test.go new file mode 100644 index 0000000..00d7944 --- /dev/null +++ b/internal/daemon/daemon_testing_test.go @@ -0,0 +1,241 @@ +package daemon + +import ( + "bytes" + "io" + "log/slog" + "path/filepath" + "testing" + + "banger/internal/model" + "banger/internal/paths" + "banger/internal/store" + "banger/internal/system" +) + +// testDaemonOpts collects everything newTestDaemon knows how to +// override. Nothing is exported: the zero value is "sensible defaults", +// tests pick overrides by option function. +type testDaemonOpts struct { + runner system.CommandRunner + config *model.DaemonConfig + store *store.Store + logger *slog.Logger + layout *paths.Layout + vmCaps []vmCapability + vmCapsSet bool + vsockHostDevice string +} + +// testDaemonOption applies a single override to testDaemonOpts. Pass +// any combination to newTestDaemon; later options win on conflict. +type testDaemonOption func(*testDaemonOpts) + +// withRunner sets the system.CommandRunner used by HostNetwork, +// ImageService, WorkspaceService, and VMService. Most tests want +// permissiveRunner or scriptedRunner; the default is a permissive +// runner that returns empty output with no error. +func withRunner(r system.CommandRunner) testDaemonOption { + return func(o *testDaemonOpts) { o.runner = r } +} + +// withConfig replaces the DaemonConfig. Useful for exercising config- +// dependent code paths (bridge name, firecracker binary path, +// default image name, etc.) without going through config.Load. +func withConfig(cfg model.DaemonConfig) testDaemonOption { + return func(o *testDaemonOpts) { o.config = &cfg } +} + +// withStore reuses an externally-opened store instead of opening a +// fresh tempdir DB. Useful when the test needs to pre-seed rows +// before the daemon is wired. +func withStore(st *store.Store) testDaemonOption { + return func(o *testDaemonOpts) { o.store = st } +} + +// withLogger routes daemon logs somewhere specific. Default is +// io.Discard so a passing test run stays quiet; failing tests that +// want structured log content can pass their own buffer-backed slog. +func withLogger(l *slog.Logger) testDaemonOption { + return func(o *testDaemonOpts) { o.logger = l } +} + +// withLayout overrides the paths.Layout. Defaults build all dirs +// under t.TempDir() so tests don't interfere with each other and +// don't write into the user's real ~/.local/state/banger. +func withLayout(layout paths.Layout) testDaemonOption { + return func(o *testDaemonOpts) { o.layout = &layout } +} + +// withVMCaps installs a specific capability list on the daemon. +// Default is an empty slice, which means wireServices skips the +// built-in workDisk/dns/nat capabilities — most harness tests don't +// want those firing real side-effects. Pass capability fakes to +// exercise dispatch paths. +func withVMCaps(caps ...vmCapability) testDaemonOption { + return func(o *testDaemonOpts) { + o.vmCaps = caps + o.vmCapsSet = true + } +} + +// withVsockHostDevice overrides the /dev/vhost-vsock path VMService +// checks during preflight. Useful for tests that need RequireFile to +// succeed against a tempfile without root access to the real device. +func withVsockHostDevice(path string) testDaemonOption { + return func(o *testDaemonOpts) { o.vsockHostDevice = path } +} + +// newTestDaemon builds a wired *Daemon backed by tempdir state, +// ready for tests that drive service methods or dispatch logic. +// All infrastructure comes from either t.TempDir() or the +// provided overrides; nothing touches the invoking user's real +// state. +// +// What the harness gives you by default: +// +// - paths.Layout rooted at t.TempDir() (distinct StateDir, +// ConfigDir, CacheDir, VMsDir, ImagesDir, KernelsDir, SSHDir, +// KnownHostsPath) +// - fresh store.Store opened against a tempdir state.db with all +// migrations run, auto-closed on t.Cleanup +// - permissiveRunner returning empty output + no error for every +// Run/RunSudo call (override with scriptedRunner or any other +// system.CommandRunner when you need assertion-style scripting) +// - io.Discard logger (quiet tests) +// - empty vmCaps (so default capability side-effects don't fire) +// - defaultVsockHostDevice on VMService (tests that need this to +// resolve via RequireFile should pass withVsockHostDevice to a +// tempfile) +// +// Returns the wired *Daemon. Every service pointer is non-nil; +// d.store is non-nil; d.vmCaps is exactly what the test asked for. +func newTestDaemon(t *testing.T, opts ...testDaemonOption) *Daemon { + t.Helper() + applied := testDaemonOpts{} + for _, opt := range opts { + opt(&applied) + } + + layout := applied.layout + if layout == nil { + dir := t.TempDir() + layout = &paths.Layout{ + StateDir: filepath.Join(dir, "state"), + ConfigDir: filepath.Join(dir, "config"), + CacheDir: filepath.Join(dir, "cache"), + VMsDir: filepath.Join(dir, "state", "vms"), + ImagesDir: filepath.Join(dir, "state", "images"), + KernelsDir: filepath.Join(dir, "state", "kernels"), + SSHDir: filepath.Join(dir, "state", "ssh"), + KnownHostsPath: filepath.Join(dir, "state", "ssh", "known_hosts"), + DBPath: filepath.Join(dir, "state", "state.db"), + SocketPath: filepath.Join(dir, "state", "banger.sock"), + RuntimeDir: filepath.Join(dir, "runtime"), + } + } + + st := applied.store + if st == nil { + st = openDaemonStore(t) + } + + runner := applied.runner + if runner == nil { + runner = &permissiveRunner{} + } + + logger := applied.logger + if logger == nil { + logger = slog.New(slog.NewTextHandler(io.Discard, nil)) + } + + cfg := model.DaemonConfig{ + StatsPollInterval: model.DefaultStatsPollInterval, + BridgeName: model.DefaultBridgeName, + BridgeIP: model.DefaultBridgeIP, + CIDR: model.DefaultCIDR, + DefaultDNS: model.DefaultDNS, + } + if applied.config != nil { + cfg = *applied.config + } + + d := &Daemon{ + layout: *layout, + config: cfg, + store: st, + runner: runner, + logger: logger, + vmCaps: applied.vmCaps, + } + wireServices(d) + // wireServices fills in the default workDisk/dns/nat capability + // list when vmCaps is empty at call time — that's the production + // path. Harness callers who didn't opt in to capabilities via + // withVMCaps explicitly want them OFF so their test doesn't + // accidentally fire real NAT rules or a DNS publish. Reset to + // nil here; withVMCaps sets vmCapsSet to skip this reset. + if !applied.vmCapsSet { + d.vmCaps = nil + } + if applied.vsockHostDevice != "" { + d.vm.vsockHostDevice = applied.vsockHostDevice + } + return d +} + +// TestNewTestDaemonDefaults pins the contract new callers rely on: +// a zero-option call returns a fully-wired daemon with every service +// pointer populated, a writable tempdir-backed store, and an empty +// capability list (so nothing fires real side-effects). If any of +// those invariants drift, every test that switches to newTestDaemon +// will silently start exercising different behaviour. +func TestNewTestDaemonDefaults(t *testing.T) { + d := newTestDaemon(t) + + if d.net == nil || d.img == nil || d.ws == nil || d.vm == nil { + t.Fatalf("wireServices left a service nil: net=%v img=%v ws=%v vm=%v", + d.net != nil, d.img != nil, d.ws != nil, d.vm != nil) + } + if d.store == nil { + t.Fatal("store is nil; harness must provide a working store") + } + if len(d.vmCaps) != 0 { + t.Fatalf("vmCaps = %d, want 0 (harness default must not fire real capabilities)", len(d.vmCaps)) + } + if d.vm.vsockHostDevice != defaultVsockHostDevice { + t.Fatalf("vsockHostDevice = %q, want default %q", d.vm.vsockHostDevice, defaultVsockHostDevice) + } +} + +// TestNewTestDaemonOptionsOverride verifies the option functions +// actually land on the resulting Daemon. Guard against a silent +// rename breaking option plumbing. +func TestNewTestDaemonOptionsOverride(t *testing.T) { + var buf bytes.Buffer + customLogger := slog.New(slog.NewTextHandler(&buf, nil)) + customRunner := &countingRunner{} + customVsock := filepath.Join(t.TempDir(), "vhost-vsock") + customCap := testCapability{name: "marker"} + + d := newTestDaemon(t, + withLogger(customLogger), + withRunner(customRunner), + withVsockHostDevice(customVsock), + withVMCaps(customCap), + ) + + if d.logger != customLogger { + t.Error("withLogger: logger not overridden") + } + if d.runner != customRunner { + t.Error("withRunner: runner not overridden") + } + if d.vm.vsockHostDevice != customVsock { + t.Errorf("withVsockHostDevice: got %q, want %q", d.vm.vsockHostDevice, customVsock) + } + if len(d.vmCaps) != 1 || d.vmCaps[0].Name() != "marker" { + t.Errorf("withVMCaps: vmCaps = %v, want one 'marker' cap", d.vmCaps) + } +}