test(fluidapp): migrate controller tests to Ginkgo/Gomega#5684
test(fluidapp): migrate controller tests to Ginkgo/Gomega#5684hxrshxz wants to merge 1 commit intofluid-cloudnative:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the testing infrastructure for the Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
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.
Code Review
This pull request primarily focuses on migrating existing unit tests from the standard Go testing package to the Ginkgo and Gomega testing framework across several controller packages. Specifically, tests in dataflowaffinity_controller_test.go and implement_test.go were refactored into Describe/Context/It blocks, replacing func Test... with Ginkgo's declarative style and Expect assertions. New suite_test.go files were introduced for both dataflowaffinity and fluidapp packages to set up the Ginkgo test suites. Additionally, a new fluidapp_controller_test.go file was added to include Ginkgo tests for the FluidAppReconciler, covering its core functionalities like ControllerName, ManagedResource, NewFluidAppReconciler, Reconcile scenarios, and internalReconcile logic, including handling fuse sidecars. The changes aim to improve test readability, structure, and maintainability by adopting the Ginkgo/Gomega framework.
There was a problem hiding this comment.
Pull request overview
Migrates pkg/controllers/v1alpha1/fluidapp/ controller unit tests from testing.T to the repository’s Ginkgo v2/Gomega BDD style, introducing Ginkgo suite bootstraps and expanding scenario coverage for reconciler and helper behaviors.
Changes:
- Added Ginkgo/Gomega suite bootstraps for
fluidappandfluidapp/dataflowaffinity. - Rewrote existing unit tests in
fluidappanddataflowaffinitypackages intoDescribe/Context/Itstructure with Gomega assertions. - Added additional unit tests for
FluidAppReconcilerandDataOpJobReconcilercontroller methods and error paths.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/controllers/v1alpha1/fluidapp/suite_test.go | Adds Ginkgo suite bootstrap for the fluidapp package. |
| pkg/controllers/v1alpha1/fluidapp/implement_test.go | Migrates umountFuseSidecars/umountFuseSidecar tests to Ginkgo/Gomega and adds error-path cases. |
| pkg/controllers/v1alpha1/fluidapp/fluidapp_controller_test.go | Adds new unit tests for FluidAppReconciler lifecycle and reconcile paths. |
| pkg/controllers/v1alpha1/fluidapp/dataflowaffinity/suite_test.go | Adds Ginkgo suite bootstrap for the dataflowaffinity subpackage. |
| pkg/controllers/v1alpha1/fluidapp/dataflowaffinity/dataflowaffinity_controller_test.go | Migrates DataOpJobReconciler tests to Ginkgo/Gomega and expands coverage of affinity injection helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5684 +/- ##
=======================================
Coverage 61.21% 61.21%
=======================================
Files 444 444
Lines 30540 30540
=======================================
Hits 18694 18694
Misses 10306 10306
Partials 1540 1540 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2b21bb3 to
8130771
Compare
…st suite Signed-off-by: Harsh <harshmastic@gmail.com>
8130771 to
fda5492
Compare
|



Ⅰ. Describe what this PR does
Migrate unit tests in
pkg/controllers/v1alpha1/fluidapp/fromtesting.Tto Ginkgo/Gomega, adding suite bootstraps, migrating existing table-driven tests, and expanding coverage to 85.2% for the main package.Ⅱ. Does this pull request fix one issue?
#5676
Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
suite_test.gobootstraps forfluidappanddataflowaffinitysubpackagesumountFuseSidecarsandumountFuseSidecartests to GinkgoDescribe/Context/Itwith Gomega matchers; added error-path cases for "not mounted" and "exit code 137"injectPodNodeLabelsToJobandfillCustomizedNodeAffinitytests to Ginkgo/GomegaFluidAppReconcilerandDataOpJobReconcilercontroller lifecycle methods (ControllerName,ManagedResource,NewFluidAppReconciler,Reconcile,internalReconcile)Ⅳ. Describe how to verify it
go test ./pkg/controllers/v1alpha1/fluidapp/... -v -count=1Ⅴ. Special notes for reviews
N/A