test: add Ginkgo/Gomega coverage for pkg/ddc/base #5803
Conversation
|
Hi @hxrshxz. Thanks for your PR. I'm waiting for a fluid-cloudnative 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. DetailsInstructions 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. |
There was a problem hiding this comment.
Pull request overview
This PR expands unit test coverage for the pkg/ddc/base dataset/setup/volume workflow by migrating tests to Ginkgo/Gomega, and it tightens TemplateEngine.Setup behavior so errors during runtime-status updates or dataset binding correctly report ready=false to the controller reconciliation loop.
Changes:
- Fix
TemplateEngine.Setupto returnready=falsewhenCheckAndUpdateRuntimeStatusorBindToDatasetfails. - Migrate
dataset_test.gofromtesting/testify-style to Ginkgo/Gomega table-driven specs. - Add new Ginkgo/Gomega specs covering setup error paths and volume operation delegation/error propagation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/ddc/base/setup.go | Adjusts error returns in Setup so failures can’t be misinterpreted as “ready”. |
| pkg/ddc/base/dataset_test.go | Migrates dataset helper tests to Ginkgo/Gomega and strengthens assertions. |
| pkg/ddc/base/dataset_setup_volume_test.go | Adds new specs for setup error handling and Create/DeleteVolume delegation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the dataset test suite to use the Ginkgo and Gomega frameworks and adds comprehensive tests for the TemplateEngine. It also fixes a bug in the Setup function where the return value was incorrectly handled during error states. Review feedback suggests cleaning up an unused constant and an empty test block, and adopting best practices for context keys by using custom types instead of raw strings.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5803 +/- ##
==========================================
+ Coverage 59.10% 59.22% +0.11%
==========================================
Files 480 480
Lines 32512 32512
==========================================
+ Hits 19215 19254 +39
+ Misses 11747 11715 -32
+ Partials 1550 1543 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new test suite for TemplateEngine setup and volume operations and refactors existing dataset tests to use the Ginkgo and Gomega frameworks. It also corrects the error return logic in the Setup function to ensure a false status is returned upon failure. Review feedback recommends removing an unused constant and an empty test block to improve code cleanliness.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hxrshxz
changed the title
hxrshxz
changed the title
cheyang
left a comment
cheyang
left a comment
There was a problem hiding this comment.
感谢贡献！stubImplement 模式设计灵活，能覆盖多种 Setup 路径。建议：
- stubImplement 中未覆盖的接口方法默认返回 nil/false，请确认这些默认返回值不会让测试产生误导性通过（如 CheckMasterReady 默认返回 false 可能导致提前终止）。
- 建议补充 CreateVolume/DeleteVolume 错误路径的测试。
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
|
English translation of the previous review comments:
|
Signed-off-by: Harsh <harshmastic@gmail.com>
Signed-off-by: Harsh <harshmastic@gmail.com>
Signed-off-by: Harsh <harshmastic@gmail.com>
Signed-off-by: Harsh <harshmastic@gmail.com>
hxrshxz
force-pushed
the
test/ddc-base-file-scope-5676
branch
from
b1ce4af to
9643034
Compare
|
|
Confirmed the I also added coverage for |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cheyang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/approve |
Ⅰ. Describe what this PR does
Add Ginkgo/Gomega test coverage for the file-scoped
pkg/ddc/basedataset, engine, setup, and volume paths.Ⅱ. Does this pull request fix one issue?
fixes #5676
Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
pkg/ddc/base/dataset_test.goto Ginkgo/Gomegapkg/ddc/base/dataset_setup_volume_test.gopkg/ddc/base/dataset_setup_volume_test.goⅣ. Describe how to verify it
go test ./pkg/ddc/base/...Ⅴ. Special notes for reviews
N/A