Repository: kubernetes-csi/csi-driver-smb
Pull request #123 (2020-09-26 13:54:09)

test: enable windows unit tests with `csi-proxy.exe` 
Signed-off-by: Mayank Shah <mayankshah1614@gmail.com>

<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://git.k8s.io/community/contributors/devel/release.md#issue-kind-label
3. Ensure you have added or ran the appropriate tests for your PR: https://git.k8s.io/community/contributors/devel/testing.md
4. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
5. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
6. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
/kind test

<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind design

Optionally add one or more of the following kinds if applicable:
/kind api-change
/kind deprecation
/kind failing-test
/kind flake
/kind regression
-->

**What this PR does / why we need it**:
test: enable windows unit tests with `csi-proxy.exe` 

**Which issue(s) this PR fixes**:
https://github.com/kubernetes-csi/csi-driver-smb/issues/121
https://github.com/kubernetes-csi/csi-driver-smb/issues/109
<!-- 
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

**Requirements**:
- [x] uses [conventional commit messages](https://www.conventionalcommits.org/)
  <!-- Common commit types:
        build: Build 🏭
        chore: Maintenance 🔧
        ci: Continuous Integration 💜
        docs: Documentation 📘
        feat: Features 🌈
        fix: Bug Fixes 🐞
        perf: Performance Improvements 🚀
        refactor: Code Refactoring 💎
        revert: Revert Change ◀️
        style: Code Style 🎶
        security: Security Fix 🛡️
        test: Testing 💚 -->
- [x] includes documentation
- [x] adds unit tests
- [ ] tested upgrade from previous version

**Special notes for your reviewer**:


**Release note**:
```
none
```


k8s-ci-robot: @mayankshah1607: The label(s) `kind/test` cannot be applied, because the repository doesn't have them

<details>

In response to [this](https://github.com/kubernetes-csi/csi-driver-smb/pull/123):

>Signed-off-by: Mayank Shah <mayankshah1614@gmail.com>
>
>
>
>**What type of PR is this?**
>/kind test
>
>
>
>**What this PR does / why we need it**:
>fix: `TestNodeStageVolume` on Windows
>
>**Which issue(s) this PR fixes**:
>
>Fixes #
>
>**Requirements**:
>- [ ] uses [conventional commit messages](https://www.conventionalcommits.org/)
>  
>- [ ] includes documentation
>- [ ] adds unit tests
>- [ ] tested upgrade from previous version
>
>**Special notes for your reviewer**:
>
>
>**Release note**:
>```
>none
>```
>


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-csi/csi-driver-smb/pull/123#" title="Author self-approved">mayankshah1607</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **andyzhangx**
You can assign the PR to them by writing `/assign @andyzhangx` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-csi%2Fcsi-driver-smb).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-csi/csi-driver-smb/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["andyzhangx"]} -->
k8s-ci-robot: Hi @mayankshah1607. Thanks for your PR.

I'm waiting for a [kubernetes-csi](https://github.com/orgs/kubernetes-csi/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://git.k8s.io/community/community-membership.md#member) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://go.k8s.io/bot-commands?repo=kubernetes-csi%2Fcsi-driver-smb).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

coveralls: ## Pull Request Test Coverage Report for [Build 274007660](https://coveralls.io/builds/33741263)

* **5** of **7**   **(71.43%)**  changed or added relevant lines in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage decreased (**-0.2%**) to **87.658%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [pkg/smb/fake_mounter.go](https://coveralls.io/builds/33741263/source?filename=pkg%2Fsmb%2Ffake_mounter.go#L67) | 5 | 7 | 71.43%
<!-- | **Total:** | **5** | **7** | **71.43%** | -->


|  Totals | [![Coverage Status](https://coveralls.io/builds/33741263/badge)](https://coveralls.io/builds/33741263) |
| :-- | --: |
| Change from base [Build 268691318](https://coveralls.io/builds/33663317): |  -0.2% |
| Covered Lines: | 625 |
| Relevant Lines: | 713 |

---
##### 💛  - [Coveralls](https://coveralls.io)

mayankshah1607: @andyzhangx Left some comments above regarding mount behavior on Linux and Windows. If this approach looks good, we can merge this and I can enable the same approach for azuredisk and azurefile repos :)
mayankshah1607: Thanks @andyzhangx , I have made changes as per above comments
andyzhangx: could you also squash all commits? thanks.
k8s-ci-robot: @mayankshah1607: PR needs rebase.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

Patch:
diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml
index efa6f6b69..e56558808 100644
--- a/.github/workflows/windows.yml
+++ b/.github/workflows/windows.yml
@@ -25,18 +25,15 @@ jobs:
       - name: Run Windows Unit Tests
         run: |
           # start the CSI Proxy before running tests on windows
-          Start-Job -Name CSIProx -ScriptBlock {
+          Start-Job -Name CSIProxy -ScriptBlock {
             Invoke-WebRequest https://kubernetesartifacts.azureedge.net/csi-proxy/v0.2.1/binaries/csi-proxy.tar.gz -OutFile csi-proxy.tar.gz;
             tar -xvf csi-proxy.tar.gz
             .\bin\csi-proxy.exe --kubelet-csi-plugins-path $pwd --kubelet-pod-path $pwd
           };
 
-          # Wait for the CSI Proxy to come up
           Start-Sleep -Seconds 30;
 
-          # Print all the named pipes to make sure that CSI-Proxy's named-pipes are created.
-          Write-Output "Getting named pipes"
+          Write-Output "getting named pipes"
           [System.IO.Directory]::GetFiles("\\.\\pipe\\")
 
-          # Start the tests
           go test -v -race ./pkg/...
diff --git a/pkg/smb/fake_mounter.go b/pkg/smb/fake_mounter.go
index 50dcb3b94..1a63713cd 100644
--- a/pkg/smb/fake_mounter.go
+++ b/pkg/smb/fake_mounter.go
@@ -18,6 +18,8 @@ package smb
 
 import (
 	"fmt"
+	"github.com/kubernetes-csi/csi-driver-smb/pkg/mounter"
+	"runtime"
 	"strings"
 
 	"k8s.io/utils/mount"
@@ -59,3 +61,12 @@ func (f *fakeMounter) IsLikelyNotMountPoint(file string) (bool, error) {
 	}
 	return true, nil
 }
+
+func NewFakeMounter() (*mount.SafeFormatAndMount, error) {
+	if runtime.GOOS == "windows" {
+		return mounter.NewSafeMounter()
+	}
+	return &mount.SafeFormatAndMount{
+		Interface: &fakeMounter{},
+	}, nil
+}


Test patch:
diff --git a/pkg/smb/nodeserver_test.go b/pkg/smb/nodeserver_test.go
index ecaeab12f..43d8b8ea9 100644
--- a/pkg/smb/nodeserver_test.go
+++ b/pkg/smb/nodeserver_test.go
@@ -22,6 +22,8 @@ import (
 	"fmt"
 	"os"
 	"reflect"
+	"runtime"
+	"strings"
 	"syscall"
 	"testing"
 
@@ -34,6 +36,17 @@ import (
 	"k8s.io/utils/mount"
 )
 
+func matchFlakyWindowsError(mainError error, substr string) bool {
+	var errorMessage string
+	if mainError == nil {
+		errorMessage = ""
+	} else {
+		errorMessage = mainError.Error()
+	}
+
+	return strings.Contains(errorMessage, substr)
+}
+
 func TestNodeStageVolume(t *testing.T) {
 	stdVolCap := csi.VolumeCapability{
 		AccessType: &csi.VolumeCapability_Mount{
@@ -58,6 +71,14 @@ func TestNodeStageVolume(t *testing.T) {
 		desc        string
 		req         csi.NodeStageVolumeRequest
 		expectedErr testutil.TestError
+
+		// use this field only when Windows
+		// gives flaky error messages due
+		// to CSI proxy
+		// This field holds the base error message
+		// that is common amongst all other flaky
+		// error messages
+		flakyWindowsErrorMessage string
 	}{
 		{
 			desc: "[Error] Volume ID missing",
@@ -105,12 +126,14 @@ func TestNodeStageVolume(t *testing.T) {
 				VolumeCapability: &stdVolCap,
 				VolumeContext:    volContext,
 				Secrets:          secrets},
+			flakyWindowsErrorMessage: fmt.Sprintf("volume(vol_1##) mount \"test_source\" on %#v failed "+
+				"with smb mapping failed with error: rpc error: code = Unknown desc = NewSmbGlobalMapping failed.",
+				errorMountSensSource),
 			expectedErr: testutil.TestError{
 				DefaultError: status.Errorf(codes.Internal,
-					fmt.Sprintf("volume(vol_1##) mount \"test_source\" on \"%s\" failed with fake MountSensitive: target error",
+					fmt.Sprintf("volume(vol_1##) mount \"test_source\" on \"%s\" failed with fake "+
+						"MountSensitive: target error",
 						errorMountSensSource)),
-				// todo: Not a desired error. This will need a better fix
-				WindowsError: fmt.Errorf("prepare stage path failed for %s with error: could not cast to csi proxy class", errorMountSensSource),
 			},
 		},
 		{
@@ -119,10 +142,10 @@ func TestNodeStageVolume(t *testing.T) {
 				VolumeCapability: &stdVolCap,
 				VolumeContext:    volContext,
 				Secrets:          secrets},
-			expectedErr: testutil.TestError{
-				// todo: Not a desired error. This will need a better fix
-				WindowsError: fmt.Errorf("prepare stage path failed for %s with error: could not cast to csi proxy class", sourceTest),
-			},
+			flakyWindowsErrorMessage: fmt.Sprintf("volume(vol_1##) mount \"test_source\" on %#v failed with "+
+				"smb mapping failed with error: rpc error: code = Unknown desc = NewSmbGlobalMapping failed.",
+				sourceTest),
+			expectedErr: testutil.TestError{},
 		},
 	}
 
@@ -130,14 +153,23 @@ func TestNodeStageVolume(t *testing.T) {
 	d := NewFakeDriver()
 
 	for _, test := range tests {
-		fakeMounter := &fakeMounter{}
-		d.mounter = &mount.SafeFormatAndMount{
-			Interface: fakeMounter,
+		mounter, err := NewFakeMounter()
+		if err != nil {
+			t.Fatalf(fmt.Sprintf("failed to get fake mounter: %v", err))
 		}
-
-		_, err := d.NodeStageVolume(context.Background(), &test.req)
-		if !testutil.AssertError(&test.expectedErr, err) {
-			t.Errorf("test case: %s, \nUnexpected error: %v\n Expected: %v", test.desc, err, test.expectedErr.GetExpectedError())
+		d.mounter = mounter
+
+		_, err = d.NodeStageVolume(context.Background(), &test.req)
+
+		// separate assertion for flaky error messages
+		if test.flakyWindowsErrorMessage != "" && runtime.GOOS == "windows" {
+			if !matchFlakyWindowsError(err, test.flakyWindowsErrorMessage) {
+				t.Errorf("test case: %s, \nUnexpected error: %v\nExpected error: %v", test.desc, err, test.flakyWindowsErrorMessage)
+			}
+		} else {
+			if !testutil.AssertError(&test.expectedErr, err) {
+				t.Errorf("test case: %s, \nUnexpected error: %v\nExpected error: %v", test.desc, err, test.expectedErr.GetExpectedError())
+			}
 		}
 	}
 
@@ -146,6 +178,7 @@ func TestNodeStageVolume(t *testing.T) {
 	assert.NoError(t, err)
 	err = os.RemoveAll(errorMountSensSource)
 	assert.NoError(t, err)
+
 }
 
 func TestNodeGetInfo(t *testing.T) {
@@ -198,41 +231,49 @@ func TestNodeExpandVolume(t *testing.T) {
 }
 
 func TestNodePublishVolume(t *testing.T) {
-	skipIfTestingOnWindows(t)
 	volumeCap := csi.VolumeCapability_AccessMode{Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER}
-	errorMountSource := "./error_mount_source"
-	alreadyMountedTarget := "./false_is_likely_exist_target"
-	smbFile := "./smb.go"
-	sourceTest := "./source_test"
-	targetTest := "./target_test"
+	errorMountSource := testutil.GetWorkDirPath("error_mount_source", t)
+	alreadyMountedTarget := testutil.GetWorkDirPath("false_is_likely_exist_target", t)
+	smbFile := testutil.GetWorkDirPath("smb.go", t)
+	sourceTest := testutil.GetWorkDirPath("source_test", t)
+	targetTest := testutil.GetWorkDirPath("target_test", t)
 
 	tests := []struct {
-		desc        string
-		req         csi.NodePublishVolumeRequest
-		expectedErr error
+		desc          string
+		req           csi.NodePublishVolumeRequest
+		skipOnWindows bool
+		expectedErr   testutil.TestError
 	}{
 		{
-			desc:        "[Error] Volume capabilities missing",
-			req:         csi.NodePublishVolumeRequest{},
-			expectedErr: status.Error(codes.InvalidArgument, "Volume capability missing in request"),
+			desc: "[Error] Volume capabilities missing",
+			req:  csi.NodePublishVolumeRequest{},
+			expectedErr: testutil.TestError{
+				DefaultError: status.Error(codes.InvalidArgument, "Volume capability missing in request"),
+			},
 		},
 		{
-			desc:        "[Error] Volume ID missing",
-			req:         csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}},
-			expectedErr: status.Error(codes.InvalidArgument, "Volume ID missing in request"),
+			desc: "[Error] Volume ID missing",
+			req:  csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}},
+			expectedErr: testutil.TestError{
+				DefaultError: status.Error(codes.InvalidArgument, "Volume ID missing in request"),
+			},
 		},
 		{
 			desc: "[Error] Target path missing",
 			req: csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap},
 				VolumeId: "vol_1"},
-			expectedErr: status.Error(codes.InvalidArgument, "Target path not provided"),
+			expectedErr: testutil.TestError{
+				DefaultError: status.Error(codes.InvalidArgument, "Target path not provided"),
+			},
 		},
 		{
 			desc: "[Error] Stage target path missing",
 			req: csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap},
 				VolumeId:   "vol_1",
 				TargetPath: targetTest},
-			expectedErr: status.Error(codes.InvalidArgument, "Staging target not provided"),
+			expectedErr: testutil.TestError{
+				DefaultError: status.Error(codes.InvalidArgument, "Staging target not provided"),
+			},
 		},
 		{
 			desc: "[Error] Not a directory",
@@ -241,7 +282,11 @@ func TestNodePublishVolume(t *testing.T) {
 				TargetPath:        smbFile,
 				StagingTargetPath: sourceTest,
 				Readonly:          true},
-			expectedErr: status.Errorf(codes.Internal, "Could not mount target \"./smb.go\": mkdir ./smb.go: not a directory"),
+
+			expectedErr: testutil.TestError{
+				DefaultError: status.Errorf(codes.Internal, "Could not mount target \"%s\": mkdir %s: not a directory", smbFile, smbFile),
+				WindowsError: status.Errorf(codes.Internal, "Could not mount target %#v: mkdir %s: The system cannot find the path specified.", smbFile, smbFile),
+			},
 		},
 		{
 			desc: "[Error] Mount error mocked by Mount",
@@ -250,7 +295,12 @@ func TestNodePublishVolume(t *testing.T) {
 				TargetPath:        targetTest,
 				StagingTargetPath: errorMountSource,
 				Readonly:          true},
-			expectedErr: status.Errorf(codes.Internal, "Could not mount \"./error_mount_source\" at \"./target_test\": fake Mount: source error"),
+			// todo: This test does not return any error on windows
+			// Once the issue is figured out, we'll remove this field
+			skipOnWindows: true,
+			expectedErr: testutil.TestError{
+				DefaultError: status.Errorf(codes.Internal, fmt.Sprintf("Could not mount \"%s\" at \"%s\": fake Mount: source error", errorMountSource, targetTest)),
+			},
 		},
 		{
 			desc: "[Success] Valid request read only",
@@ -259,7 +309,7 @@ func TestNodePublishVolume(t *testing.T) {
 				TargetPath:        targetTest,
 				StagingTargetPath: sourceTest,
 				Readonly:          true},
-			expectedErr: nil,
+			expectedErr: testutil.TestError{},
 		},
 		{
 			desc: "[Success] Valid request already mounted",
@@ -268,7 +318,7 @@ func TestNodePublishVolume(t *testing.T) {
 				TargetPath:        alreadyMountedTarget,
 				StagingTargetPath: sourceTest,
 				Readonly:          true},
-			expectedErr: nil,
+			expectedErr: testutil.TestError{},
 		},
 		{
 			desc: "[Success] Valid request",
@@ -277,135 +327,161 @@ func TestNodePublishVolume(t *testing.T) {
 				TargetPath:        targetTest,
 				StagingTargetPath: sourceTest,
 				Readonly:          true},
-			expectedErr: nil,
+			expectedErr: testutil.TestError{},
 		},
 	}
 
 	// Setup
 	_ = makeDir(alreadyMountedTarget)
 	d := NewFakeDriver()
-	fakeMounter := &fakeMounter{}
-	d.mounter = &mount.SafeFormatAndMount{
-		Interface: fakeMounter,
+	mounter, err := NewFakeMounter()
+	if err != nil {
+		t.Fatalf(fmt.Sprintf("failed to get fake mounter: %v", err))
 	}
+	d.mounter = mounter
 
 	for _, test := range tests {
-		_, err := d.NodePublishVolume(context.Background(), &test.req)
-		if !reflect.DeepEqual(err, test.expectedErr) {
-			t.Errorf("test case: %s, Unexpected error: %v", test.desc, err)
+		if !(test.skipOnWindows && runtime.GOOS == "windows") {
+			_, err := d.NodePublishVolume(context.Background(), &test.req)
+			if !testutil.AssertError(&test.expectedErr, err) {
+				t.Errorf("test case: %s, \nUnexpected error: %v\nExpected error: %v", test.desc, err, test.expectedErr.GetExpectedError())
+			}
 		}
 	}
 
 	// Clean up
-	err := os.RemoveAll(targetTest)
+	err = os.RemoveAll(targetTest)
 	assert.NoError(t, err)
 	err = os.RemoveAll(alreadyMountedTarget)
 	assert.NoError(t, err)
 }
 
 func TestNodeUnpublishVolume(t *testing.T) {
-	skipIfTestingOnWindows(t)
-	errorTarget := "./error_is_likely_target"
-	targetFile := "./abc.go"
-	targetTest := "./target_test"
+	errorTarget := testutil.GetWorkDirPath("error_is_likely_target", t)
+	targetFile := testutil.GetWorkDirPath("abc.go", t)
+	targetTest := testutil.GetWorkDirPath("target_test", t)
 
 	tests := []struct {
-		desc        string
-		req         csi.NodeUnpublishVolumeRequest
-		expectedErr error
+		desc          string
+		req           csi.NodeUnpublishVolumeRequest
+		expectedErr   testutil.TestError
+		skipOnWindows bool
 	}{
 		{
-			desc:        "[Error] Volume ID missing",
-			req:         csi.NodeUnpublishVolumeRequest{TargetPath: targetTest},
-			expectedErr: status.Error(codes.InvalidArgument, "Volume ID missing in request"),
+			desc: "[Error] Volume ID missing",
+			req:  csi.NodeUnpublishVolumeRequest{TargetPath: targetTest},
+			expectedErr: testutil.TestError{
+				DefaultError: status.Error(codes.InvalidArgument, "Volume ID missing in request"),
+			},
 		},
 		{
-			desc:        "[Error] Target missing",
-			req:         csi.NodeUnpublishVolumeRequest{VolumeId: "vol_1"},
-			expectedErr: status.Error(codes.InvalidArgument, "Target path missing in request"),
+			desc: "[Error] Target missing",
+			req:  csi.NodeUnpublishVolumeRequest{VolumeId: "vol_1"},
+			expectedErr: testutil.TestError{
+				DefaultError: status.Error(codes.InvalidArgument, "Target path missing in request"),
+			},
 		},
 		{
-			desc:        "[Error] Unmount error mocked by IsLikelyNotMountPoint",
-			req:         csi.NodeUnpublishVolumeRequest{TargetPath: errorTarget, VolumeId: "vol_1"},
-			expectedErr: status.Error(codes.Internal, "failed to unmount target \"./error_is_likely_target\": fake IsLikelyNotMountPoint: fake error"),
+			desc: "[Error] Unmount error mocked by IsLikelyNotMountPoint",
+			req:  csi.NodeUnpublishVolumeRequest{TargetPath: errorTarget, VolumeId: "vol_1"},
+			// todo: This test does not return any error on windows
+			// Once the issue is figured out, we'll remove this field
+			skipOnWindows: true,
+			expectedErr: testutil.TestError{
+				DefaultError: status.Error(codes.Internal, fmt.Sprintf("failed to unmount target \"%s\": fake IsLikelyNotMountPoint: fake error", errorTarget)),
+			},
 		},
 		{
 			desc:        "[Success] Valid request",
 			req:         csi.NodeUnpublishVolumeRequest{TargetPath: targetFile, VolumeId: "vol_1"},
-			expectedErr: nil,
+			expectedErr: testutil.TestError{},
 		},
 	}
 
 	// Setup
 	_ = makeDir(errorTarget)
 	d := NewFakeDriver()
-	fakeMounter := &fakeMounter{}
-	d.mounter = &mount.SafeFormatAndMount{
-		Interface: fakeMounter,
+	mounter, err := NewFakeMounter()
+	if err != nil {
+		t.Fatalf(fmt.Sprintf("failed to get fake mounter: %v", err))
 	}
+	d.mounter = mounter
 
 	for _, test := range tests {
-		_, err := d.NodeUnpublishVolume(context.Background(), &test.req)
-		if !reflect.DeepEqual(err, test.expectedErr) {
-			t.Errorf("test case: %s, Unexpected error: %v", test.desc, err)
+		if !(test.skipOnWindows && runtime.GOOS == "windows") {
+			_, err := d.NodeUnpublishVolume(context.Background(), &test.req)
+			if !testutil.AssertError(&test.expectedErr, err) {
+				t.Errorf("test case: %s, \nUnexpected error: %v\nExpected error: %v", test.desc, err, test.expectedErr.GetExpectedError())
+			}
 		}
 	}
 
 	// Clean up
-	err := os.RemoveAll(errorTarget)
+	err = os.RemoveAll(errorTarget)
 	assert.NoError(t, err)
 }
 
 func TestNodeUnstageVolume(t *testing.T) {
-	skipIfTestingOnWindows(t)
-	errorTarget := "./error_is_likely_target"
-	targetFile := "./abc.go"
-	targetTest := "./target_test"
+	errorTarget := testutil.GetWorkDirPath("error_is_likely_target", t)
+	targetFile := testutil.GetWorkDirPath("abc.go", t)
+	targetTest := testutil.GetWorkDirPath("target_test", t)
 
 	tests := []struct {
-		desc        string
-		req         csi.NodeUnstageVolumeRequest
-		expectedErr error
+		desc          string
+		req           csi.NodeUnstageVolumeRequest
+		skipOnWindows bool
+		expectedErr   testutil.TestError
 	}{
 		{
-			desc:        "[Error] Volume ID missing",
-			req:         csi.NodeUnstageVolumeRequest{StagingTargetPath: targetTest},
-			expectedErr: status.Error(codes.InvalidArgument, "Volume ID missing in request"),
+			desc: "[Error] Volume ID missing",
+			req:  csi.NodeUnstageVolumeRequest{StagingTargetPath: targetTest},
+			expectedErr: testutil.TestError{
+				DefaultError: status.Error(codes.InvalidArgument, "Volume ID missing in request"),
+			},
 		},
 		{
-			desc:        "[Error] Target missing",
-			req:         csi.NodeUnstageVolumeRequest{VolumeId: "vol_1"},
-			expectedErr: status.Error(codes.InvalidArgument, "Staging target not provided"),
+			desc: "[Error] Target missing",
+			req:  csi.NodeUnstageVolumeRequest{VolumeId: "vol_1"},
+			expectedErr: testutil.TestError{
+				DefaultError: status.Error(codes.InvalidArgument, "Staging target not provided"),
+			},
 		},
 		{
-			desc:        "[Error] CleanupMountPoint error mocked by IsLikelyNotMountPoint",
-			req:         csi.NodeUnstageVolumeRequest{StagingTargetPath: errorTarget, VolumeId: "vol_1"},
-			expectedErr: status.Error(codes.Internal, "failed to unmount staging target \"./error_is_likely_target\": fake IsLikelyNotMountPoint: fake error"),
+			desc:          "[Error] CleanupMountPoint error mocked by IsLikelyNotMountPoint",
+			req:           csi.NodeUnstageVolumeRequest{StagingTargetPath: errorTarget, VolumeId: "vol_1"},
+			skipOnWindows: true,
+			expectedErr: testutil.TestError{
+				DefaultError: status.Error(codes.Internal, fmt.Sprintf("failed to unmount staging target %#v: fake IsLikelyNotMountPoint: fake error", errorTarget)),
+			},
 		},
 		{
 			desc:        "[Success] Valid request",
 			req:         csi.NodeUnstageVolumeRequest{StagingTargetPath: targetFile, VolumeId: "vol_1"},
-			expectedErr: nil,
+			expectedErr: testutil.TestError{},
 		},
 	}
 
 	// Setup
 	_ = makeDir(errorTarget)
 	d := NewFakeDriver()
-	fakeMounter := &fakeMounter{}
-	d.mounter = &mount.SafeFormatAndMount{
-		Interface: fakeMounter,
+	mounter, err := NewFakeMounter()
+	if err != nil {
+		t.Fatalf(fmt.Sprintf("failed to get fake mounter: %v", err))
 	}
+	d.mounter = mounter
 
 	for _, test := range tests {
-		_, err := d.NodeUnstageVolume(context.Background(), &test.req)
-		if !reflect.DeepEqual(err, test.expectedErr) {
-			t.Errorf("test case: %s, Unexpected error: %v", test.desc, err)
+		if !(test.skipOnWindows && runtime.GOOS == "windows") {
+			_, err := d.NodeUnstageVolume(context.Background(), &test.req)
+			if !testutil.AssertError(&test.expectedErr, err) {
+				t.Errorf("test case: %s, \nUnexpected error: %v\nExpected error: %v", test.desc, err, test.expectedErr.GetExpectedError())
+			}
 		}
+
 	}
 
 	// Clean up
-	err := os.RemoveAll(errorTarget)
+	err = os.RemoveAll(errorTarget)
 	assert.NoError(t, err)
 }
 
diff --git a/pkg/smb/smb_test.go b/pkg/smb/smb_test.go
index b3367bdde..6f42dd52f 100644
--- a/pkg/smb/smb_test.go
+++ b/pkg/smb/smb_test.go
@@ -20,7 +20,6 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"runtime"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -40,15 +39,7 @@ func TestNewFakeDriver(t *testing.T) {
 	assert.NotNil(t, d)
 }
 
-// skip test if running on windows
-func skipIfTestingOnWindows(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("Skipping test on Windows")
-	}
-}
-
 func TestIsCorruptedDir(t *testing.T) {
-	skipIfTestingOnWindows(t)
 	existingMountPath, err := ioutil.TempDir(os.TempDir(), "csi-mount-test")
 	if err != nil {
 		t.Fatalf("failed to create tmp dir: %v", err)
diff --git a/test/utils/testutil/testutil.go b/test/utils/testutil/testutil.go
index 6331ea5f9..82620ac86 100644
--- a/test/utils/testutil/testutil.go
+++ b/test/utils/testutil/testutil.go
@@ -32,10 +32,10 @@ type TestError struct {
 
 // GetExpectedError returns the expected error depending on OS
 func (t TestError) GetExpectedError() error {
-	if isWindows() {
-		return t.WindowsError
+	if t.WindowsError == nil || !isWindows() {
+		return t.DefaultError
 	}
-	return t.DefaultError
+	return t.WindowsError
 }
 
 // AssertError matches the actual and expected errors