-
Notifications
You must be signed in to change notification settings - Fork 476
[ci][osx] build libboinc with cmake in parallel for x64 and arm64. #6289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
7029b89
to
246c8a6
Compare
924c5d5
to
236899e
Compare
236899e
to
887dbc2
Compare
11f8884
to
f3fb7a5
Compare
Run unit tests after x64 build on CI. Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
82e36d8
to
f4849d6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for parallel macOS builds for x64 and arm64, updates test exclusions and assertions for architecture differences, and adjusts CI scripts and CMake files accordingly.
- Introduce a GitHub Actions matrix for macOS x64/arm64 builds and run unit tests on the second build
- Adjust unit-test preprocessor guards and platform‐specific expected outputs
- Update CMake and shell scripts to locate GTest, include directories, libraries, and trigger the correct tests per architecture
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
tests/unit-tests/lib/test_util.cpp | Modified preprocessor guard to skip Linux-specific test on Apple |
tests/unit-tests/lib/test_str_util.cpp | Added architecture-specific expected outputs for precision_time_to_string |
tests/unit-tests/CMakeLists.txt | Configured GTest paths and include dirs for Apple builds |
tests/executeUnitTests.sh | Made module list conditional on OSTYPE |
osx/ci_build_libs_cmake.sh | Switched to single-triplet builds via command-line argument |
lib/CMakeLists.txt | Added synch.h and synch.cpp to Apple source and header lists |
3rdParty/vcpkg_ports/configs/libs/vcpkg.json | Added "gtest" to the list of dependencies |
.github/workflows/osx.yml | Implemented macOS build matrix, updated build/test steps, and artifact naming |
.codecov.yml | Incremented after_n_builds counters |
Comments suppressed due to low confidence (1)
tests/unit-tests/lib/test_str_util.cpp:230
- The macro
__arm64__
may not be defined by all compilers; consider using a standard identifier like__aarch64__
or inject an explicit define via CMake for arm64 builds.
#ifdef __arm64__
// here we have an undefined behavior that gives negative value on x64 and positive value on arm64 | ||
#ifdef __arm64__ | ||
EXPECT_STREQ(buf, "2361-03-21 19:15:10.2147483647"); | ||
#else | ||
EXPECT_STREQ(buf, "2361-03-21 19:15:10.-2147483648"); | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Relying on undefined behavior for large timestamps leads to brittle, architecture-dependent tests. It’s better to fix precision_time_to_string
to handle overflows or clamp inputs so outputs are consistent across platforms.
// here we have an undefined behavior that gives negative value on x64 and positive value on arm64 | |
#ifdef __arm64__ | |
EXPECT_STREQ(buf, "2361-03-21 19:15:10.2147483647"); | |
#else | |
EXPECT_STREQ(buf, "2361-03-21 19:15:10.-2147483648"); | |
#endif | |
// Large timestamps are clamped or handled consistently across platforms. | |
EXPECT_STREQ(buf, "2361-03-21 19:15:10.9999"); |
Copilot uses AI. Check for mistakes.
run: osx/ci_build_libs_cmake.sh ${{ matrix.triplet }} | ||
|
||
- name: run unit tests | ||
if: ${{ success() && matrix.triplet == 'arm64-osx' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test step only runs on the arm64 job. If tests should run after the x64 build, change the condition to matrix.triplet == 'x64-osx'
or remove the filter to run on both.
if: ${{ success() && matrix.triplet == 'arm64-osx' }} | |
if: ${{ success() }} |
Copilot uses AI. Check for mistakes.
…behavior that gives negative value of x64 and positive value of arm64. Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
f4849d6
to
79fc005
Compare
Run unit tests after x64 build on CI.