Migrate to a hybrid of GH actions and Travis #5183

GrahamCampbell · 2020-10-18T13:29:18Z

Removes circle and appveyor, and some of the travis jobs. Long term, we could probably remove travis, but this is not necessary yet. The purpose of this PR is to have fast robust testing across multiple PHP versions and OSs, and not for overkill consolidation, taking extra developer time without significant benefit.

julienfalque

Damn, I was working on this too, but you were faster than me :)

tests/Fixtures/Integration/set/@PHP8Migration.test

phpunit.xml.dist

.github/workflows/tests.yml

julienfalque · 2020-10-19T08:21:56Z

Shouldn't the jobs appear in the checks of this PR?

GrahamCampbell · 2020-10-19T10:13:14Z

Related: PHPUnitGoodPractices/Traits#48.

GrahamCampbell · 2020-10-19T10:13:34Z

Shouldn't the jobs appear in the checks of this PR?

No, not until this is merged.

GrahamCampbell · 2020-10-21T21:24:45Z

Looks like the accessible object package is causing us issues. We need to mark it as compatible with PHP 8 and remove the HHVM conflict.

GrahamCampbell · 2020-10-21T21:34:46Z

See PHP-CS-Fixer/AccessibleObject#3.

GrahamCampbell · 2020-10-21T22:03:49Z

Looks like these tests have actually highlighted issues with our minimum symfony console versions.

GrahamCampbell · 2020-11-21T18:30:08Z

This is passing, and could be merged right away. A follow up PR can remove the rest of the travis jobs.

GrahamCampbell · 2020-11-21T18:30:17Z

https://github.com/GrahamCampbell/PHP-CS-Fixer/actions/runs/376358990

julienfalque · 2020-11-21T21:34:10Z

@GrahamCampbell The PR is still a draft, shall we mark it as ready for review?

keradus · 2020-11-22T19:26:37Z

@GrahamCampbell , can you talk to creator of #5268 and align how to achieve best outcome together ;)
We have 2 competitive PRs, let's avoid duplicating the effort in work on the job separately.

keradus · 2020-11-22T19:28:20Z

tests/AutoReview/TravisTest.php

- * @group auto-review
- * @group covers-nothing
- */
-final class TravisTest extends TestCase


👎 for dropping
i would rather expecting to have those tests migrated to read github actions config than fully dropped.

Do you consider this a blocker for merging? I feel like the GitHub UI already makes it pretty clear if stuff is missing, with the human readable job names I have gone for.

yes, I do

but pls, align with @sanmai to prepare single PR instead of putting more effort in one or another PR

sanmai

This is a fine PR, but since it has a lot if if:s, and separate jobs for different OSes (where it should have just used matrix), it'll be a headache to support.

sanmai · 2020-11-23T00:48:48Z

.github/workflows/tests.yml

+      - name: Set git to use LF
+        run: |
+          git config --global core.autocrlf false
+          git config --global core.eol lf


Why is this required?

I haven't seen anything bad happen without these lines.

Tests fail without I think.

Haven't seen this in other repos - can we try without this? Would be easier not to worry that this might affect some tests.

Are you sure? I have a workflow which does not fail without this on Windows.

.github/workflows/tests.yml

kubawerlos · 2020-11-23T23:07:58Z

.github/workflows/tests.yml

+      - name: Set git to use LF
+        run: |
+          git config --global core.autocrlf false
+          git config --global core.eol lf


Haven't seen this in other repos - can we try without this? Would be easier not to worry that this might affect some tests.

kubawerlos · 2020-11-23T23:10:39Z

.github/workflows/tests.yml

+          tools: composer:snapshot
+          coverage: none
+
+      - name: Setup problem matchers


What is this? Can you elaborate or link to some PR when it is run or better - failing a build for good reason?

Shows test failures inline on diffs on github's ui.

Is it? I don't think I can reproduce: https://github.com/sanmai/phpunit-primer/pull/6/files

Yes, that stuff works properly. It's exactly as documented in https://github.com/shivammathur/setup-php#phpunit.

Well, the thing is I made a test to fail intentionally, added these matchers, and there's nothing. How can we verify that this thing actually works?

@GrahamCampbell why closing the thread? looks like the question is still open

kubawerlos · 2020-11-23T23:11:41Z

.github/workflows/tests.yml

+        run: PHP_CS_FIXER_FUTURE_MODE=1 PHP_CS_FIXER_IGNORE_ENV=1 php php-cs-fixer --diff --dry-run -v fix
+        if: "matrix.php == '8.0'"
+
+  windows:


Why do we need this? Why OS cannot be part of matrix?

Each OS is different, and in the end, trying to put it all into one single thing ends up an absolute mess. Sometimes duplication is least worst. Certainly makes the config file more readable.

not sure, here it was added without that "absolute mess"
https://github.com/FriendsOfPHP/PHP-CS-Fixer/pull/5268/files#diff-944291df2c9c06359d37cc8833d182d705c9e8c3108e7cfe132d61a06e9133ddR33

GrahamCampbell · 2020-11-24T00:59:29Z

https://github.com/GrahamCampbell/PHP-CS-Fixer/actions/runs/380086877

sanmai · 2020-11-24T06:59:58Z

.github/workflows/tests.yml

+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php }}
+          tools: composer


This is as redundant as before.

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

GrahamCampbell · 2020-11-28T23:35:05Z

If someone else wants to finish this, they should take over from here.

keradus · 2020-11-29T00:31:15Z

Thanks for your effort on this PR, @GrahamCampbell .
It's a pity that we didn't manage to have single PR for GH actions.
Closing in favor of #5268, but let's treat #5268 as a starting point where we can all improve further, same time having CI for repo running

julienfalque reviewed Oct 18, 2020

View reviewed changes

GrahamCampbell force-pushed the gh-actions branch from 4f83cf9 to 42550bf Compare October 21, 2020 21:07

GrahamCampbell force-pushed the gh-actions branch from fac9e94 to f040faa Compare October 22, 2020 10:13

This was referenced Oct 22, 2020

Fixed minimum symfony versions #5196

Closed

Upgrade php-cs-fixer/accessible-object #5197

Closed

julienfalque mentioned this pull request Oct 27, 2020

DocumentationTest - decide about future of disabled test #5209

Closed

This was referenced Nov 13, 2020

Test with PHPUnit 9.x #5262

Closed

Prep work to migrate to PHPUnit 9.x #5269

Merged

keradus mentioned this pull request Nov 16, 2020

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

Merged

keradus changed the base branch from 2.15 to 2.16 November 20, 2020 15:23

GrahamCampbell force-pushed the gh-actions branch from f040faa to 3f18c51 Compare November 21, 2020 18:01

Migrate to GitHub actions

8edfa38

GrahamCampbell force-pushed the gh-actions branch from 3f18c51 to 8edfa38 Compare November 21, 2020 18:02

Update tests.yml

01cc7b9

GrahamCampbell marked this pull request as ready for review November 21, 2020 21:41

keradus reviewed Nov 22, 2020

View reviewed changes

sanmai reviewed Nov 23, 2020

View reviewed changes

keradus mentioned this pull request Nov 23, 2020

Urgent Need To Migrate Off Travis #5285

Closed

kubawerlos requested changes Nov 23, 2020

View reviewed changes

Skip broken auto-review test on PHP 8

3788108

GrahamCampbell added 2 commits November 24, 2020 00:52

Use stable composer v2

ed07dc1

Removed scheduled builds

a6b64e4

GrahamCampbell force-pushed the gh-actions branch from 7924e09 to a6b64e4 Compare November 24, 2020 00:52

sanmai reviewed Nov 24, 2020

View reviewed changes

sanmai mentioned this pull request Nov 25, 2020

Add yamllint workflow, validates .yaml files #5298

Merged

keradus closed this Nov 29, 2020

Uh oh!

Migrate to a hybrid of GH actions and Travis #5183

Migrate to a hybrid of GH actions and Travis #5183

Uh oh!

Conversation

GrahamCampbell commented Oct 18, 2020

Uh oh!

julienfalque left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

julienfalque commented Oct 19, 2020

Uh oh!

GrahamCampbell commented Oct 19, 2020

Uh oh!

GrahamCampbell commented Oct 19, 2020

Uh oh!

GrahamCampbell commented Oct 21, 2020

Uh oh!

GrahamCampbell commented Oct 21, 2020

Uh oh!

GrahamCampbell commented Oct 21, 2020

Uh oh!

GrahamCampbell commented Nov 21, 2020

Uh oh!

GrahamCampbell commented Nov 21, 2020

Uh oh!

julienfalque commented Nov 21, 2020

Uh oh!

keradus commented Nov 22, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GrahamCampbell Nov 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanmai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

GrahamCampbell Nov 24, 2020 •

edited

Loading