Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Bump macOS GitHub CI runner version #6006

Merged
merged 3 commits into from
May 22, 2024
Merged

Bump macOS GitHub CI runner version #6006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b045e5
Constrain matplotlib version
knelli2 May 18, 2024
b56a6e2
Unconstrain pandas version
nilsvu May 8, 2024
653173d
Upgrade macOS CI workflow
nilsvu May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/Tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ ${{ matrix.build_type }}-pch-${{ matrix.use_pch || 'ON' }}"
# Build all test executables and run unit tests on macOS
unit_tests_macos:
name: Unit tests on macOS
runs-on: macos-11
runs-on: macos-13
env:
# We install some low-level dependencies with Homebrew. They get picked up
# by `spack external find`.
Expand Down Expand Up @@ -1001,10 +1001,10 @@ ${{ matrix.build_type }}-pch-${{ matrix.use_pch || 'ON' }}"
$GITHUB_WORKSPACE
- name: Build unit tests
working-directory: build
# Build on 3 threads because GitHub's macOS VMs have 3 cores:
# Build on 4 threads because GitHub's macOS VMs have 4 cores:
# https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources
run: |
make -j3 unit-tests
make -j4 unit-tests
- name: Build executables
working-directory: build
run: |
Expand Down Expand Up @@ -1032,7 +1032,7 @@ ${{ matrix.build_type }}-pch-${{ matrix.use_pch || 'ON' }}"
- name: Run unit tests
working-directory: build
run: |
ctest -j3 --repeat after-timeout:3 --output-on-failure
ctest -j4 --repeat after-timeout:3 --output-on-failure
- name: Install
working-directory: build
run: |
Expand Down
5 changes: 3 additions & 2 deletions support/Python/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ humanize
# Jinja2: To work with placeholders in input files and submit scripts
Jinja2
numpy
matplotlib
# Newer version on macOS causes slow down in plotting utilities
matplotlib <= 3.8.4
# Pandas: To work with columns of data. Need a sufficiently recent version
# to do some fancy indexing in Status CLI
pandas ~= 1.5
pandas >= 1.5
pyyaml
# Rich: to format CLI output and tracebacks
rich >= 12.0.0
Expand Down
14 changes: 13 additions & 1 deletion tests/Unit/Utilities/ErrorHandling/Test_SegfaultHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
#include "Framework/TestingFramework.hpp"

#include <csignal>
#include <string>

#ifdef __APPLE__
#include "Parallel/Printf/Printf.hpp"
#endif
#include "Utilities/ErrorHandling/SegfaultHandler.hpp"


// [[OutputRegex, Segmentation fault!]]
SPECTRE_TEST_CASE("Unit.ErrorHandling.SegfaultHandler",
"[ErrorHandling][Unit]") {
Expand All @@ -17,4 +20,13 @@ SPECTRE_TEST_CASE("Unit.ErrorHandling.SegfaultHandler",
enable_segfault_handler();
CHECK_THROWS_WITH(std::raise(SIGSEGV),
Catch::Matchers::ContainsSubstring("Segmentation fault!"));

// For some reason on the newer macOS-13 CI runners, this test never prints out
// "Segmentation fault!". Somehow, the CHECK_THROWS_WITH must catch the signal
// and exit cleanly if the test is working. Because of this, the test fails
// because this is an output test. So we manually print out the message so the
// output test is happy.
Comment on lines +24 to +28
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like the test might actually work as intended on Mac but not Linux.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think mac does something different when handling signals vs exceptions. Because in the catch docs it says all the CHECK_(NO)THROW* macros handle exceptions and doesn't mention signals. So my best guess is that on linux this test works as intended because CATCH_THROWS_WITH doesn't catch the SIGSEGV, so the test "fails" and our output test catches that and reports that the test actually passes. However, on mac, somehow CHECK_THROWS_WITH catches SIGSEGV so the test "succeeds" and then our output test actually fails because no message was printed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We throw an exception in the signal handler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... Then I have no clue 🤷🏻

#ifdef __APPLE__
Parallel::printf("Workaround for Apple: Segmentation fault!\n");
#endif
}
Loading