workflows: windows: Add a testing flow with using ARM64 Windows runner - #10931
Conversation
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
WalkthroughReplaces a fixed Windows runner with a matrix-driven Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Matrix as Matrix Strategy
participant Job as call-build-windows-unit-test
participant Runner as Runner (matrix.config.os)
Dev->>GH: Push / Open PR
GH->>Matrix: Expand strategy (win32, win64, win64-arm64)
loop For each matrix.config
Matrix->>Job: Provide config (os, arch, triplet, system options)
Job->>Runner: Select runner via matrix.config.os
Note right of Runner #DDEEFF: Runner may be `windows-latest` or `windows-11-arm`
Runner->>Job: Execute CI steps (install deps, build, run unit tests)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/call-windows-unit-tests.yaml (4)
88-94: Avoid bash dependency on Windows ARM runnersshell: bash may not be present on self-hosted ARM runners. Use PowerShell to compute the UTC date.
- - name: Get Date + - name: Get Date id: get-date - run: | - echo "date=$(/bin/date -u "+%Y%m%d")" >> $GITHUB_OUTPUT - shell: bash + run: | + $d = Get-Date -AsUTC -Format yyyyMMdd + "date=$d" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 + shell: pwsh
130-157: Step name still says “only for x86 and x64”This runs for all matrix entries including ARM64. Rename for clarity.
- - name: Build unit-test for Fluent Bit packages (only for x86 and x64) + - name: Build unit tests for Fluent Bit packages (Windows)
168-171: dumpbin availability on ARM runnersdumpbin is part of MSVC tools and may not be present (or on PATH) on an ARM64 host installation. Consider guarding this step or verifying tool presence.
Example guard:
- name: Display dependencies w/ dumpbin - run: | - dumpbin /dependents .\bin\fluent-bit.exe + run: | + if (Get-Command dumpbin -ErrorAction SilentlyContinue) { + dumpbin /dependents .\bin\fluent-bit.exe + } else { + Write-Host "dumpbin not available on this runner; skipping." + } working-directory: build
173-177: Second step name also mentions “only for x86 and x64”Rename for consistency with the ARM64 addition.
- - name: Build unit-test for Fluent Bit packages (only for x86 and x64) + - name: Run unit tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/call-windows-unit-tests.yaml(2 hunks)
🔇 Additional comments (3)
.github/workflows/call-windows-unit-tests.yaml (3)
31-31: Good change: runner selection via matrix is correctUsing matrix.config.os decouples runner choice from job logic and enables the ARM entry cleanly.
42-48: Explicit os per matrix entry is fineExplicitly pinning os for x86/x64 keeps behavior stable while enabling ARM separately.
49-55: Fix ARM64 job: arch/os mismatch on windows-11-arm runner
- arch: amd64_arm64 selects an x64-host→ARM64 cross toolchain; combined with os: windows-11-arm (an ARM host) will likely fail. For native ARM runners use arch: arm64 and drop cross CMake flags.
- Proposed fix (native ARM64 build):
- - name: "Windows 64bit (Arm64)" - arch: amd64_arm64 - cmake_additional_opt: "-DCMAKE_SYSTEM_NAME=Windows -DCMAKE_SYSTEM_VERSION=10.0 -DCMAKE_SYSTEM_PROCESSOR=ARM64" + - name: "Windows 64bit (ARM64)" + arch: arm64 + cmake_additional_opt: "" vcpkg_triplet: arm64-windows-static cmake_version: "3.31.6" os: windows-11-arm
- If you intend to cross-compile on an x64 runner, keep arch: amd64_arm64 and set os: windows-latest (not an ARM runner).
- Update the workflow title/step names that say “only for x86 and x64” to include ARM64.
- Confirm that the label windows-11-arm exists on your runners and that Visual Studio toolchains support ARM64 host (VsDevCmd for arm64). If self-hosted, ensure the label maps to an ARM64 machine.
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
|
Now, ARM64 Windows workflow of unit testing is working: |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/call-windows-unit-tests.yaml (1)
2-2: Update workflow description to reflect ARM64 support.The workflow name still indicates "(only for x86 and x64)" but now includes ARM64 support.
-name: Reusable workflow to run unit tests on Windows packages (only for x86 and x64) +name: Reusable workflow to run unit tests on Windows packages (x86, x64, and ARM64)
🧹 Nitpick comments (1)
.github/workflows/call-windows-unit-tests.yaml (1)
130-130: Update step names to reflect accurate scope.The step names mention "(x86, x64, and ARM64)" but should clarify that ARM64 is cross-compiled, not natively built and tested on ARM64 hardware. This could be misleading about the actual testing coverage.
- - name: Build unit-test for Fluent Bit packages (x86, x64, and ARM64) + - name: Build unit-test for Fluent Bit packages (x86, x64, and ARM64 cross-compile)- - name: Run unit tests for Fluent Bit packages (x86, x64, and ARM64) + - name: Run unit tests for Fluent Bit packages (x86, x64, and ARM64 cross-compile)Also applies to: 173-173
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/call-windows-unit-tests.yaml(5 hunks)
🔇 Additional comments (5)
.github/workflows/call-windows-unit-tests.yaml (5)
31-31: Perfect matrix-driven runner configuration.The change from a fixed
windows-latestto a dynamic${{ matrix.config.os }}enables proper OS-specific runner selection for each matrix configuration, which is essential for the ARM64 support being added.
42-48: LGTM! Explicit OS configuration for existing Windows variants.Adding explicit
osfields to existing configurations maintains compatibility while enabling the new ARM64 configuration. The use ofwindows-latestfor x86 and x64 ensures these continue to use the most current Windows runner image.
73-73: Good: Updated to use portable CMake package.The change from
cmaketocmake.portableis appropriate for the workflow context, providing a more reliable installation method.
49-54: Verify ARM64 runner availability and matrix arch mapping.
- The workflow defines a matrix entry for "Windows 64bit (Arm64)" with arch:
amd64_arm64, CMake targeting ARM64 andos: windows-11-arm— .github/workflows/call-windows-unit-tests.yaml:49-54.- The Developer Command Prompt action is invoked with
arch: ${{ matrix.config.arch }}(ilammy/msvc-dev-cmd) — .github/workflows/call-windows-unit-tests.yaml:78-82. Confirm the action supports the literal valueamd64_arm64or change the matrix value toarm64for the ARM runner.- call-build-windows.yaml already guards
amd64_arm64with anarmSupportedcheck (see .github/workflows/call-build-windows.yaml:177,186); either add a similar guard here or normalize the matrix arch value.
173-177: Run ARM64 tests on an arm64 runner (windows-11-arm) or acknowledge emulation. GitHub's windows-11-arm runners are arm64 and run ARM64 Windows binaries natively; if you cross-compile on an x64 runner (amd64_arm64) those ARM64 binaries will execute under x64 emulation. Verify this workflow's runs-on and either run the ARM64 tests on runs-on: windows-11-arm or document/accept the emulation trade-off.
|
@cosmo0920 can we adjust the ignore filters for the unit test workflow so these changes actually run? It's using the skip workflow at the moment. |
Yup. At the moment, this PR just skips to run workflows. So, if we accept to pile up unit testing queue, we can adjust for ignore filters. BTW, I rebased off master to include fix for failing unit tests on Windows. The corresponding manually kicked off testing workflow job is: |
3 participants
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit