-
Notifications
You must be signed in to change notification settings - Fork 194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: unit test failure on Windows #522
fix: unit test failure on Windows #522
Conversation
Welcome @mayankshah1607! |
Hi @mayankshah1607. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@andyzhangx PTAL! :) |
pkg/azuredisk/azuredisk_test.go
Outdated
@@ -544,6 +544,7 @@ func TestIsAvailabilityZone(t *testing.T) { | |||
} | |||
|
|||
func TestIsCorruptedDir(t *testing.T) { | |||
skipTestsIfWindows(t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you know how to configure this test in admin mode, actually this unit test TestIsCorruptedDir
could pass in admin mode on Windows:
>go test -run TestIsCorruptedDir
--- FAIL: TestIsCorruptedDir (0.00s)
azuredisk_test.go:555: failed to create curruptedPath: symlink C:\Users\xiazhang\AppData\Local\Temp\csi-mount-test769943179 C:\Users\xiazhang\AppData\Local\Temp\csi-mount-test769943179\curruptedPath: A required privilege is not held by the client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be some setting in windows.yaml config:
https://github.com/kubernetes-sigs/azuredisk-csi-driver/blob/master/.github/workflows/window#L28
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a possibility to use following runas command to solve this issue, while it still prompt password, need to figure it out later:
https://www.tenforums.com/general-support/111929-how-use-runas-without-password-prompt.html
C:\Users\xiazhang\go\src\sigs.k8s.io\azuredisk-csi-driver\pkg\azuredisk>runas /user:Administrator "go test -run TestIsCorruptedDir"
Enter the password for Administrator:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andyzhangx Just went through this issue thread - golang/dep#773
Seems like its an issue from the os.Symlink
client library that's preventing the machine from creating Symlinks.
I tried running the test as administrator from Command promt and it seemed to PASS.
WDYT about skipping this test for now until we figure out a way to run as admin from the workflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
waiting for this PR: #521 |
/ok-to-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls also add a new line go test -v -race ./pkg/azuredisk/...
under
or set as go test -v -race ./pkg/...
go test -v -race ./pkg/mounter/... |
pkg/azuredisk/azure_test.go
Outdated
func TestGetCloudProvider(t *testing.T) { | ||
skipTestsIfWindows(t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename as skipIfTestingOnWindows
Signed-off-by: Mayank Shah <[email protected]>
Signed-off-by: Mayank Shah <[email protected]>
Signed-off-by: Mayank Shah <[email protected]>
11f2293
to
a922d6c
Compare
Signed-off-by: Mayank Shah <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andyzhangx, mayankshah1607 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…e-e2e test: fix in-tree e2e test failure
Signed-off-by: Mayank Shah [email protected]
What type of PR is this?
/kind test
/kind cleanup
What this PR does / why we need it:
Fix unit test failures on Windows
Which issue(s) this PR fixes:
Fixes #520
Requirements:
Special notes for your reviewer:
Release note: