-
Notifications
You must be signed in to change notification settings - Fork 182
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
Add Windows CI #61
Add Windows CI #61
Conversation
It can be advantageous to have distinct workflows on a per-operating system basis, to quickly see the scope of a failure
Here is the failure:
That's weird, it looks like some compiler bug. |
@scivision now all the tests pass! I disabled qp for now. |
.github/workflows/ci_windows.yml
Outdated
if: failure() | ||
with: | ||
name: WindowsCMakeTestlog | ||
path: build/Testing/Temporary/LastTest.log |
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.
Small nit: a final new line would be good here. But this is a triviality so ¯_(ツ)_/¯
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.
Fixed in 6b4d861. My vim adds it automatically, but I haven't touched this file.
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.
[Deleted duplicate comment]
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.
Looks good to me!
A question to consider: When do we want to split out separate workflows vs use a test matrix approach? There is a bit of duplicate code in the Windows CI and I think that it could be integrated into the main CI workflow.
Given that the quadruple precision is buggy on Windows, probably the best way forward is to split the tests into a single/double file, and a quadruple file. The quadruple file test would be skipped on Windows. |
@zbeekman do you want to merge this first, so that you can work on consolidating the CI into one file? And I can work on getting quadruple working again at least on Linux and macOS? |
No, that's why we should all allow "edits by maintainers" in our PRs. Yes, if you can figure out how to consolidate the Windows test into the same matrix, I think this would be the best. |
@zbeekman go ahead and merge this when you are ready. |
I think we're mis-communicating again, sorry, probably my fault. I'll merge this now. |
(My "No" meant "No, I don't mind", as a reply to yours "Mind if I also rebase and merge #64?".) |
I am trying to troubleshoot #58.