test: add alluxio engine and transform coverage#5804
test: add alluxio engine and transform coverage#5804hxrshxz wants to merge 1 commit intofluid-cloudnative:masterfrom
Conversation
Signed-off-by: Harsh <harshmastic@gmail.com>
|
[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 |
|
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. |
|
![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 Alluxio API gateway tests to use the Ginkgo and Gomega frameworks and introduces a new test file, engine_transform_file_scope_test.go, to cover engine building, prechecking, and transformation logic. I have no feedback to provide.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5804 +/- ##
==========================================
+ Coverage 58.51% 58.80% +0.29%
==========================================
Files 473 478 +5
Lines 32283 32485 +202
==========================================
+ Hits 18890 19103 +213
+ Misses 11849 11825 -24
- Partials 1544 1557 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds Ginkgo/Gomega-based unit tests to increase coverage for pkg/ddc/alluxio, focusing on engine build/precheck behavior and file-scoped transform behavior, and migrates the API gateway tests from testify to Ginkgo/Gomega (part of the broader testing framework modernization in #5676).
Changes:
- Add same-package Ginkgo/Gomega coverage for
Build,Precheck,transformPlacementMode, and top-leveltransform(...)behavior in the Alluxio engine. - Migrate
api_gateway_test.gofromtesting+testify/assertto Ginkgo/Gomega with table-driven specs. - Add focused assertions for placement defaults and key transform outputs (ports, properties, pod metadata, tiered store paths).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/ddc/alluxio/engine_transform_file_scope_test.go | New Ginkgo/Gomega specs covering Build/Precheck and file-scoped transform behaviors and defaults. |
| pkg/ddc/alluxio/api_gateway_test.go | Migrates API gateway unit tests to Ginkgo/Gomega table-driven specs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Expect(value.Fuse.Args).To(HaveLen(2)) | ||
| Expect(value.Fuse.Args[1]).To(ContainSubstring("max_readahead=0")) | ||
| Expect(value.Fuse.Args[1]).To(ContainSubstring("allow_other")) |
There was a problem hiding this comment.
The assertions around value.Fuse.Args are brittle because the arg shape depends on the fuse image tag (via parseFuseImage + checkIfNewFuseArgVersion), not runtime.Spec.AlluxioVersion.ImageTag. If the default DefaultAlluxioFuseImage (or ALLUXIO_FUSE_IMAGE_ENV) ever moves to >= 2.8.0, value.Fuse.Args will include mount args and won’t have length 2, and Args[1] may no longer be the only place to check options. Consider explicitly setting runtime.Spec.Fuse.ImageTag to a known <2.8.0 value in this test (or relaxing the assertions to be version-agnostic by searching across the args slice).
Ⅰ. Describe what this PR does
Add test-only Ginkgo/Gomega coverage for the file-scoped
pkg/ddc/alluxioengine and transform paths, while migrating the scoped API gateway tests to Ginkgo/Gomega.Ⅱ. 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/alluxio/api_gateway_test.goto Ginkgo/GomegaBuildandPrecheckinpkg/ddc/alluxio/engine_transform_file_scope_test.gotransform(...)behavior and placement defaults inpkg/ddc/alluxio/engine_transform_file_scope_test.goⅣ. Describe how to verify it
go test ./pkg/ddc/alluxio -run '^(TestAlluxio|TestBuild|TestTransformPodMetadata|TestGetMediumTypeFromVolumeSource|TestAlluxioEngine_allocateSinglePort|TestAlluxioEngine_allocatePorts|TestTransformShortCircuit|TestTransformMaster|TestTransformWorkers|TestGenerateStaticPorts|TestGenerateNonNativeMountsInfo|TestTransformMasterProperties|TestTransformWorkerProperties|TestTransformMasterMountConfigMap|TestTransformFuseProperties|TestTransformFuse)$' -ginkgo.label-filter='pkg.ddc.alluxio.engine_transform_file_scope_test.go || pkg.ddc.alluxio.api_gateway_test.go'Ⅴ. Special notes for reviews
N/A