adding tests for Detect struct in collector. Adding ability to mock out exec.Command calls.

collector/pkg/collector/metrics.go

@@ -4,7 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"github.com/analogj/scrutiny/collector/pkg/common"
+	"github.com/analogj/scrutiny/collector/pkg/common/shell"
 	"github.com/analogj/scrutiny/collector/pkg/config"
 	"github.com/analogj/scrutiny/collector/pkg/detect"
 	"github.com/analogj/scrutiny/collector/pkg/errors"
@@ -20,6 +20,7 @@ type MetricsCollector struct {
 	config config.Interface
 	BaseCollector
 	apiEndpoint *url.URL
+	shell       shell.Interface
 }
 
 func CreateMetricsCollector(appConfig config.Interface, logger *logrus.Entry, apiEndpoint string) (MetricsCollector, error) {
@@ -34,6 +35,7 @@ func CreateMetricsCollector(appConfig config.Interface, logger *logrus.Entry, ap
 		BaseCollector: BaseCollector{
 			logger: logger,
 		},
+		shell: shell.Create(),
 	}
 
 	return sc, nil
@@ -120,7 +122,7 @@ func (mc *MetricsCollector) Collect(deviceWWN string, deviceName string, deviceT
 	}
 	args = append(args, fmt.Sprintf("%s%s", detect.DevicePrefix(), deviceName))
 
-	result, err := common.ExecCmd(mc.logger, "smartctl", args, "", os.Environ())
+	result, err := mc.shell.Command(mc.logger, "smartctl", args, "", os.Environ())
 	resultBytes := []byte(result)
 	if err != nil {
 		if exitError, ok := err.(*exec.ExitError); ok {

collector/pkg/common/shell/factory.go

@@ -0,0 +1,5 @@
+package shell
+
+func Create() Interface {
+	return new(localShell)
+}

collector/pkg/common/shell/interface.go

@@ -0,0 +1,11 @@
+package shell
+
+import (
+	"github.com/sirupsen/logrus"
+)
+
+// Create mock using:
+// mockgen -source=collector/pkg/common/shell/interface.go -destination=collector/pkg/common/shell/mock/mock_shell.go
+type Interface interface {
+	Command(logger *logrus.Entry, cmdName string, cmdArgs []string, workingDir string, environ []string) (string, error)
+}

collector/pkg/common/shell/local_shell.go

@@ -1,4 +1,4 @@
-package common
+package shell
 
 import (
 	"bytes"
@@ -10,7 +10,9 @@ import (
 	"strings"
 )
 
-func ExecCmd(logger *logrus.Entry, cmdName string, cmdArgs []string, workingDir string, environ []string) (string, error) {
+type localShell struct{}
+
+func (s *localShell) Command(logger *logrus.Entry, cmdName string, cmdArgs []string, workingDir string, environ []string) (string, error) {
 	logger.Infof("Executing command: %s %s", cmdName, strings.Join(cmdArgs, " "))
 
 	cmd := exec.Command(cmdName, cmdArgs...)

collector/pkg/common/shell/local_shell_test.go

@@ -1,33 +1,33 @@
-package common_test
+package shell
 
 import (
-	"github.com/analogj/scrutiny/collector/pkg/common"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
 	"os/exec"
 	"testing"
 )
 
-func TestExecCmd(t *testing.T) {
+func TestLocalShellCommand(t *testing.T) {
 	t.Parallel()
 
 	//setup
-
+	testShell := localShell{}
 	//test
-	result, err := common.ExecCmd(logrus.WithField("exec", "test"), "echo", []string{"hello world"}, "", nil)
+	result, err := testShell.Command(logrus.WithField("exec", "test"), "echo", []string{"hello world"}, "", nil)
 
 	//assert
 	require.NoError(t, err)
 	require.Equal(t, "hello world\n", result)
 }
 
-func TestExecCmd_Date(t *testing.T) {
+func TestLocalShellCommand_Date(t *testing.T) {
 	t.Parallel()
 
 	//setup
+	testShell := localShell{}
 
 	//test
-	_, err := common.ExecCmd(logrus.WithField("exec", "test"), "date", []string{}, "", nil)
+	_, err := testShell.Command(logrus.WithField("exec", "test"), "date", []string{}, "", nil)
 
 	//assert
 	require.NoError(t, err)
@@ -51,13 +51,14 @@ func TestExecCmd_Date(t *testing.T) {
 //}
 //
 
-func TestExecCmd_InvalidCommand(t *testing.T) {
+func TestLocalShellCommand_InvalidCommand(t *testing.T) {
 	t.Parallel()
 
 	//setup
+	testShell := localShell{}
 
 	//test
-	_, err := common.ExecCmd(logrus.WithField("exec", "test"), "invalid_binary", []string{}, "", nil)
+	_, err := testShell.Command(logrus.WithField("exec", "test"), "invalid_binary", []string{}, "", nil)
 
 	//assert
 	_, castOk := err.(*exec.ExitError)

collector/pkg/common/shell/mock/mock_shell.go

@@ -0,0 +1,50 @@
+// Code generated by MockGen. DO NOT EDIT.
+// Source: collector/pkg/common/shell/interface.go
+
+// Package mock_shell is a generated GoMock package.
+package mock_shell
+
+import (
+	reflect "reflect"
+
+	gomock "github.com/golang/mock/gomock"
+	logrus "github.com/sirupsen/logrus"
+)
+
+// MockInterface is a mock of Interface interface.
+type MockInterface struct {
+	ctrl     *gomock.Controller
+	recorder *MockInterfaceMockRecorder
+}
+
+// MockInterfaceMockRecorder is the mock recorder for MockInterface.
+type MockInterfaceMockRecorder struct {
+	mock *MockInterface
+}
+
+// NewMockInterface creates a new mock instance.
+func NewMockInterface(ctrl *gomock.Controller) *MockInterface {
+	mock := &MockInterface{ctrl: ctrl}
+	mock.recorder = &MockInterfaceMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockInterface) EXPECT() *MockInterfaceMockRecorder {
+	return m.recorder
+}
+
+// Command mocks base method.
+func (m *MockInterface) Command(logger *logrus.Entry, cmdName string, cmdArgs []string, workingDir string, environ []string) (string, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Command", logger, cmdName, cmdArgs, workingDir, environ)
+	ret0, _ := ret[0].(string)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// Command indicates an expected call of Command.
+func (mr *MockInterfaceMockRecorder) Command(logger, cmdName, cmdArgs, workingDir, environ interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Command", reflect.TypeOf((*MockInterface)(nil).Command), logger, cmdName, cmdArgs, workingDir, environ)
+}

collector/pkg/detect/detect.go

@@ -3,7 +3,7 @@ package detect
 import (
 	"encoding/json"
 	"fmt"
-	"github.com/analogj/scrutiny/collector/pkg/common"
+	"github.com/analogj/scrutiny/collector/pkg/common/shell"
 	"github.com/analogj/scrutiny/collector/pkg/config"
 	"github.com/analogj/scrutiny/collector/pkg/models"
 	"github.com/analogj/scrutiny/webapp/backend/pkg/models/collector"
@@ -15,6 +15,7 @@ import (
 type Detect struct {
 	Logger *logrus.Entry
 	Config config.Interface
+	Shell  shell.Interface
 }
 
 //private/common functions
@@ -27,7 +28,7 @@ type Detect struct {
 // models.Device returned from this function only contain the minimum data for smartctl to execute: device type and device name (device file).
 func (d *Detect) SmartctlScan() ([]models.Device, error) {
 	//we use smartctl to detect all the drives available.
-	detectedDeviceConnJson, err := common.ExecCmd(d.Logger, "smartctl", []string{"--scan", "-j"}, "", os.Environ())
+	detectedDeviceConnJson, err := d.Shell.Command(d.Logger, "smartctl", []string{"--scan", "-j"}, "", os.Environ())
 	if err != nil {
 		d.Logger.Errorf("Error scanning for devices: %v", err)
 		return nil, err
@@ -58,7 +59,7 @@ func (d *Detect) SmartCtlInfo(device *models.Device) error {
 	}
 	args = append(args, fmt.Sprintf("%s%s", DevicePrefix(), device.DeviceName))
 
-	availableDeviceInfoJson, err := common.ExecCmd(d.Logger, "smartctl", args, "", os.Environ())
+	availableDeviceInfoJson, err := d.Shell.Command(d.Logger, "smartctl", args, "", os.Environ())
 	if err != nil {
 		d.Logger.Errorf("Could not retrieve device information for %s: %v", device.DeviceName, err)
 		return err

collector/pkg/detect/detect_test.go

@@ -1,14 +1,103 @@
 package detect_test
 
 import (
+	mock_shell "github.com/analogj/scrutiny/collector/pkg/common/shell/mock"
 	mock_config "github.com/analogj/scrutiny/collector/pkg/config/mock"
 	"github.com/analogj/scrutiny/collector/pkg/detect"
 	"github.com/analogj/scrutiny/collector/pkg/models"
 	"github.com/golang/mock/gomock"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
+	"io/ioutil"
 	"testing"
 )
 
+func TestDetect_SmartctlScan(t *testing.T) {
+	//setup
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	fakeConfig := mock_config.NewMockInterface(mockCtrl)
+	fakeConfig.EXPECT().GetString("host.id").AnyTimes().Return("")
+	fakeConfig.EXPECT().GetScanOverrides().AnyTimes().Return([]models.ScanOverride{})
+
+	fakeShell := mock_shell.NewMockInterface(mockCtrl)
+	testScanResults, err := ioutil.ReadFile("testdata/smartctl_scan_simple.json")
+	fakeShell.EXPECT().Command(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(string(testScanResults), err)
+
+	d := detect.Detect{
+		Logger: logrus.WithFields(logrus.Fields{}),
+		Shell:  fakeShell,
+		Config: fakeConfig,
+	}
+
+	//test
+	scannedDevices, err := d.SmartctlScan()
+
+	//assert
+	require.NoError(t, err)
+	require.Equal(t, 7, len(scannedDevices))
+	require.Equal(t, "scsi", scannedDevices[0].DeviceType)
+}
+
+func TestDetect_SmartctlScan_Megaraid(t *testing.T) {
+	//setup
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	fakeConfig := mock_config.NewMockInterface(mockCtrl)
+	fakeConfig.EXPECT().GetString("host.id").AnyTimes().Return("")
+	fakeConfig.EXPECT().GetScanOverrides().AnyTimes().Return([]models.ScanOverride{})
+
+	fakeShell := mock_shell.NewMockInterface(mockCtrl)
+	testScanResults, err := ioutil.ReadFile("testdata/smartctl_scan_megaraid.json")
+	fakeShell.EXPECT().Command(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(string(testScanResults), err)
+
+	d := detect.Detect{
+		Logger: logrus.WithFields(logrus.Fields{}),
+		Shell:  fakeShell,
+		Config: fakeConfig,
+	}
+
+	//test
+	scannedDevices, err := d.SmartctlScan()
+
+	//assert
+	require.NoError(t, err)
+	require.Equal(t, 2, len(scannedDevices))
+	require.Equal(t, []models.Device{
+		models.Device{DeviceName: "bus/0", DeviceType: "megaraid,0"},
+		models.Device{DeviceName: "bus/0", DeviceType: "megaraid,1"},
+	}, scannedDevices)
+}
+
+func TestDetect_SmartctlScan_Nvme(t *testing.T) {
+	//setup
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	fakeConfig := mock_config.NewMockInterface(mockCtrl)
+	fakeConfig.EXPECT().GetString("host.id").AnyTimes().Return("")
+	fakeConfig.EXPECT().GetScanOverrides().AnyTimes().Return([]models.ScanOverride{})
+
+	fakeShell := mock_shell.NewMockInterface(mockCtrl)
+	testScanResults, err := ioutil.ReadFile("testdata/smartctl_scan_nvme.json")
+	fakeShell.EXPECT().Command(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(string(testScanResults), err)
+
+	d := detect.Detect{
+		Logger: logrus.WithFields(logrus.Fields{}),
+		Shell:  fakeShell,
+		Config: fakeConfig,
+	}
+
+	//test
+	scannedDevices, err := d.SmartctlScan()
+
+	//assert
+	require.NoError(t, err)
+	require.Equal(t, 1, len(scannedDevices))
+	require.Equal(t, []models.Device{
+		models.Device{DeviceName: "nvme0", DeviceType: "nvme"},
+	}, scannedDevices)
+}
+
 func TestDetect_TransformDetectedDevices_Empty(t *testing.T) {
 	//setup
 	mockCtrl := gomock.NewController(t)

collector/pkg/detect/devices_darwin.go

@@ -1,6 +1,7 @@
 package detect
 
 import (
+	"github.com/analogj/scrutiny/collector/pkg/common/shell"
 	"github.com/analogj/scrutiny/collector/pkg/models"
 	"github.com/jaypipes/ghw"
 	"strings"
@@ -11,6 +12,7 @@ func DevicePrefix() string {
 }
 
 func (d *Detect) Start() ([]models.Device, error) {
+	d.Shell = shell.Create()
 	// call the base/common functionality to get a list of devicess
 	detectedDevices, err := d.SmartctlScan()
 	if err != nil {

collector/pkg/detect/devices_freebsd.go

@@ -1,6 +1,7 @@
 package detect
 
 import (
+	"github.com/analogj/scrutiny/collector/pkg/common/shell"
 	"github.com/analogj/scrutiny/collector/pkg/models"
 	"github.com/jaypipes/ghw"
 	"strings"
@@ -11,6 +12,7 @@ func DevicePrefix() string {
 }
 
 func (d *Detect) Start() ([]models.Device, error) {
+	d.Shell = shell.Create()
 	// call the base/common functionality to get a list of devices
 	detectedDevices, err := d.SmartctlScan()
 	if err != nil {

collector/pkg/detect/devices_linux.go

@@ -1,6 +1,7 @@
 package detect
 
 import (
+	"github.com/analogj/scrutiny/collector/pkg/common/shell"
 	"github.com/analogj/scrutiny/collector/pkg/models"
 	"github.com/jaypipes/ghw"
 	"strings"
@@ -11,6 +12,7 @@ func DevicePrefix() string {
 }
 
 func (d *Detect) Start() ([]models.Device, error) {
+	d.Shell = shell.Create()
 	// call the base/common functionality to get a list of devices
 	detectedDevices, err := d.SmartctlScan()
 	if err != nil {

collector/pkg/detect/devices_windows.go

@@ -1,6 +1,7 @@
 package detect
 
 import (
+	"github.com/analogj/scrutiny/collector/pkg/common/shell"
 	"github.com/analogj/scrutiny/collector/pkg/models"
 	"strings"
 )
@@ -10,6 +11,7 @@ func DevicePrefix() string {
 }
 
 func (d *Detect) Start() ([]models.Device, error) {
+	d.Shell = shell.Create()
 	// call the base/common functionality to get a list of devices
 	detectedDevices, err := d.SmartctlScan()
 	if err != nil {

collector/pkg/detect/testdata/smartctl_scan_megaraid.json

@@ -0,0 +1,35 @@
+{
+  "json_format_version": [
+    1,
+    0
+  ],
+  "smartctl": {
+    "version": [
+      7,
+      1
+    ],
+    "svn_revision": "5022",
+    "platform_info": "x86_64-linux-5.4.0-45-generic",
+    "build_info": "(local build)",
+    "argv": [
+      "smartctl",
+      "-j",
+      "--scan"
+    ],
+    "exit_status": 0
+  },
+  "devices": [
+    {
+      "name": "/dev/bus/0",
+      "info_name": "/dev/bus/0 [megaraid_disk_00]",
+      "type": "megaraid,0",
+      "protocol": "SCSI"
+    },
+    {
+      "name": "/dev/bus/0",
+      "info_name": "/dev/bus/0 [megaraid_disk_01]",
+      "type": "megaraid,1",
+      "protocol": "SCSI"
+    }
+  ]
+}

collector/pkg/detect/testdata/smartctl_scan_nvme.json

@@ -0,0 +1,29 @@
+{
+  "json_format_version": [
+    1,
+    0
+  ],
+  "smartctl": {
+    "version": [
+      7,
+      0
+    ],
+    "svn_revision": "4883",
+    "platform_info": "x86_64-linux-4.19.107-Unraid",
+    "build_info": "(local build)",
+    "argv": [
+      "smartctl",
+      "-j",
+      "--scan"
+    ],
+    "exit_status": 0
+  },
+  "devices": [
+    {
+      "name": "/dev/nvme0",
+      "info_name": "/dev/nvme0",
+      "type": "nvme",
+      "protocol": "NVMe"
+    }
+  ]
+}

collector/pkg/detect/testdata/smartctl_scan_simple.json

@@ -0,0 +1,65 @@
+{
+  "json_format_version": [
+    1,
+    0
+  ],
+  "smartctl": {
+    "version": [
+      7,
+      0
+    ],
+    "svn_revision": "4883",
+    "platform_info": "x86_64-linux-5.15.32-flatcar",
+    "build_info": "(local build)",
+    "argv": [
+      "smartctl",
+      "--scan",
+      "-j"
+    ],
+    "exit_status": 0
+  },
+  "devices": [
+    {
+      "name": "/dev/sda",
+      "info_name": "/dev/sda",
+      "type": "scsi",
+      "protocol": "SCSI"
+    },
+    {
+      "name": "/dev/sdb",
+      "info_name": "/dev/sdb",
+      "type": "scsi",
+      "protocol": "SCSI"
+    },
+    {
+      "name": "/dev/sdc",
+      "info_name": "/dev/sdc",
+      "type": "scsi",
+      "protocol": "SCSI"
+    },
+    {
+      "name": "/dev/sdd",
+      "info_name": "/dev/sdd",
+      "type": "scsi",
+      "protocol": "SCSI"
+    },
+    {
+      "name": "/dev/sde",
+      "info_name": "/dev/sde",
+      "type": "scsi",
+      "protocol": "SCSI"
+    },
+    {
+      "name": "/dev/sdf",
+      "info_name": "/dev/sdf",
+      "type": "scsi",
+      "protocol": "SCSI"
+    },
+    {
+      "name": "/dev/sdg",
+      "info_name": "/dev/sdg",
+      "type": "scsi",
+      "protocol": "SCSI"
+    }
+  ]
+}