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

Add GitHub Workflows CI, including testing on PHP 8 and on macOS/Windows/Ubuntu #5268

Merged
merged 1 commit into from Nov 29, 2020

Conversation

sanmai
Copy link
Contributor

@sanmai sanmai commented Nov 14, 2020

This PR introduces a GitHub workflow-based CI, that replicates most of the testing done on Travis CI, AppVeyor, CircleCI.

  • Like on Travis CI on Linux we build on PHP 7.1 ... 8.0. There are things done on Travis CI, like static analysis and coverage reporting, which are not replicated yet, to make this PR shorter and simpler to review.
  • On Windows we build on PHP 7.3 and 5.6, just like it is done on AppVeyor.
  • On macOS we build on PHP 7.4, just like on CircleCI.

After this PR AppVeyor and CircleCI builds will become pretty much redundant.

Fixes #5179. Extracted from #5262. Without PHPUnit 9.x yet.

@sanmai sanmai changed the title Add GitHub Workflows CI Add GitHub Workflows CI, test on PHP 8 Nov 14, 2020
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@@ -44,6 +44,10 @@ public static function setUpBeforeClass()
{
parent::setUpBeforeClass();

if ('\\' === \DIRECTORY_SEPARATOR) {
static::markTestIncomplete('This test is broken on Windows');
Copy link
Contributor Author

@sanmai sanmai Nov 15, 2020

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.

@sanmai sanmai changed the title Add GitHub Workflows CI, test on PHP 8 Add GitHub Workflows CI, test on PHP 8, on macOS/Windows/Ubuntu Nov 16, 2020
@keradus
Copy link
Member

keradus commented Nov 16, 2020

can you combine the effort with #5183 , please?

@sanmai
Copy link
Contributor Author

sanmai commented Nov 16, 2020

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.

@sanmai
Copy link
Contributor Author

sanmai commented Nov 16, 2020

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.

@sanmai
Copy link
Contributor Author

sanmai commented Nov 16, 2020

@keradus If you have something specific in mind you want ported from #5183, please let me know.

@sanmai
Copy link
Contributor Author

sanmai commented Nov 20, 2020

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.

@keradus keradus changed the base branch from 2.15 to 2.16 November 20, 2020 15:23
@sanmai sanmai force-pushed the GHWorkflows branch 2 times, most recently from 42e5078 to 3c5c81d Compare November 20, 2020 15:37
@keradus
Copy link
Member

keradus commented Nov 22, 2020

@keradus If you have something specific in mind you want ported from #5183, please let me know.

I don't want you to port from #5183 or vice versa.
We have 2 competitive PRs, let's avoid duplicating the effort in work on the job separately.

what I'm asking for is you and creator of #5183 to talk one to another and align how to achieve best outcome together ;)

.github/workflows/ci.yaml Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
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' }}
Copy link
Contributor Author

@sanmai sanmai Nov 23, 2020

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

Copy link
Contributor Author

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.

@sanmai

This comment has been minimized.

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/yaml.yaml Outdated Show resolved Hide resolved
keradus added a commit that referenced this pull request Nov 26, 2020
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()
Copy link
Member

Choose a reason for hiding this comment

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

👍

@keradus
Copy link
Member

keradus commented Nov 29, 2020

A week passed and TravisCI still didn't respond for request for open-source credits.
let's merge this one as-is to unblock further effort on other PRs, while keep this PR a starting point for potential future improvements to CICD

@keradus keradus changed the title Add GitHub Workflows CI, test on PHP 8, on macOS/Windows/Ubuntu Add GitHub Workflows CI, including testing on PHP 8 and on macOS/Windows/Ubuntu Nov 29, 2020
@keradus
Copy link
Member

keradus commented Nov 29, 2020

Thank you @sanmai.

@keradus keradus merged commit 844d0d1 into PHP-CS-Fixer:2.16 Nov 29, 2020
@SpacePossum
Copy link
Contributor

Thanks @sanmai ! : )

@sanmai sanmai deleted the GHWorkflows branch December 3, 2020 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use GH actions to test on PHP 8.0
4 participants