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.

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

Treat GitHub Actions CI like travis/appveyor CI #297

Merged
merged 1 commit into from
Aug 31, 2021

Conversation

bernhardmgruber
Copy link
Collaborator

@bernhardmgruber bernhardmgruber commented Aug 30, 2021

The problem with the current GitHub CI is that we do not know what instruction set the runners support. test.cmake has a detection mechanism for travis, but not for github actions, where it ran a simple CTest build (whatever that does, maybe testing what the host supports itself). On appveyor, a more complicated test setup is used, where CTest is invoked multiple times with different instruction sets, based on the environment variable subset. When the #278 was merged, the tests ran in a skylake CPU supporting all instruction sets. Current CI builds like #286 run on a broadwell CPU not supporting all instruction sets, which likely causes CTest to fail to enumerate the test suite.

@bernhardmgruber bernhardmgruber changed the title Treat GitHub Actions CI like travis CI Treat GitHub Actions CI like travis/appveyor CI Aug 30, 2021
test.cmake Outdated
@@ -434,7 +437,7 @@ macro(go)
unset(CTEST_NOTES_FILES) # less clutter in ctest -V output
if(res EQUAL 0)
set(test_results 0)
if(travis_os)
if(travis_os OR (github_ci AND NOT $ENV{RUNNER_OS} MATCHES "Windows"))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The question for me is, why were the Windows builds handled differently than the Linux ones? Most likely it was the build time, becaues VS takes 44min to build and run the test whereas clang takes 22min. Because we could just run all builds using the first branch.

@bernhardmgruber bernhardmgruber marked this pull request as ready for review August 30, 2021 12:50
@bernhardmgruber
Copy link
Collaborator Author

bernhardmgruber commented Aug 30, 2021

I think there could have been a change in CMake.
Appveyor runs with CMake 3.16.2 and GitHub actions with CMake 3.21.1. There is this recent issue with CMake: https://gitlab.kitware.com/cmake/cmake/-/issues/20283

Unfortunately, I cannot find a way to pass the -C inside test.cmake to ctest_test.

@bernhardmgruber
Copy link
Collaborator Author

I merged the Windows builds now into 1 CI job and let them use the same codepath as the Linux builds. That avoids multiple reconfigurations of ctest during 1 CI run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants