-
Notifications
You must be signed in to change notification settings - Fork 711
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
Update a workflow file for GitHub Actions to test on Windows #1046
Conversation
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.
Wait, what is the use of the tests always succeeding?
Wouldn't it be much better to fix the failing tests instead?
Exactly. It's much better to fix the failing tests. I would like to see this PR merged after there are no failing tests. |
I have removed “exit 0” and "continue-on-error: true". Test results on Windows are here.
|
@wisskid
Thank you. |
@wisskid I tested https://github.com/smarty-php/smarty/pull/1056/files on windows, it works fine and solved the issue #1018. Waiting for the merge. Thanks |
@matsuo I merged the 3 underlying PRs, but the windows checks are still failing. I think you need to update/rebase your branch against the HEAD for smarty-php:master maybe? |
@wisskid Thank you so much. |
@matsuo awesome! Thank you for your work. |
@wisskid Thank you too! |
@wisskid thank you for the merging! |
@rodriguezny I released 5.4.1 just now. Let me know if it solves your problems. |
I update Smarty to v5.4.1, it solves my problem. Thanks! |
Thank you for your developing!
This PR makes it possible to run tests on Windows with GitHub Actions.
Because I saw the comment in #1018 .
#1018 (comment)
I have adjusted the CI results to always be green on Windows because there are tests that fail on Windows now.
I would like to see “exit 0” and "continue-on-error: true" removed in the future.
I would be happy if this PR could help solve the issue.