Skip to content

[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

Merged
merged 2 commits into from
May 21, 2025

Conversation

AenBleidd
Copy link
Member

Run unit tests after x64 build on CI.

@AenBleidd AenBleidd force-pushed the vko_enable_macos_tests_on_ci branch 2 times, most recently from 7029b89 to 246c8a6 Compare May 1, 2025 02:27
@AenBleidd AenBleidd force-pushed the vko_enable_macos_tests_on_ci branch 2 times, most recently from 924c5d5 to 236899e Compare May 20, 2025 23:25
@AenBleidd AenBleidd changed the title [ci][osx] build libboinc with cmake in paraller ofr x64 and arm64. [ci][osx] build libboinc with cmake in paraller for x64 and arm64. May 20, 2025
@AenBleidd AenBleidd force-pushed the vko_enable_macos_tests_on_ci branch from 236899e to 887dbc2 Compare May 20, 2025 23:35
@AenBleidd AenBleidd changed the title [ci][osx] build libboinc with cmake in paraller for x64 and arm64. [ci][osx] build libboinc with cmake in parallel for x64 and arm64. May 20, 2025
@AenBleidd AenBleidd force-pushed the vko_enable_macos_tests_on_ci branch 3 times, most recently from 11f8884 to f3fb7a5 Compare May 21, 2025 00:33
Run unit tests after x64 build on CI.

Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
@AenBleidd AenBleidd force-pushed the vko_enable_macos_tests_on_ci branch 2 times, most recently from 82e36d8 to f4849d6 Compare May 21, 2025 02:21
@AenBleidd AenBleidd marked this pull request as ready for review May 21, 2025 02:24
@Copilot Copilot AI review requested due to automatic review settings May 21, 2025 02:24
@AenBleidd AenBleidd added this to the Client/Manager 8.4.0 milestone May 21, 2025
@github-project-automation github-project-automation bot moved this to In progress in Client/Manager May 21, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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__

Comment on lines +229 to +234
// 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
Copy link
Preview

Copilot AI May 21, 2025

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.

Suggested change
// 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' }}
Copy link
Preview

Copilot AI May 21, 2025

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.

Suggested change
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>
@AenBleidd AenBleidd force-pushed the vko_enable_macos_tests_on_ci branch from f4849d6 to 79fc005 Compare May 21, 2025 02:30
@AenBleidd AenBleidd merged commit 38dd785 into master May 21, 2025
175 checks passed
@AenBleidd AenBleidd deleted the vko_enable_macos_tests_on_ci branch May 21, 2025 07:41
@github-project-automation github-project-automation bot moved this from In progress to Merged in Client/Manager May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

1 participant