Automate merges up

Story

As a maintainer, I want patch in older minor releases to automatically flow up to later minor releases.

Run test in test/live modes as well as dev

Currently the CI is only ever run in dev mode.

It's unclear if there are unit or behat tests that correctly test if things work as expected in test or live mode. If there aren't, we should either run CI in those modes (and see what breaks) or add appropriate tests that set the mode to test or live for the duration of those tests.

Broken builds from initial github actions CI config PRs

We've added github actions CI but many of the builds are broken. We have decided to merge in CI config and treat the broken builds separately, so this issue is just to track fixing the broken builds.

Module	Build	Reason	Effort
silverstripe/silverstripe-elemental-bannerblock	build link	failing behat test php 8 mysql 8 (was re-run)	medium
silverstripe/silverstripe-externallinks	build link	broken behat test	medium
silverstripe/silverstripe-ckan-registry	build link	behat failure	medium
silverstripe/silverstripe-asset-admin	build link	behat 7.3 failed after re-run	medium
silverstripe/silverstripe-subsites	build link	failing behat tests	medium
silverstripe/silverstripe-graphql	build link	behat php 7.4 asset-admin 1.11.x-dev failed after 2x re-run (not same failure as asset-admin or recipe-cms)	medium
silverstripe/silverstripe-login-forms	build link	sass-lint linting admin scss	medium
silverstripe/cwp-agencyextensions	build link	failing yarn test	medium
silverstripe/cwp-watea-theme	build link	can't build css	medium
silverstripe/cwp-search	build link	prefer-lowest solr connection issue	hard
silverstripe/silverstripe-contentreview	build link	wrong version of node - have tried both 6 and 10, pure guess module want 6 while admin uses 10	hard
symbiote/silverstripe-advancedworkflow	build link	git diff failing in js test	hard
silverstripe/cow	build link	out of date requirements	hard
silverstripe/recipe-solr-search	build link	unit test failure	hard
silverstripe/silverstripe-behat-extension	build link	phplinting job only cannot install, likely conflict cow has with symfony and behat requirements	hard
silverstripe/silverstripe-config		codesniffer not added to composer.json as dev dependency	easy
silverstripe/doorman		codesniffer not added to composer.json as dev dependency	easy

silverstripe/recipe-blog	build link	failing unit tests, probably phpunit 5 on silverstripe-comments possible needs to require a theme	medium - do last
silverstripe/recipe-cms	build link	failing graphql unit test, same failure as on graphql 3.7 pr	medium - do last
silverstripe/recipe-kitchen-sink	build link	various failures	medium - do last
silverstripe/silverstripe-installer	build link	pdo utf8mb3 unit test failure + same failure as on graphql 3.7 pr	medium - do last
silverstripe/recipe-content-blocks	build link	prefer-lowest a mess, 7.4 plus missing graphql FakeResolveInfo	medium - do last
silverstripe/recipe-core	build link	prefer-lowest a mess, 7.4 plus various unit test failures	medium - do last

Guy PRs

Steve PRs

Require v1 of gha actions

Update all gha requirements to @v1 and tag stable versions

gha modules

gha-ci
gha-run-tests
gha-generate-matrix
gha-auto-tag
gha-pull-request
gha-tag-release
gha-update-js
gha-keepalive

ACs - for each gha module

Create pull-requests to change gha requirements from @main to @v1
Create 1.0 branches
Release 1.0.0
Confirm that corresponding v1 tags were created by auto-tag.yml workflow

PRs

The `save-state` command is deprecated and will be disabled soon

Warning: The save-state command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

The above warning is showing in a lot of our CI runs. We should stop using deprecated save-state.

node12 github actions are deprecated

https://github.com/silverstripe/silverstripe-installer/actions/runs/3220762331
Github actions is throwing some warnings at us:

Node.js 12 actions are deprecated. For more information see: https://github.blog/changelog/2022-09-22-github-actions-all-actions-will-begin-running-on-node16-instead-of-node12/. Please update the following actions to use Node.js 16: actions/checkout, actions/checkout

Acceptance criteria

any action currently using node12 is updated to use node16

Fix Logger - silverstripe.log

Should just be a matter of changing "notice" to "info" - it worked for https://github.com/emteknetnz/silverstripe-installer/blob/pulls/4/test-depr-reroute-cms4/app/_config/logger.yml#L15 for showing deprecated warnings in silverstripe.log

I believe that is there are zero log entires, then silverstripe.log is simply not created by the Logger service, hence it never shows in artifacts

If the resulting silverstripe.log generated from "info" is too annoying and provides no values, then revert it to "notice" and leave an inline comment in bash saying 'just change this to "info" if you need this to actually do something'

PRs

#52

Add ci.yml workflow to modules

ACs

Add workflow to all supported silverstripe modules to use ci.yml - NOTE: this needs to target the oldest supported branches and be merged-up, not just the default branches
Best effort to maintain any custom logic within .travis.yml files
Maintain travis ci support for a while to validate things work as expected
Update all module readme status badges from travis-ci to github actions default branch
Ensure workflows do not run on forks, or at least scheduled workflows do not run on forks
Workflows are green

ACs Round 2 PRs

Remove .travis.yml files
In ci.yml and keepalive.yml (and readmes gha-ci, gha-keepalive and gha-update-js) change startsWith(github.repository, '<account>/') to github.repository_owner == '<account>'
Some travis badges were not updated e.g. silvestripe/silversripe-postgresql - the initial $find in module-standardiser was too strict
Remove travis badges and add new CI badges to https://github.com/silverstripe/supported-modules
- Use the updated table to populate the supported modules table in silverstripe.org

Notes

Usually makes more sense to just hardcode things in ci.yml rather than trying to make ci.yml move configurable
Workarounds to make workflows not run on forks - https://github.com/microsoft/TypeScript/pull/48693/files - https://github.community/t/do-not-run-cron-workflows-in-forks/17636
When creating a pull-request to add ci.yml to a repo, it will run on pull-request BUT ONLY if there are free github action worker available. It will not queue the workflow like you would expect. This is certainly inconvenient.
I have validated that account that have maxed out their github actions workers will still queue up ci runs, provided there is a ci.yml defined on the repo - i.e. you cannot bypass ci.yml by just spamming pull-requests. Test here.

Custom .travis files

~~assets - PHP_COVERAGE_SUITE=assets .. to stop travis timing out after 10 minutes~~ not required
graphql (both 3 & 4)
framework - custom matrix cos phpcoverage was giving weird results on travis
auditor - mfa + session_manager custom

Custom .travis files, though probably does not need to be custom

recipe-core - requires themes simple + admin + versioned - but there's no behat
vendor-plugin - may work out of the box with ci.yml
sharedraftcontent
userforms
spellcheck - apt packages (ci.yml has this hardcoded)

Graphql3

Graphql4 will install by default
Add explicitly add a graphql3 behat 'extra_job' for asset-admin, versioned-admin and elemental

missing .travis.yml / ancient non-shared .travis

mink-facebook-web-driver (only unit testing thing in vendor folder, should probably just remove)
recipe-plugin (no current CI, no unit tests)
recipe-testing (no current CI, no unit tests, no code)
serve (has unit tests)
testsession (no current CI, no unit tests)

.travis files where REQUIRE_EXTRA should be moved to composer.json require-dev

Note: keep silverstripe/frameworktest as a requirement in module ci.yml, rather than composer require-dev, because it should eventually get split to move the 'behat test models' to a separate module - issue. If/when that happens, consider moving the requirement of the new module to require-dev

PRs - CI

PRs - Modules

PARENT_BRANCH detection doesn't work with default actions/checkout

Parent branch detection is currently implemented on gha-generate-matrix to get the parent branch on pull-requests only. This isn't required pull-request targets following the standard branch naming convention. The original intention was to use this to work out which version of installer to use, though this still wouldn't work for non-lockedstepped modules. For non-lockedstepped modules, they should have no issues working with the latest minor of installer, however the version of php used does matter in the case of php 8.1, so limiting to installer 4.10.x-dev is sometimes desirable.

The actual use case for getting the parent branch is fairly limited, it's for running CI on push events on forked repos when the common naming convention isn't used, for example myaccount-patch-1. One solution would be to add a step on gha-generate-matrix

===

Old description:

In gha-ci + gha-generate-matrix, parent branch detection does seem to work, presumably because actions/checkout on retrieves the latest commit by default

PARENT_BRANCH was put in place to work out COMPOSER_ROOT_VERSION on push events on forked repos

Options include:

!change the fetch-depth of actions/checkout to 0 to retrieve all commits (framework will suffer the worst)
change the fetch-depth to 10 or 20 and assume that it covers all commits in any non-squashed pull-request
somehow retrieve this information from the github api (unlikely to work)
(probably the best approach) use the github api to pull the latest minor version of the repo, and set COMPOSER_ROOT_VERSION to that. If a fork points to say 4.10, and the latest version is 4.11, it's very unlikely that using installer 4.11 is going to break the fork

Central action that pings repo endpoints as a form of keep-alive

Have a central repo ping a repo endpoint to prevent github-actions from silently stopping after 60 days of non-activity.

Automatically tag patch releases

Story

As contributor, I want my code changes to be available as quickly as possible to the wider community.

Don't add mikey179/vfsstream in CI - should be require-dev in composer.json

mikey179/vfsstream is being added for every module, regardless of if it is needed or not. We ideally shouldn't just add a package here that most modules don't need, as it means we could use some API from it in a non-assets module and the tests will pass even though it's not a dependency in that module.

The package is "Required for any module/recipe that runs silverstripe/assets unit tests" - so the composer.json for those modules/recipes should have it as a dev dependency.

What's more, it is being added with the constraint ^1.6.10 in CI which is necessary for PHP 8.1 but silverstripe/assets has it constrained too loosely as ^1.6.

composer require mikey179/vfsstream:^1.6.10 --dev --no-update

Use file permissions as per "server requirements" documentation

Currently apache is being run as runner:docker which gives it write access to everything in the project root.
Instead of this, we should adhere to (and amend if necessary) the permissions we say are necessary in the server requirements documentation.
This ensures both that the documentation is correct, and that we do not create regressions that make new permissions necessary.

Remove hard-coded references to classes that were previously incorrectly named

There are hard-coded references to files that were incorrectly named. I have already created PRs (listed below) to fix the naming of those classes.
References to these files should be removed from CI.

PRs

CI isn't working for module tags

Modules - framework

~~-release branch not installing - https://github.com/silverstripe/silverstripe-framework/actions/runs/3441318809~~

-beta1 tag not installing - https://github.com/silverstripe/silverstripe-framework/actions/runs/3441319137

Other tag not installing - https://github.com/silverstripe/silverstripe-framework/actions/runs/4042593676

Installer

~~-release branch installs - https://github.com/silverstripe/silverstripe-installer/actions/runs/3441336248/jobs/5740760583~~
-beta1 tag installs - https://github.com/silverstripe/silverstripe-installer/actions/runs/3441336653

Acceptance criteria

CI works for ~~-release and~~ -beta1 (and other pre-release) branches/tags for modules and installer

Stretch goal

CI works for stable and pre-release tags

Note:

CI for the -release branches was fixed with https://github.com/silverstripeltd/product-issues/issues/635

Follow-up tasks

ACs

Ensure new tags trigger builds
Update confluence release document that update the map of installer version -> php versions upon release of a new minor

EPIC: Standardise GitHub Action CI on all our modules

We want to be in a position to replace Travis on all our modules with GitHub actions.

Use codeowners and branch protection

Use codeowners and branch protection to further ensure workflow files are not modified https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners#codeowners-and-branch-protection

Special fail on deprecation warnings job

Get gha-generate-matrix to create a special job that only runs on the silverstripe account when using the latest version of installer, and only when using the maximum version of php. Get this to somehow upgrade E_DEPRECATED and E_USER_DEPRECATED php errors to something creates an exit code of 1 so the phpunit jobs fail.

May wish to add the following:

Only limit to running on the silverstripe account
Only limit to running on non-pull-request events
The equivalent of 'allow_failures' - not sure if that's possible - 'continue-on-error' might be the best we can do

Automated process to review workflows for common risk

Central action that reviews contents of yml files in .github/workflow folders to ensure that we don't use the pull_request_event trigger outside of an allow list of other actions

Find a cleaner approach to removing unnecessary tests

There is a section in ci.yml where any file ending with Test.php in a tests directory is deleted.

Remove vendor unit tests files that were installed because of the use of --prefer-source
Some older silverstripe vendor modules may still have the phpunit5 setUp() signatures without :void which when loaded will throw fatal PHP errors.
We cannot simply get rid of the 'tests' folders because behat requires vendor/silverstripe/[framework|cms]/tests/behat/serve-bootstrap.php

There are some hardcoded exceptions to this, though, like any file named ExtendTest.php because:

Make an exception for 'ExtendTest.php' which is a misnamed TestOnly non-test class

Except there are a lot of other classes in the same directory as ExtendTest.php which are also DataObjects and should be similarly excluded.

What's more, the only reason specific files are being targetted like this is apparently:

We cannot simply get rid of the 'tests' folders because behat requires vendor/silverstripe/[framework|cms]/tests/behat/serve-bootstrap.php

The only reason this wasn't done initially is because there was resistance to touch it at that stage since it it took a long time getting this fragile approach working in the first place.

Proposal

According to the above comment, only two files from tests directories actually need to be kept. So instead of going through and deleting all the files with seemingly arbitrary exceptions, we should just delete all of the tests directories, with exception of those in framework and cms, where we should delete all files inside tests except the two we need to keep.

If the above comment is found to be incorrect, at the very least it should be updated, and we should find a more robust way of setting exceptions since the current way seems to be pretty inconsistent with what is or isn't excluded.

Remove guzzle 6 cow support

This PR allowed installing cow ~2.0 which uses guzzle 6. It should be reverted at some point.

It's entirely possible that something else (e.g. changing the PHP version of the linting job to PHP 8) will mean that dev-master is no longer installed, and instead the older 2.0.x version is installed

If we change the linting done in cow schema:validate (admittedly very unlikely) on dev-master, then we'll still be using the old linting logic in 2.0.x

silverstripe / gha-ci Goto Github PK

gha-ci's People

Contributors

Watchers

Forkers

gha-ci's Issues

Story

Guy PRs

Steve PRs

gha modules

ACs - for each gha module

PRs

Acceptance criteria

PRs

ACs

ACs Round 2 PRs

Notes

Custom .travis files

Custom .travis files, though probably does not need to be custom

Graphql3

missing .travis.yml / ancient non-shared .travis

.travis files where REQUIRE_EXTRA should be moved to composer.json require-dev

PRs - CI

PRs - Modules

Story

PRs

Modules - framework

Installer

Acceptance criteria

Stretch goal

Note:

ACs

Proposal

Recommend Projects

Recommend Topics

Recommend Org