From e12a3326147950f488e74e1f93d303b382bec306 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Thu, 26 Sep 2024 14:51:54 -0700 Subject: [PATCH] simplify presence detector --- ee/presencedetection/mocks/detectorIface.go | 52 +++++++++ ee/presencedetection/presencedetection.go | 33 ++++-- .../presencedetection_test.go | 105 +++++++++++------- 3 files changed, 137 insertions(+), 53 deletions(-) create mode 100644 ee/presencedetection/mocks/detectorIface.go diff --git a/ee/presencedetection/mocks/detectorIface.go b/ee/presencedetection/mocks/detectorIface.go new file mode 100644 index 000000000..19bd705d4 --- /dev/null +++ b/ee/presencedetection/mocks/detectorIface.go @@ -0,0 +1,52 @@ +// Code generated by mockery v2.44.1. DO NOT EDIT. + +package mocks + +import mock "github.com/stretchr/testify/mock" + +// DetectorIface is an autogenerated mock type for the detectorIface type +type DetectorIface struct { + mock.Mock +} + +// Detect provides a mock function with given fields: reason +func (_m *DetectorIface) Detect(reason string) (bool, error) { + ret := _m.Called(reason) + + if len(ret) == 0 { + panic("no return value specified for Detect") + } + + var r0 bool + var r1 error + if rf, ok := ret.Get(0).(func(string) (bool, error)); ok { + return rf(reason) + } + if rf, ok := ret.Get(0).(func(string) bool); ok { + r0 = rf(reason) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(reason) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewDetectorIface creates a new instance of DetectorIface. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewDetectorIface(t interface { + mock.TestingT + Cleanup(func()) +}) *DetectorIface { + mock := &DetectorIface{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/ee/presencedetection/presencedetection.go b/ee/presencedetection/presencedetection.go index 0cb2ab57f..95f3abe55 100644 --- a/ee/presencedetection/presencedetection.go +++ b/ee/presencedetection/presencedetection.go @@ -9,11 +9,21 @@ import ( const DetectionFailedDurationValue = -1 * time.Second type PresenceDetector struct { - lastDetectionUTC time.Time - mutext sync.Mutex - hadOneSuccessfulDetection bool - // detectFunc that can be set for testing - detectFunc func(string) (bool, error) + lastDetectionUTC time.Time + mutext sync.Mutex + // detector is an interface to allow for mocking in tests + detector detectorIface +} + +// just exists for testing purposes +type detectorIface interface { + Detect(reason string) (bool, error) +} + +type detector struct{} + +func (d *detector) Detect(reason string) (bool, error) { + return Detect(reason) } // DetectPresence checks if the user is present by detecting the presence of a user. @@ -22,19 +32,21 @@ func (pd *PresenceDetector) DetectPresence(reason string, detectionInterval time pd.mutext.Lock() defer pd.mutext.Unlock() - if pd.detectFunc == nil { - pd.detectFunc = Detect + if pd.detector == nil { + pd.detector = &detector{} } + hadHadSuccessfulDetection := pd.lastDetectionUTC != time.Time{} + // Check if the last detection was within the detection interval - if pd.hadOneSuccessfulDetection && time.Since(pd.lastDetectionUTC) < detectionInterval { + if hadHadSuccessfulDetection && time.Since(pd.lastDetectionUTC) < detectionInterval { return time.Since(pd.lastDetectionUTC), nil } - success, err := pd.detectFunc(reason) + success, err := pd.detector.Detect(reason) switch { - case err != nil && pd.hadOneSuccessfulDetection: + case err != nil && hadHadSuccessfulDetection: return time.Since(pd.lastDetectionUTC), fmt.Errorf("detecting presence: %w", err) case err != nil: // error without initial successful detection @@ -42,7 +54,6 @@ func (pd *PresenceDetector) DetectPresence(reason string, detectionInterval time case success: pd.lastDetectionUTC = time.Now().UTC() - pd.hadOneSuccessfulDetection = true return 0, nil default: // failed detection without error, maybe not possible? diff --git a/ee/presencedetection/presencedetection_test.go b/ee/presencedetection/presencedetection_test.go index 88c7e583a..6ed82f2fb 100644 --- a/ee/presencedetection/presencedetection_test.go +++ b/ee/presencedetection/presencedetection_test.go @@ -2,85 +2,106 @@ package presencedetection import ( "errors" + "math" "testing" "time" + "github.com/kolide/launcher/ee/presencedetection/mocks" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) func TestPresenceDetector_DetectPresence(t *testing.T) { t.Parallel() tests := []struct { - name string - interval time.Duration - detectFunc func(string) (bool, error) - initialLastDetectionUTC time.Time - hadOneSuccessfulDetection bool - errAssertion assert.ErrorAssertionFunc - expectedLastDetectionDelta time.Duration + name string + interval time.Duration + detector func(t *testing.T) detectorIface + initialLastDetectionUTC time.Time + expectError bool }{ { - name: "first detection success", - detectFunc: func(string) (bool, error) { - return true, nil + name: "first detection success", + interval: 0, + detector: func(t *testing.T) detectorIface { + d := mocks.NewDetectorIface(t) + d.On("Detect", mock.AnythingOfType("string")).Return(true, nil) + return d }, - errAssertion: assert.NoError, - expectedLastDetectionDelta: 0, }, { - name: "detection within interval", - detectFunc: func(string) (bool, error) { - return false, errors.New("should not have called detectFunc, since within interval") + name: "detection outside interval", + interval: time.Minute, + detector: func(t *testing.T) detectorIface { + d := mocks.NewDetectorIface(t) + d.On("Detect", mock.AnythingOfType("string")).Return(true, nil) + return d }, - errAssertion: assert.NoError, - initialLastDetectionUTC: time.Now().UTC(), - interval: time.Minute, - hadOneSuccessfulDetection: true, + initialLastDetectionUTC: time.Now().UTC().Add(-time.Minute), }, { - name: "error first detection", - detectFunc: func(string) (bool, error) { - return false, errors.New("error") + name: "detection within interval", + interval: time.Minute, + detector: func(t *testing.T) detectorIface { + // should not be called, will get error if it is + return mocks.NewDetectorIface(t) }, - errAssertion: assert.Error, - expectedLastDetectionDelta: -1, + initialLastDetectionUTC: time.Now().UTC(), }, { - name: "error after first detection", - detectFunc: func(string) (bool, error) { - return false, errors.New("error") + name: "error first detection", + interval: 0, + detector: func(t *testing.T) detectorIface { + d := mocks.NewDetectorIface(t) + d.On("Detect", mock.AnythingOfType("string")).Return(true, errors.New("error")) + return d }, - errAssertion: assert.Error, - initialLastDetectionUTC: time.Now().UTC(), - hadOneSuccessfulDetection: true, + expectError: true, }, { - name: "detection failed without OS error", - detectFunc: func(string) (bool, error) { - return false, nil + name: "error after first detection", + interval: 0, + detector: func(t *testing.T) detectorIface { + d := mocks.NewDetectorIface(t) + d.On("Detect", mock.AnythingOfType("string")).Return(true, errors.New("error")) + return d }, - errAssertion: assert.Error, - initialLastDetectionUTC: time.Now().UTC(), - hadOneSuccessfulDetection: true, + initialLastDetectionUTC: time.Now().UTC(), + expectError: true, + }, + { + name: "detection failed without OS error", + interval: 0, + detector: func(t *testing.T) detectorIface { + d := mocks.NewDetectorIface(t) + d.On("Detect", mock.AnythingOfType("string")).Return(false, nil) + return d + }, + initialLastDetectionUTC: time.Now().UTC(), + expectError: true, }, } + for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() pd := &PresenceDetector{ - detectFunc: tt.detectFunc, - lastDetectionUTC: tt.initialLastDetectionUTC, - hadOneSuccessfulDetection: tt.hadOneSuccessfulDetection, + detector: tt.detector(t), + lastDetectionUTC: tt.initialLastDetectionUTC, } timeSinceLastDetection, err := pd.DetectPresence("this is a test", tt.interval) - tt.errAssertion(t, err) - delta := timeSinceLastDetection - tt.expectedLastDetectionDelta - assert.LessOrEqual(t, delta, time.Second) + if tt.expectError { + assert.Error(t, err) + return + } + + absDelta := math.Abs(timeSinceLastDetection.Seconds() - tt.interval.Seconds()) + assert.LessOrEqual(t, absDelta, tt.interval.Seconds()) }) } }