Repository: hashicorp/nomad
File: client/heartbeatstop_test.go (Go)

Code before:
		})))

	// send a heartbeat and make sure nothing changes
	stopper.setLastOk(time.Now())

	must.Wait(t, wait.ContinualSuccess(
		wait.Timeout(time.Second),
		wait.ErrorFunc(func() error {
			if destroyers[alloc1.ID].checkCalls() != 1 {
				return fmt.Errorf("first alloc should no longer be tracked")
			}
			if destroyers[alloc2.ID].checkCalls() != 0 {
				return fmt.Errorf("second alloc was unexpectedly destroyed")
			}
			if destroyers[alloc3.ID].checkCalls() != 0 {
				return fmt.Errorf("third alloc should never be destroyed")
			}
			return nil
		})))

	// set the last heartbeat into the past
	stopper.setLastOk(time.Now().Add(-1 * time.Hour))

	// update the second alloc so that we unblock without having to wait forever
	alloc2.Job.TaskGroups[0].Disconnect.StopOnClientAfter = pointer.Of(time.Microsecond)
	stopper.allocHook(alloc2)

	must.Wait(t, wait.InitialSuccess(
		wait.Timeout(time.Second),
		wait.ErrorFunc(func() error {
			if destroyers[alloc1.ID].checkCalls() != 1 {
				return fmt.Errorf("first alloc should no longer be tracked")
			}
			if destroyers[alloc2.ID].checkCalls() != 1 {
				return fmt.Errorf("second alloc should have been destroyed")
			}
			if destroyers[alloc3.ID].checkCalls() != 0 {
				return fmt.Errorf("third alloc should never be destroyed")
			}
			return nil
		})))
}

type mockAllocRunnerDestroyer struct {
	callsLock sync.Mutex
	calls     int
	interfaces.AllocRunner
}

func (ar *mockAllocRunnerDestroyer) checkCalls() int {
	ar.callsLock.Lock()

Review comment (suggestion):
this section has be furrowing eyebrows.

the second comment (line 108) is a little ambiguous to me: `setLastOk` sent to `heartbeatCh`, which unblocks the `select`, so it's not that we would wait forever for it to unblock the whole loop, but that we need the _timer_ to trip it, because that's what stops allocs.

and that's really tricky to test against a heartbeat in the past, because the `time.Microsecond` will always pass before line 113's `wait.Timeout(time.Second)` as long as the lastOK is less than one second _in the future_.

i.e. if line 106 is this, the test still passes:

```go
stopper.setLastOk(time.Now().Add(500 * time.Millisecond))
```

because it's more than a microsecond, but less than a second.

I'm not sure what to suggest here, but I don't think we're actually testing heartbeat timeout, specifically.

Code after:
			return nil
		})))

	// send a heartbeat and make sure nothing changes
	stopper.setLastOk(time.Now())

	must.Wait(t, wait.ContinualSuccess(
		wait.Timeout(200*time.Millisecond),
		wait.Gap(10*time.Millisecond),
		wait.ErrorFunc(func() error {
			if destroyers[alloc1.ID].checkCalls() != 1 {
				return errors.New("first alloc should no longer be tracked")
			}
			if destroyers[alloc2.ID].checkCalls() != 0 {
				return errors.New("second alloc was unexpectedly destroyed")
			}
			if destroyers[alloc3.ID].checkCalls() != 0 {
				return errors.New("third alloc should never be destroyed")
			}
			return nil
		})))

	// skip the next heartbeat

	must.Wait(t, wait.InitialSuccess(
		wait.Timeout(1*time.Second),
		wait.Gap(10*time.Millisecond),
		wait.ErrorFunc(func() error {
			if destroyers[alloc1.ID].checkCalls() != 1 {
				return errors.New("first alloc should no longer be tracked")
			}
			if destroyers[alloc2.ID].checkCalls() != 1 {
				return errors.New("second alloc should have been destroyed")
			}
			if destroyers[alloc3.ID].checkCalls() != 0 {
				return errors.New("third alloc should never be destroyed")
			}
			return nil
		})))
}

type mockAllocRunnerDestroyer struct {
	callsLock sync.Mutex
	calls     int
	interfaces.AllocRunner
}

func (ar *mockAllocRunnerDestroyer) checkCalls() int {
	ar.callsLock.Lock()
	defer ar.callsLock.Unlock()
	return ar.calls