Skip to content
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 errors in existing Unit Tests on Local environment #27

Merged
merged 2 commits into from
Nov 8, 2024

Conversation

hlts2
Copy link
Member

@hlts2 hlts2 commented Nov 7, 2024

WHAT

I fixed the errors in the existing tests.
Additionally, the driver tests were not running on CI, so I modified the actions workflow to enable them. This ensures that the existing driver test code will now run on CI.

WHY

When I run all existing tests in the local environment, the following error occurred. Since I not sure whether the previous driver code was working properly, I have investigated. Upon investigation, I concluded that it was necessary to modify the only test code.

❯ go clean -testcache && go test ./...
ok  	github.com/civo/civo-csi	70.094s
ok  	github.com/civo/civo-csi/e2e	0.006s
{"level":"info","api_url":"https://civo-api.example.com","region":"TEST1","namespace":"default","cluster_id":"12345678","socketFilename":"unix:///var/lib/kubelet/plugins/civo-csi/csi.sock","user_agent":"civo-csi","time":"2024-11-07T18:06:02+09:00","message":"Created a new driver"}
--- FAIL: TestNodePublishVolume (0.00s)
    --- FAIL: TestNodePublishVolume/Bind-mount_the_volume_from_the_general_mount_point_in_to_the_container (0.00s)
        node_server_test.go:129:
            	Error Trace:	/xxx/go/src/github.com/civo/civo-csi/pkg/driver/node_server_test.go:129
            	Error:      	Expected nil, but got: &fs.PathError{Op:"mkdir", Path:"/var/lib/kubelet", Err:0xd}
            	Test:       	TestNodePublishVolume/Bind-mount_the_volume_from_the_general_mount_point_in_to_the_container
        node_server_test.go:132:
            	Error Trace:	/xxx/go/src/github.com/civo/civo-csi/pkg/driver/node_server_test.go:132
            	Error:      	Should be true
            	Test:       	TestNodePublishVolume/Bind-mount_the_volume_from_the_general_mount_point_in_to_the_container
FAIL
FAIL	github.com/civo/civo-csi/pkg/driver	10.012s
FAIL

↑ Error due to permissions when creating a folder in the local environment.

FYI

⚠️ This change focuses on fixing existing unit test error, not adding new ones.
While additional test cases will be considered in the future, I modified the current tests were not running successfully in both local and CI environments.

  • The following is the result of all unit test on Local
❯ go clean -testcache && go test ./...
ok  	github.com/civo/civo-csi	70.093s
ok  	github.com/civo/civo-csi/e2e	0.006s
ok  	github.com/civo/civo-csi/pkg/driver	10.012s

_, err := d.NodePublishVolume(context.Background(), &csi.NodePublishVolumeRequest{
VolumeId: "volume-1",
StagingTargetPath: "/mnt/my-target",
TargetPath: "/var/lib/kubelet/some-path",
Copy link
Member Author

@hlts2 hlts2 Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

In a Kubernetes environment, the /var/lib/kubelet/ directory is used, but when trying to use the same path in unit tests on Local and CI, a permission error occurs.
Since this function(d.NodePublishVolume) involves creating and mounting volumes, I used a temporary directory for test.

@hlts2 hlts2 self-assigned this Nov 7, 2024
@hlts2 hlts2 requested review from rytswd and Praveen005 November 7, 2024 09:57
@hlts2 hlts2 marked this pull request as ready for review November 7, 2024 10:04
@hlts2 hlts2 changed the title Fix errors in existing Unit Tests for Local environment and CI Fix errors in existing Unit Tests for Local environment Nov 7, 2024
@hlts2 hlts2 changed the title Fix errors in existing Unit Tests for Local environment Fix errors in existing Unit Tests on Local environment Nov 7, 2024
Copy link
Member

@rytswd rytswd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@hlts2
Copy link
Member Author

hlts2 commented Nov 8, 2024

@rytswd
Thank you for reviewing it! 🙏
I will merge this PR 🚀

@hlts2 hlts2 merged commit b7261a3 into master Nov 8, 2024
4 checks passed
@hlts2 hlts2 deleted the fix/unit-test-error branch November 8, 2024 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants