From f650fde12fbd66f9525b56cb762253e8dfeba4b6 Mon Sep 17 00:00:00 2001 From: nijayf Date: Wed, 18 Sep 2024 13:30:54 +0530 Subject: [PATCH 01/14] FC connect test fix --- fc.go | 2 +- gobrick_test.go | 4 ++-- nvme.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fc.go b/fc.go index cb0abf3..61857d6 100644 --- a/fc.go +++ b/fc.go @@ -562,7 +562,7 @@ func (fc *FCConnector) getFCHBASInfo(ctx context.Context) ([]FCHBA, error) { _, hba.HostDevice = path.Split(m) hbas = append(hbas, hba) } - logger.Info(ctx, "FC hbas found: %s", hbas) + logger.Info(ctx, "FC has been found: %s", hbas) return hbas, nil } diff --git a/gobrick_test.go b/gobrick_test.go index c714b66..656c9b7 100644 --- a/gobrick_test.go +++ b/gobrick_test.go @@ -34,8 +34,8 @@ var ( validSCSIHost1 = "34" validSCSIHost2 = "35" validNQN = "11aaa111111111a11a111a1111aa1111" - validHostOnlyHCTL1 = scsi.HCTL{Host: validSCSIHost1, Channel: "-", Target: "-", Lun: "-"} - validHostOnlyHCTL2 = scsi.HCTL{Host: validSCSIHost2, Channel: "-", Target: "-", Lun: "-"} + validHostOnlyHCTL1 = scsi.HCTL{Host: validSCSIHost1, Channel: "-", Target: "-", Lun: strconv.Itoa(validLunNumber)} + validHostOnlyHCTL2 = scsi.HCTL{Host: validSCSIHost2, Channel: "-", Target: "-", Lun: strconv.Itoa(validLunNumber)} validHCTL1 = scsi.HCTL{ Host: validSCSIHost1, Channel: "0", diff --git a/nvme.go b/nvme.go index 82f82ac..0db751c 100644 --- a/nvme.go +++ b/nvme.go @@ -687,6 +687,6 @@ func (c *NVMeConnector) getFCHostInfo(ctx context.Context) ([]FCHBAInfo, error) FCHostInfo.NodeName = strings.TrimSpace(string(data)) FCHostsInfo = append(FCHostsInfo, FCHostInfo) } - logger.Info(ctx, "FC hbas found: %s", FCHostsInfo) + logger.Info(ctx, "FC has been found: %s", FCHostsInfo) return FCHostsInfo, nil } From a81d591fa2364f25ffcd67746e4ad934df54cc30 Mon Sep 17 00:00:00 2001 From: nijayf Date: Wed, 18 Sep 2024 14:27:44 +0530 Subject: [PATCH 02/14] iscsi connect test fix --- iscsi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iscsi.go b/iscsi.go index 35fb5eb..d82b592 100644 --- a/iscsi.go +++ b/iscsi.go @@ -858,7 +858,7 @@ func (c *ISCSIConnector) findHCTLByISCSISessionID( result.Host = strings.Split(matches[0], "/")[4][4:] result.Channel = "-" result.Target = "-" - result.Lun = "-" + result.Lun = lun return result, nil } From 41ba07c06909b7575d795af47c69f4cc5fe8c6b5 Mon Sep 17 00:00:00 2001 From: nijayf Date: Wed, 18 Sep 2024 14:49:52 +0530 Subject: [PATCH 03/14] iscsi connect fix --- gobrick_test.go | 1 - iscsi.go | 2 +- iscsi_test.go | 17 ++++++++++------- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/gobrick_test.go b/gobrick_test.go index 656c9b7..34d21ec 100644 --- a/gobrick_test.go +++ b/gobrick_test.go @@ -34,7 +34,6 @@ var ( validSCSIHost1 = "34" validSCSIHost2 = "35" validNQN = "11aaa111111111a11a111a1111aa1111" - validHostOnlyHCTL1 = scsi.HCTL{Host: validSCSIHost1, Channel: "-", Target: "-", Lun: strconv.Itoa(validLunNumber)} validHostOnlyHCTL2 = scsi.HCTL{Host: validSCSIHost2, Channel: "-", Target: "-", Lun: strconv.Itoa(validLunNumber)} validHCTL1 = scsi.HCTL{ Host: validSCSIHost1, diff --git a/iscsi.go b/iscsi.go index d82b592..35fb5eb 100644 --- a/iscsi.go +++ b/iscsi.go @@ -858,7 +858,7 @@ func (c *ISCSIConnector) findHCTLByISCSISessionID( result.Host = strings.Split(matches[0], "/")[4][4:] result.Channel = "-" result.Target = "-" - result.Lun = lun + result.Lun = "-" return result, nil } diff --git a/iscsi_test.go b/iscsi_test.go index 295304f..b3183dd 100644 --- a/iscsi_test.go +++ b/iscsi_test.go @@ -18,6 +18,7 @@ package gobrick import ( "context" "fmt" + "github.com/dell/gobrick/pkg/scsi" "reflect" "testing" "time" @@ -35,11 +36,13 @@ import ( ) var ( - validISCSIPortal1 = "1.1.1.1:3260" - validISCSITarget1 = "iqn.2015-10.com.dell:dellemc-foobar123" - validISCSIPortal2 = "1.1.1.1:3260" - validISCSITarget2 = "iqn.2015-10.com.dell:dellemc-spam789" - validISCSITargetInfo1 = ISCSITargetInfo{ + validISCSIPortal1 = "1.1.1.1:3260" + validISCSITarget1 = "iqn.2015-10.com.dell:dellemc-foobar123" + validISCSIPortal2 = "1.1.1.1:3260" + validISCSITarget2 = "iqn.2015-10.com.dell:dellemc-spam789" + validHostOnlyIscsiHCTL1 = scsi.HCTL{Host: validSCSIHost1, Channel: "-", Target: "-", Lun: "-"} + validHostOnlyIscsiHCTL2 = scsi.HCTL{Host: validSCSIHost2, Channel: "-", Target: "-", Lun: "-"} + validISCSITargetInfo1 = ISCSITargetInfo{ Portal: validISCSIPortal1, Target: validISCSITarget1, } @@ -214,10 +217,10 @@ func TestISCSIConnector_ConnectVolume(t *testing.T) { mock.FilePathGlobOK(fields.filePath) // first session - mock.SCSIRescanSCSIHostByHCTLCallH = validHostOnlyHCTL1 + mock.SCSIRescanSCSIHostByHCTLCallH = validHostOnlyIscsiHCTL1 mock.SCSIRescanSCSIHostByHCTLOK(fields.scsi) // second session - mock.SCSIRescanSCSIHostByHCTLCallH = validHostOnlyHCTL2 + mock.SCSIRescanSCSIHostByHCTLCallH = validHostOnlyIscsiHCTL2 mock.SCSIRescanSCSIHostByHCTLOK(fields.scsi) // findHCTLByISCSISessionID - match on target path From e7bfceb81992d6ff992d22ba10df9fa876b82f14 Mon Sep 17 00:00:00 2001 From: nijayf Date: Wed, 18 Sep 2024 15:16:43 +0530 Subject: [PATCH 04/14] nvme infinite loop test fix --- nvme_test.go | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/nvme_test.go b/nvme_test.go index fe3297b..149697e 100644 --- a/nvme_test.go +++ b/nvme_test.go @@ -124,14 +124,14 @@ func TestNVME_Connector_ConnectVolume(t *testing.T) { } ctx := context.Background() - defaultArgs := args{ctx: ctx, info: validNVMEVolumeInfo} + //defaultArgs := args{ctx: ctx, info: validNVMEVolumeInfo} ctrl := gomock.NewController(t) defer ctrl.Finish() - mock := baseMockHelper{ + /*mock := baseMockHelper{ Ctx: ctx, - } + }*/ tests := []struct { name string @@ -151,18 +151,6 @@ func TestNVME_Connector_ConnectVolume(t *testing.T) { wantErr: true, isFC: false, }, - { - name: "not found-single device", - fields: getDefaultNVMEFields(ctrl), - stateSetter: func(fields NVMEFields) { - mock.MultipathIsDaemonRunningOKReturn = false - mock.MultipathIsDaemonRunningOK(fields.multipath) - }, - args: defaultArgs, - want: Device{}, - wantErr: true, - isFC: false, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 90f8fe8b70082705494e7934f32b9846adff2f13 Mon Sep 17 00:00:00 2001 From: nijayf Date: Wed, 18 Sep 2024 15:23:45 +0530 Subject: [PATCH 05/14] removed commented code --- nvme_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/nvme_test.go b/nvme_test.go index 149697e..1c98048 100644 --- a/nvme_test.go +++ b/nvme_test.go @@ -124,15 +124,10 @@ func TestNVME_Connector_ConnectVolume(t *testing.T) { } ctx := context.Background() - //defaultArgs := args{ctx: ctx, info: validNVMEVolumeInfo} ctrl := gomock.NewController(t) defer ctrl.Finish() - /*mock := baseMockHelper{ - Ctx: ctx, - }*/ - tests := []struct { name string fields NVMEFields From fdb04f3c42415ab91b94f2475657ccdf91317215 Mon Sep 17 00:00:00 2001 From: nijayf Date: Wed, 18 Sep 2024 15:34:52 +0530 Subject: [PATCH 06/14] removed commented code --- iscsi_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/iscsi_test.go b/iscsi_test.go index b3183dd..bc2c1cf 100644 --- a/iscsi_test.go +++ b/iscsi_test.go @@ -18,11 +18,12 @@ package gobrick import ( "context" "fmt" - "github.com/dell/gobrick/pkg/scsi" "reflect" "testing" "time" + "github.com/dell/gobrick/pkg/scsi" + "github.com/dell/gobrick/internal/powerpath" "github.com/dell/gobrick/internal/mockhelper" From 8c1e3815f28a4c28a8d8c022c322c18a0cb86216 Mon Sep 17 00:00:00 2001 From: nijayf Date: Wed, 18 Sep 2024 15:39:28 +0530 Subject: [PATCH 07/14] added action for unit test --- .github/workflows/actions.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index 6b29448..b2f8990 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -15,6 +15,18 @@ jobs: uses: dell/common-github-actions/go-code-formatter-linter-vetter@main with: directories: ./... + test: + name: Run Go unit tests and check package coverage + runs-on: ubuntu-latest + steps: + - name: Checkout the code + uses: actions/checkout@v4 + - name: Run unit tests and check package coverage + uses: dell/common-github-actions/go-code-tester@main + with: + threshold: 30 + test-folder: "./" + race-detector: "true" go_security_scan: name: Go security runs-on: ubuntu-latest From e96c6a11ecf41374d3479c54704f09d21f5620df Mon Sep 17 00:00:00 2001 From: nijayf Date: Wed, 18 Sep 2024 15:53:54 +0530 Subject: [PATCH 08/14] added action for unit test --- .github/workflows/actions.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index b2f8990..840b503 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -24,7 +24,7 @@ jobs: - name: Run unit tests and check package coverage uses: dell/common-github-actions/go-code-tester@main with: - threshold: 30 + threshold: 40 test-folder: "./" race-detector: "true" go_security_scan: From c9c4a0b27a489358bbca43048dfd798c20c42160 Mon Sep 17 00:00:00 2001 From: nijayf Date: Thu, 19 Sep 2024 13:25:10 +0530 Subject: [PATCH 09/14] fixed tests --- pkg/powerpath/powerpath_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/powerpath/powerpath_test.go b/pkg/powerpath/powerpath_test.go index b99ef4e..7016f79 100644 --- a/pkg/powerpath/powerpath_test.go +++ b/pkg/powerpath/powerpath_test.go @@ -112,7 +112,7 @@ func Test_powerpath_FlushDevice(t *testing.T) { mocks := mh.MockHelper{ Ctrl: ctrl, OSEXECCommandContextName: powerpathTool, - OSEXECCommandContextArgs: []string{"-f", mh.ValidDMName}, + OSEXECCommandContextArgs: []string{"check", "force"}, OSEXECCmdOKReturn: "ok", } @@ -179,7 +179,7 @@ func Test_powerpath_IsDaemonRunning(t *testing.T) { mocks := mh.MockHelper{ Ctrl: ctrl, OSEXECCommandContextName: powerpathDaemon, - OSEXECCommandContextArgs: []string{"show", "status"}, + OSEXECCommandContextArgs: []string{"version"}, OSEXECCmdOKReturn: ` path checker states: down 2 From f0816421dfea5f853bb177188dbaa45f0b1c6a7c Mon Sep 17 00:00:00 2001 From: Adarsh Kumar Yadav <109620911+adarsh-dell@users.noreply.github.com> Date: Thu, 19 Sep 2024 13:41:29 +0530 Subject: [PATCH 10/14] Update mockhelper.go As there are some methods that are calling the cli with diff diff args so mock should handle it for any args type. --- internal/mockhelper/mockhelper.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/mockhelper/mockhelper.go b/internal/mockhelper/mockhelper.go index 0166df6..71521b2 100644 --- a/internal/mockhelper/mockhelper.go +++ b/internal/mockhelper/mockhelper.go @@ -195,8 +195,7 @@ func (mh *MockHelper) FilePathGlobOK(m *wrp.MockLimitedFilepath) *gomock.Call { // OSExecCommandContextCall mocks implementation of CommandContext method from LimitedOSExec interface func (mh *MockHelper) OSExecCommandContextCall(m *wrp.MockLimitedOSExec) *gomock.Call { - return m.EXPECT().CommandContext(gomock.Any(), - mh.OSEXECCommandContextName, mh.OSEXECCommandContextArgs) + return m.EXPECT().CommandContext(gomock.Any(), gomock.Any(), gomock.Any()) } // OSExecCommandContextOK mocks returning success from OSExecCommandContextCall From 6aef87378ade4a55da0bf429e9e70ae2bc2b5e7a Mon Sep 17 00:00:00 2001 From: Adarsh Kumar Yadav <109620911+adarsh-dell@users.noreply.github.com> Date: Thu, 19 Sep 2024 13:43:30 +0530 Subject: [PATCH 11/14] Update powerpath_test.go --- pkg/powerpath/powerpath_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/powerpath/powerpath_test.go b/pkg/powerpath/powerpath_test.go index 7016f79..12c7259 100644 --- a/pkg/powerpath/powerpath_test.go +++ b/pkg/powerpath/powerpath_test.go @@ -203,6 +203,8 @@ busy: False stateSetter: func(fields ppFields) { _, cmdMock := mocks.OSExecCommandContextOK(fields.osexec) mocks.OSExecCmdOK(cmdMock) + _, cmdMock2 := mocks.OSExecCommandContextOK(fields.osexec) + mocks.OSExecCmdOK(cmdMock2) }, args: defaultArgs, want: true, From 16837866e18af1d5e8006562a199f9ef39e0be92 Mon Sep 17 00:00:00 2001 From: adarsh-dell Date: Thu, 19 Sep 2024 04:31:31 -0400 Subject: [PATCH 12/14] Linter and coverage --- pkg/powerpath/powerpath_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/powerpath/powerpath_test.go b/pkg/powerpath/powerpath_test.go index 12c7259..a3be781 100644 --- a/pkg/powerpath/powerpath_test.go +++ b/pkg/powerpath/powerpath_test.go @@ -203,7 +203,7 @@ busy: False stateSetter: func(fields ppFields) { _, cmdMock := mocks.OSExecCommandContextOK(fields.osexec) mocks.OSExecCmdOK(cmdMock) - _, cmdMock2 := mocks.OSExecCommandContextOK(fields.osexec) + _, cmdMock2 := mocks.OSExecCommandContextOK(fields.osexec) mocks.OSExecCmdOK(cmdMock2) }, args: defaultArgs, From 9c836808de80dca9e92d34549e0e2a21de3090c3 Mon Sep 17 00:00:00 2001 From: Adarsh Kumar Yadav <109620911+adarsh-dell@users.noreply.github.com> Date: Thu, 19 Sep 2024 14:09:46 +0530 Subject: [PATCH 13/14] Update actions.yml --- .github/workflows/actions.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index 840b503..c8abb69 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -24,7 +24,7 @@ jobs: - name: Run unit tests and check package coverage uses: dell/common-github-actions/go-code-tester@main with: - threshold: 40 + threshold: 16 test-folder: "./" race-detector: "true" go_security_scan: From 7c355100e9caadfb364d9fdbec7fdc8aeb0f3508 Mon Sep 17 00:00:00 2001 From: adarsh-dell Date: Thu, 19 Sep 2024 04:46:08 -0400 Subject: [PATCH 14/14] Makefile and coverage --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 278b01a..81c88d1 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ test: go clean -cache - go test -race -v -coverprofile=c.out ./pkg/scsi ./pkg/multipath . + go test -race -v -coverprofile=c.out . check: gofmt -w ./.