ci: add arm arch runner in github actions#5187
ci: add arm arch runner in github actions#5187cheyang merged 6 commits intofluid-cloudnative:masterfrom
Conversation
|
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
…y lingma Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
TrafalgarZZZ
force-pushed
the
enhancement/add_arm_runner_in_gha
branch
from
f6362c8 to
b716f34
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5187 +/- ##
==========================================
- Coverage 56.96% 56.85% -0.12%
==========================================
Files 442 442
Lines 30529 30461 -68
==========================================
- Hits 17391 17318 -73
- Misses 11522 11527 +5
Partials 1616 1616 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: TzZtzt <trafalgarz@outlook.com>
Signed-off-by: TzZtzt <trafalgarz@outlook.com>
| return "", "", errors.New("fail to run the command") | ||
| } | ||
|
|
||
| // 修复Windows环境下gomonkey.ApplyMethod的问题 |
There was a problem hiding this comment.
@xliuqq Would you like to have a double check here? I'm not sure if it may cause problems on Windows.
There was a problem hiding this comment.
checked, no problems on windows.
It's generated by AI, you can delete it directly.
There was a problem hiding this comment.
Pull Request Overview
This PR adds ARM architecture support to Fluid's GitHub Actions CI pipeline and updates test code for ARM compatibility.
- Separates build and unit test workflows to enable ARM build testing while keeping unit tests on x86_64 (due to race detector limitations on ARM)
- Updates gomonkey test mocking patterns to be ARM-compatible
- Enhances Makefile for better cross-platform builds
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 |
|---|---|
.github/workflows/project-check.yml |
Splits build and unittest into separate jobs; adds ARM runner (ubuntu-24.04-arm) to build job matrix |
Makefile |
Makes GOOS dynamic, adds arm64 UNAME detection, and aligns LOCAL_FLAGS with GC_FLAGS for consistency |
pkg/ddc/alluxio/operations/base.go |
Changes FindUnmountedAlluxioPaths to use pointer receiver for ARM-compatible mocking |
pkg/ddc/alluxio/operations/base_test.go |
Updates TestAlluxioFileUtils_ExecMountScripts to use ARM-compatible gomonkey pattern with instance-based mocking |
pkg/ddc/alluxio/ufs_test.go |
Updates TestFindUnmountedUFS to use ARM-compatible gomonkey pattern without reflect.TypeOf |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| err = a.ExecMountScripts() | ||
| if err != nil { | ||
| t.Errorf("check failure, want nil, got err: %v", err) | ||
| } | ||
| defer patches.Reset() |
There was a problem hiding this comment.
[nitpick] The defer patches.Reset() statement is placed at the end of the function, which is unconventional. It would be more idiomatic to place the defer immediately after the assignment on line 661 where the patch is created, making the cleanup logic clearer and following Go best practices.
| err = a.ExecMountScripts() | |
| if err != nil { | |
| t.Errorf("check failure, want nil, got err: %v", err) | |
| } | |
| defer patches.Reset() | |
| defer patches.Reset() | |
| err = a.ExecMountScripts() | |
| if err != nil { | |
| t.Errorf("check failure, want nil, got err: %v", err) | |
| } |
| a := &AlluxioFileUtils{log: fake.NullLogger()} | ||
| patches := gomonkey.ApplyPrivateMethod(*a, "exec", ExecErr) |
There was a problem hiding this comment.
Inconsistent test mocking approaches within the same file. Only TestAlluxioFileUtils_ExecMountScripts was updated to use the new ARM-compatible gomonkey approach (using instance instead of reflect.TypeOf), while all other tests in this file still use reflect.TypeOf(AlluxioFileUtils{}).
For consistency and to ensure all tests work correctly on ARM architecture, consider updating all tests in this file to use the same approach as TestAlluxioFileUtils_ExecMountScripts, or provide a justification for the partial update.
Signed-off-by: TzZtzt <trafalgarz@outlook.com>
- Consolidate aarch64 and arm64 architecture checks into a single condition Signed-off-by: TzZtzt <trafalgarz@outlook.com>
|
|
[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 |
Ⅰ. Describe what this PR does
Add arm arch runner in Fluid's github actions to check building in ARM.
buildandunittestare separated into two Github Actions which can run in parallelⅡ. Does this pull request fix one issue?
NONE
Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews