Skip to content

Cleaning up controller tests#463

Merged
lukeatdell merged 6 commits into
mainfrom
usr/lukeatdell/controller-test-pkg-clean
Apr 14, 2025
Merged

Cleaning up controller tests#463
lukeatdell merged 6 commits into
mainfrom
usr/lukeatdell/controller-test-pkg-clean

Conversation

@lukeatdell

@lukeatdell lukeatdell commented Apr 11, 2025

Copy link
Copy Markdown
Contributor

Description

Moving controller tests to the from the controller_test pkg to the controller pkg to facilitate test having access to the struct members and functions they're testing without requiring everything from controller be exported.

In doing so, two unused, auto-generated mocks files were removed because they are imported by the controller pkg tests and, in turn, the two files would import the csi-powerstore controller pkg, creating an import loop.

I confirmed they were unused, the tests still run and pass, and the mocks package is not imported by any other repos: source

  • removed imports of the controller package from the _test.go files contained within the controller dir.
  • refactored elements to remove references to controller from the tests, as they are no longer needed since they are now part of the same package.

Additionally, making small improvements to organization in the Makefile, and fixing one unit tests in node_test.go that fails when run locally.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
https://github.com/dell/csm/issues/1742

Checklist:

  • I have verified that new and existing unit tests pass locally with my changes
  • I have not allowed coverage numbers to degenerate
  • I have maintained at least 90% code coverage
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • Backward compatibility is not broken

How Has This Been Tested?

Executed unit tests locally and via GH actions.

- move contoller pkg tests from "controller_test" pkg to "controller" pkg.
- requires deletion of mocks that were not in use because it caused an import cycle. mocks were generated and can be generated again if needed

@santhoshatdell santhoshatdell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

GitHub issue is missing.

@lukeatdell

Copy link
Copy Markdown
Contributor Author

GitHub issue is missing.

added!

@github-actions

Copy link
Copy Markdown
Contributor

Merging this branch will not change overall coverage

Impacted Packages Coverage Δ 🤖
github.com/dell/csi-powerstore/mocks 0.00% (ø)
github.com/dell/csi-powerstore/pkg/controller 0.00% (ø)
github.com/dell/csi-powerstore/pkg/service 0.00% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/dell/csi-powerstore/mocks/GeneralSnapshot.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/mocks/VolumeSnapshotter.go 0.00% (ø) 0 0 0

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/dell/csi-powerstore/pkg/controller/controller_test.go
  • github.com/dell/csi-powerstore/pkg/controller/creator_test.go
  • github.com/dell/csi-powerstore/pkg/controller/csi_extension_server_test.go
  • github.com/dell/csi-powerstore/pkg/controller/publisher_test.go
  • github.com/dell/csi-powerstore/pkg/controller/replication_test.go
  • github.com/dell/csi-powerstore/pkg/service/node_test.go

@lukeatdell lukeatdell merged commit bc71e1a into main Apr 14, 2025
@lukeatdell lukeatdell deleted the usr/lukeatdell/controller-test-pkg-clean branch April 14, 2025 16:44
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.

3 participants