-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 GitHub Workflows CI, including testing on PHP 8 and on macOS/Windows/Ubuntu #5268
Conversation
@@ -44,6 +44,10 @@ public static function setUpBeforeClass() | |||
{ | |||
parent::setUpBeforeClass(); | |||
|
|||
if ('\\' === \DIRECTORY_SEPARATOR) { | |||
static::markTestIncomplete('This test is broken on Windows'); |
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.
It wasn't working on Windows before due to missing composer
executable. Otherwise put, this is a reflection of a status quo.
can you combine the effort with #5183 , please? |
This is possible, but #5183 does quite more than just to replicate existing processes. How about we merge this, get it running properly, and then I'll do what I can? It's really annoying to use a fork to keep track of the changes here. |
What's bad about #5183 it tries to do several things at once. One thing to add GitHub CI, and another is to remove AppVeyor etc. These could be done step-by-step, smoothly, not all at once. We did this for Infection in multiple steps, and it was just flawless. |
I feel like this PR can bring definite improvements to the speed and efficiency of the CI process. If there's anything I can do to help it getting reviewed/merged, I'll be happy to help. |
42e5078
to
3c5c81d
Compare
I don't want you to port from #5183 or vice versa. what I'm asking for is you and creator of #5183 to talk one to another and align how to achieve best outcome together ;) |
composer update --optimize-autoloader --no-interaction --no-progress --ignore-platform-req=hhvm ${{ matrix.composer-flags }} | ||
|
||
- name: Run tests | ||
continue-on-error: ${{ matrix.php-version == '8.0' }} |
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.
Several tests are broken on PHP 8 since not very long time ago due to a bug in TimeEfficientLongestCommonSubsequenceImplementation. It looks like this:
Undefined array key 140725147700175
/home/runner/work/PHP-CS-Fixer/PHP-CS-Fixer/vendor/php-cs-fixer/diff/src/v1_4/LCS/TimeEfficientLongestCommonSubsequenceImplementation.php:57
/home/runner/work/PHP-CS-Fixer/PHP-CS-Fixer/vendor/php-cs-fixer/diff/src/v1_4/Differ.php:230
/home/runner/work/PHP-CS-Fixer/PHP-CS-Fixer/vendor/php-cs-fixer/diff/src/v1_4/Differ.php:55
/home/runner/work/PHP-CS-Fixer/PHP-CS-Fixer/src/Differ/SebastianBergmannShortDiffer.php:37
/home/runner/work/PHP-CS-Fixer/PHP-CS-Fixer/tests/Differ/SebastianBergmannShortDifferTest.php:36
/home/runner/work/PHP-CS-Fixer/PHP-CS-Fixer/vendor/phpunitgoodpractices/traits/src/ExpectationViaCodeOverAnnotationTrait7.php:39
https://github.com/sanmai/PHP-CS-Fixer/pull/3/checks?check_run_id=1439764906#step:7:413
https://github.com/sanmai/PHP-CS-Fixer/runs/1439738972?#step:7:411
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.
This might be a PHP 8 bug FWIW.
This comment has been minimized.
This comment has been minimized.
This PR was merged into the 2.16 branch. Discussion ---------- Add yamllint workflow, validates .yaml files This has two effects: - It will validate `.yaml` files for general correctness everywhere except `vendor` and `dev-tools/vendor` - And as a side-effect, it might enable other workflows to happen. E.g. we will no longer need to consult with forks to see if #5295 #5268 #5183 etc are working. Commits ------- f6cdee1 Add yamllint workflow
@@ -181,4 +188,17 @@ private function getTravisJobs() | |||
|
|||
return $yaml['jobs']['include']; | |||
} | |||
|
|||
private function getGitHubPhpVersions() |
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.
👍
A week passed and TravisCI still didn't respond for request for open-source credits. |
Thank you @sanmai. |
Thanks @sanmai ! : ) |
This PR introduces a GitHub workflow-based CI, that replicates most of the testing done on Travis CI, AppVeyor, CircleCI.
After this PR AppVeyor and CircleCI builds will become pretty much redundant.
Fixes #5179. Extracted from #5262. Without PHPUnit 9.x yet.