gsadmin/spinifex
1
0
Fork 0
mirror of https://github.com/mulgadc/spinifex.git synced 2026-10-20 11:21:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

test(vm): remove tautologies and asserts-nothing tests

Browse Source 
manager_deps_test.go: drop TestDeps_VolumeMounterSatisfiesInterface,
TestDeps_StateStoreSatisfiesInterface, TestDeps_StateStoreErrorPropagates,
TestSetDeps_OverridesDeps, TestManagerHooks_InitiallyNil — they assert a
fake recorded its own input, that a fake returns an error literally
constructed in the test, or Go zero-value/direct-assignment semantics.
Compile-time interface checks already prove satisfaction.

state_test.go: replace TestEC2StateCodes_AllStatesHaveMapping and
TestEC2StateCodes_CorrectValues — both compared the production map to
its own literal. Now assert against an independent AWS contract using
ec2.InstanceStateName* SDK enum constants paired with the
AWS-documented numeric codes; a typo in production no longer
replicates into the test.

watchdog_test.go: replace TestStartPendingWatchdog_CtxCancelStopsGoroutine's
time.Sleep(10ms) "assertion" with goleak.VerifyNone. The previous form
passed even if the goroutine never returned on cancel — the harness
reaped it on test exit.
This commit is contained in:
Julian Sommer
2026-05-06 15:19:51 +10:00
parent 5b9a35914e
commit 3eff6ccff2
3 changed files with 54 additions and 129 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spinifex/vm/manager_deps_test.go
-93
View File
@@ -1,7 +1,6 @@
package vm
import (
"errors"
"maps"
"sync"
"testing"
@@ -105,37 +104,6 @@ func TestNewManagerWithDeps_StoresDeps(t *testing.T) {
}
}
func TestSetDeps_OverridesDeps(t *testing.T) {
m := NewManager()
if m.NodeID() != "" {
t.Fatalf("NodeID on default manager: got %q, want empty", m.NodeID())
}
deps := Deps{NodeID: "node-b"}
m.SetDeps(deps)
if m.NodeID() != "node-b" {
t.Fatalf("NodeID after SetDeps: got %q, want %q", m.NodeID(), "node-b")
}
// Replacing deps fully overwrites prior deps, not merges.
m.SetDeps(Deps{NodeID: "node-c"})
if m.NodeID() != "node-c" {
t.Fatalf("NodeID after second SetDeps: got %q, want %q", m.NodeID(), "node-c")
}
}
func TestManagerHooks_InitiallyNil(t *testing.T) {
// A manager constructed without hooks must expose nil hook fields so
// call sites can no-op rather than panicking.
m := NewManager()
if m.deps.Hooks.OnInstanceUp != nil {
t.Fatal("OnInstanceUp: got non-nil on default manager")
}
if m.deps.Hooks.OnInstanceDown != nil {
t.Fatal("OnInstanceDown: got non-nil on default manager")
}
}
// fakeVolumeMounter records every call so lifecycle tests can assert ordering.
// The mutex covers the recording slices so StopAll's per-instance fan-out
// goroutines stay race-free.
@@ -193,64 +161,3 @@ func (f *fakeVolumeMounter) UnmountOne(req types.EBSRequest) {
}
var _ VolumeMounter = (*fakeVolumeMounter)(nil)
func TestDeps_VolumeMounterSatisfiesInterface(t *testing.T) {
mounter := &fakeVolumeMounter{}
deps := Deps{VolumeMounter: mounter}
m := NewManagerWithDeps(deps)
if err := m.deps.VolumeMounter.Mount(&VM{ID: "i-1"}); err != nil {
t.Fatalf("Mount: %v", err)
}
if err := m.deps.VolumeMounter.Unmount(&VM{ID: "i-1"}); err != nil {
t.Fatalf("Unmount: %v", err)
}
if len(mounter.mounted) != 1 || mounter.mounted[0] != "i-1" {
t.Fatalf("mounted: got %v, want [i-1]", mounter.mounted)
}
if len(mounter.unmounted) != 1 || mounter.unmounted[0] != "i-1" {
t.Fatalf("unmounted: got %v, want [i-1]", mounter.unmounted)
}
}
func TestDeps_StateStoreSatisfiesInterface(t *testing.T) {
store := newFakeStateStore()
deps := Deps{NodeID: "n", StateStore: store}
m := NewManagerWithDeps(deps)
if err := m.deps.StateStore.WriteStoppedInstance("i-1", &VM{ID: "i-1"}); err != nil {
t.Fatalf("WriteStoppedInstance: %v", err)
}
if got := store.stopped["i-1"]; got == nil || got.ID != "i-1" {
t.Fatalf("WriteStoppedInstance: got %v, want VM{ID:i-1}", got)
}
if err := m.deps.StateStore.SaveRunningState("n", map[string]*VM{"i-1": {ID: "i-1"}}); err != nil {
t.Fatalf("SaveRunningState: %v", err)
}
loaded, err := m.deps.StateStore.LoadRunningState("n")
if err != nil {
t.Fatalf("LoadRunningState: %v", err)
}
if len(loaded) != 1 {
t.Fatalf("LoadRunningState: got %d entries, want 1", len(loaded))
}
}
// failStateStore returns errors so lifecycle tests can verify propagation
// when the manager wires through StateStore.
type failStateStore struct{ *fakeStateStore }
func (failStateStore) SaveRunningState(string, map[string]*VM) error {
return errors.New("save failed")
}
func TestDeps_StateStoreErrorPropagates(t *testing.T) {
deps := Deps{NodeID: "n", StateStore: failStateStore{newFakeStateStore()}}
m := NewManagerWithDeps(deps)
err := m.deps.StateStore.SaveRunningState("n", nil)
if err == nil {
t.Fatal("SaveRunningState: expected error from failStateStore, got nil")
}
}
spinifex/vm/state_test.go
+48 -29
View File
@@ -3,6 +3,7 @@ package vm
import (
"testing"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/stretchr/testify/assert"
)
@@ -20,40 +21,58 @@ func TestIsValidTransition(t *testing.T) {
assert.False(t, IsValidTransition(StateStopped, StateStopping))
}
func TestEC2StateCodes_AllStatesHaveMapping(t *testing.T) {
allStates := []InstanceState{
StateProvisioning,
StatePending,
StateRunning,
StateStopping,
StateStopped,
StateShuttingDown,
StateTerminated,
StateError,
}
// awsEC2StateCodes is the AWS-documented EC2 InstanceState code/name
// contract (see API_InstanceState). Names come from the SDK's enum
// constants; codes are fixed by the AWS API and have no SDK constant.
// Verifying our production map against an independent literal protects
// against a typo replicated identically in test and production.
var awsEC2StateCodes = map[string]int64{
ec2.InstanceStateNamePending: 0,
ec2.InstanceStateNameRunning: 16,
ec2.InstanceStateNameShuttingDown: 32,
ec2.InstanceStateNameTerminated: 48,
ec2.InstanceStateNameStopping: 64,
ec2.InstanceStateNameStopped: 80,
}
for _, s := range allStates {
func TestEC2StateCodes_AllInstanceStatesMapped(t *testing.T) {
for _, s := range []InstanceState{
StateProvisioning, StatePending, StateRunning, StateStopping,
StateStopped, StateShuttingDown, StateTerminated, StateError,
} {
info, ok := EC2StateCodes[s]
assert.True(t, ok, "State %s should have an EC2 mapping", s)
assert.NotEmpty(t, info.Name, "State %s EC2 name should not be empty", s)
assert.True(t, ok, "InstanceState %s missing from EC2StateCodes", s)
assert.NotEmpty(t, info.Name, "InstanceState %s mapped to empty EC2 name", s)
}
}
func TestEC2StateCodes_CorrectValues(t *testing.T) {
expected := map[InstanceState]EC2StateInfo{
StateProvisioning: {Code: 0, Name: "pending"},
StatePending: {Code: 0, Name: "pending"},
StateRunning: {Code: 16, Name: "running"},
StateStopping: {Code: 64, Name: "stopping"},
StateStopped: {Code: 80, Name: "stopped"},
StateShuttingDown: {Code: 32, Name: "shutting-down"},
StateTerminated: {Code: 48, Name: "terminated"},
StateError: {Code: 0, Name: "error"},
func TestEC2StateCodes_MatchAWSContract(t *testing.T) {
// Production states with a direct AWS equivalent must match the AWS
// (code, name) tuple exactly. State{Provisioning,Error} are
// Spinifex-only and verified separately below.
awsBacked := map[InstanceState]string{
StatePending: ec2.InstanceStateNamePending,
StateRunning: ec2.InstanceStateNameRunning,
StateStopping: ec2.InstanceStateNameStopping,
StateStopped: ec2.InstanceStateNameStopped,
StateShuttingDown: ec2.InstanceStateNameShuttingDown,
StateTerminated: ec2.InstanceStateNameTerminated,
}
for state, exp := range expected {
actual := EC2StateCodes[state]
assert.Equal(t, exp.Code, actual.Code, "State %s code mismatch", state)
assert.Equal(t, exp.Name, actual.Name, "State %s name mismatch", state)
for state, awsName := range awsBacked {
got := EC2StateCodes[state]
assert.Equal(t, awsName, got.Name,
"InstanceState %s name diverges from AWS contract", state)
assert.Equal(t, awsEC2StateCodes[awsName], got.Code,
"InstanceState %s code diverges from AWS contract", state)
}
}
func TestEC2StateCodes_SpinifexOnlyStatesPresentAsPending(t *testing.T) {
// Provisioning is a pre-launch internal state; it surfaces to AWS
// callers as "pending" so DescribeInstances returns a code AWS clients
// understand. Error is a terminal-ish failure state with no AWS
// equivalent — code 0 is the safest fallback ("pending" code).
assert.Equal(t, "pending", EC2StateCodes[StateProvisioning].Name)
assert.Equal(t, int64(0), EC2StateCodes[StateProvisioning].Code)
assert.Equal(t, int64(0), EC2StateCodes[StateError].Code)
}
spinifex/vm/watchdog_test.go
+6 -7
View File
@@ -8,6 +8,7 @@ import (
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
)
// pendingInstance returns a VM in the supplied state with LaunchTime set
@@ -116,18 +117,16 @@ func TestScanAndMarkStuckPending_OnlyPendingStatesScanned(t *testing.T) {
}
func TestStartPendingWatchdog_CtxCancelStopsGoroutine(t *testing.T) {
// goleak fails the test if the watchdog goroutine outlives ctx.
// Without this, a regression that ignored ctx.Done would still pass:
// the harness reaps the leaked goroutine on test process exit.
defer goleak.VerifyNone(t)
m, _, _, _ := crashTestManager(t)
ctx, cancel := context.WithCancel(t.Context())
m.StartPendingWatchdog(ctx)
// Immediately cancel; with PendingWatchdogInterval at 60s, the only
// observable signal that the goroutine respected ctx.Done is that the
// test exits cleanly without waiting for the next tick.
cancel()
// Yield briefly so the goroutine actually returns before the test
// finishes; race detector catches any leaked work.
time.Sleep(10 * time.Millisecond)
}
func assertStuckMarkedFailed(t *testing.T, m *Manager, rt *recordedTransitions, v *VM) {