Testing: Implement missing unit tests for pkg/backup/snapshots.go - #10315
Open
opbot-xd wants to merge 1 commit into
Open
Testing: Implement missing unit tests for pkg/backup/snapshots.go#10315opbot-xd wants to merge 1 commit into
opbot-xd wants to merge 1 commit into
Conversation
opbot-xd
force-pushed
the
add-snapshots-tests
branch
from
August 17, 2026 18:09
a0fbd17 to
2c29f00
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Signed-off-by: opbot_xd <awasthikrishna23052005@gmail.com>
opbot-xd
force-pushed
the
add-snapshots-tests
branch
from
August 17, 2026 19:14
2c29f00 to
62f5e38
Compare
👷 Deploy request for velero pending review.Visit the deploys page to approve it
|
kaovilai
approved these changes
Aug 18, 2026
Joeavaikath
reviewed
Aug 18, 2026
| SnapshotMoveData: boolptr.True(), | ||
| }, | ||
| }, | ||
| csiFeatureEnabled: true, |
Collaborator
There was a problem hiding this comment.
I think it would be prudent to have existingObjects for cases 1 and 2 just to showcase that no ops happen on them
As is, turning the flags on and off wouldn't prove anything
Joeavaikath
requested changes
Aug 18, 2026
| SnapshotMoveData: boolptr.False(), | ||
| }, | ||
| }, | ||
| csiFeatureEnabled: false, |
Collaborator
There was a problem hiding this comment.
Same as previous comment, existingObjects here would be wise
|
|
||
| for _, tc := range tests { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| if tc.csiFeatureEnabled { |
Collaborator
There was a problem hiding this comment.
Adding a defer to reset the feature flag set would be great, to prevent leakage into other tests
I believe that's good convention
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you for contributing to Velero!
Please add a summary of your change
The
pkg/backup/snapshots.gofile contains the core logic for handling CSI snapshots and VolumeSnapshotTrackers (e.g.,GetBackupCSIResources), but it currently lacks unit test coverage.This PR introduces a brand new test file
pkg/backup/snapshots_test.goand implements a comprehensive table-driven unit test suite for theGetBackupCSIResourcesfunction. It covers all branches of the function, ensuring that snapshot filtering, tracker sync logic, and object deduplication remain robust as Velero evolves.Does your change fix a particular issue?
Fixes #10314
Please indicate you've done the following:
make new-changelog) or comment/kind changelog-not-requiredon this PR.site/content/docs/main.