-
-
Notifications
You must be signed in to change notification settings - Fork 354
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
Use php.ini-development by default instead of production one #450
Labels
Comments
I would not like to change it right now. May be in next major version this can be done. |
jrfnl
added a commit
to jrfnl/phpunit
that referenced
this issue
Jun 22, 2021
Turns out the default setting for `error_reporting` used by the SetupPHP action is `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT` and `display_errors` is set to `Off`. For the purposes of CI, I'd recommend running with `E_ALL` and `display_errors=On` to ensure **all** PHP notices are shown. Ref: * shivammathur/setup-php#469 * shivammathur/setup-php#450
sebastianbergmann
pushed a commit
to sebastianbergmann/phpunit
that referenced
this issue
Jun 22, 2021
Turns out the default setting for `error_reporting` used by the SetupPHP action is `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT` and `display_errors` is set to `Off`. For the purposes of CI, I'd recommend running with `E_ALL` and `display_errors=On` to ensure **all** PHP notices are shown. Ref: * shivammathur/setup-php#469 * shivammathur/setup-php#450
jrfnl
added a commit
to jrfnl/patchwork
that referenced
this issue
Aug 15, 2021
Similar to the change made in commit antecedent@2657911 , `zend.assertions` is set to `-1` by default, so needs to be turned on by setting it to `1` for the tests to work. Ref: shivammathur/setup-php#450
jrfnl
added a commit
to jrfnl/patchwork
that referenced
this issue
Aug 15, 2021
Similar to the change made in commit antecedent@2657911 , `zend.assertions` is set to `-1` by default, so needs to be turned on by setting it to `1` for the tests to work. Ref: shivammathur/setup-php#450
jrfnl
added a commit
to Yoast/i18n-module
that referenced
this issue
Oct 2, 2021
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes the, now redundant, `.travis.yml` configuration. * Updates the `.gitattributes` file to reflect the removal of the `.travis.yml` file. * Updates the "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT` and `display_errors` is set to `Off`. For the purposes of CI, especially for linting and testing code, I'd recommend running with `-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 3. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 4. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 5. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The toggle for running `composer install` with or without `--ignore-platform-reqs` has not been implemented as it is currently unnecessary. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Linting runs against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as succesfull, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
jrfnl
added a commit
to Yoast/i18n-module
that referenced
this issue
Oct 2, 2021
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes the, now redundant, `.travis.yml` configuration. * Updates the `.gitattributes` file to reflect the removal of the `.travis.yml` file. * Updates the "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT` and `display_errors` is set to `Off`. For the purposes of CI, especially for linting and testing code, I'd recommend running with `-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 3. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 4. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 5. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The toggle for running `composer install` with or without `--ignore-platform-reqs` has not been implemented as it is currently unnecessary. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Linting runs against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as succesfull, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
jrfnl
added a commit
to Yoast/i18n-module
that referenced
this issue
Oct 2, 2021
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes the, now redundant, `.travis.yml` configuration. * Updates the `.gitattributes` file to reflect the removal of the `.travis.yml` file. * Updates the "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 3. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 4. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 5. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The toggle for running `composer install` with or without `--ignore-platform-reqs` has not been implemented as it is currently unnecessary. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Linting runs against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as succesfull, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
jrfnl
added a commit
to Yoast/i18n-module
that referenced
this issue
Oct 2, 2021
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes the, now redundant, `.travis.yml` configuration. * Updates the `.gitattributes` file to reflect the removal of the `.travis.yml` file. * Updates the "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 3. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 4. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 5. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The toggle for running `composer install` with or without `--ignore-platform-reqs` has not been implemented as it is currently unnecessary. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Linting runs against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as succesfull, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
jrfnl
added a commit
to Yoast/whip
that referenced
this issue
Oct 2, 2021
This commit: * Adds a GH Actions workflow for the PHP lint and the PHPUnit CI checks. * Removes the, now redundant, `.travis.yml` configuration. * Updates the `.gitattributes` file to reflect the removal of the `.travis.yml` file. * Adds a "Build Status" badge to the Readme to use the results from this particular GH Actions run. Notes: 1. Builds will run on: - All pushes and all pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 3. While the `setup-php` action offers the ability to [install the PHAR files for Parallel Lint and PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `test.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 4. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 5. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * Linting and running of the tests will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Builds against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
jrfnl
added a commit
to Yoast/i18n-module
that referenced
this issue
Oct 2, 2021
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes the, now redundant, `.travis.yml` configuration. * Updates the `.gitattributes` file to reflect the removal of the `.travis.yml` file. * Updates the "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 3. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 4. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 5. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The toggle for running `composer install` with or without `--ignore-platform-reqs` has not been implemented as it is currently unnecessary. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Linting runs against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
jrfnl
added a commit
to Yoast/whip
that referenced
this issue
Oct 2, 2021
This commit: * Adds a GH Actions workflow for the PHP lint and the PHPUnit CI checks. * Removes the, now redundant, `.travis.yml` configuration. * Updates the `.gitattributes` file to reflect the removal of the `.travis.yml` file. * Adds a "Build Status" badge to the Readme to use the results from this particular GH Actions run. Notes: 1. Builds will run on: - All pushes and all pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 3. While the `setup-php` action offers the ability to [install the PHAR files for Parallel Lint and PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `test.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 4. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 5. The Composer install step for PHP 8.2 is special cased for now as PHPUnit 9.x does not allow for installation on PHP 8.2 yet. 6. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * Linting and running of the tests will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Builds against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Oct 9, 2021
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes all references to that action from the `.travis.yml` configuration. * Add a "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The tag-based branch filter has not been added as that filter was only intended for use with the `deploy` stage. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Linting runs against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successfull, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late. :point_right: **Open question**: Should the prefixing be run during this workflow so the prefixed files will be scanned for parse errors as well ?
jrfnl
added a commit
to Yoast/yoast-acf-analysis
that referenced
this issue
Oct 11, 2021
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes all references to that action from the `.travis.yml` configuration, as well as does some additional clean up of parts of the Travis script which have now become unused. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The toggle for running `composer install` with or without `--ignore-platform-reqs` has not been implemented as it is currently unnecessary. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Linting runs against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successfull, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
jrfnl
added a commit
to Yoast/yoast-acf-analysis
that referenced
this issue
Oct 11, 2021
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes all references to that action from the `.travis.yml` configuration, as well as does some additional clean up of parts of the Travis script which have now become unused. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The toggle for running `composer install` with or without `--ignore-platform-reqs` has not been implemented as it is currently unnecessary. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Linting runs against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successfull, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
jrfnl
added a commit
to Yoast/i18n-module
that referenced
this issue
Oct 11, 2021
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes the, now redundant, `.travis.yml` configuration. * Updates the `.gitattributes` file to reflect the removal of the `.travis.yml` file. * Updates the "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The toggle for running `composer install` with or without `--ignore-platform-reqs` has not been implemented as it is currently unnecessary. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Linting runs against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late. ADD: lint
jrfnl
added a commit
to Yoast/whip
that referenced
this issue
Oct 11, 2021
This commit: * Adds a GH Actions workflow for the PHP lint and the PHPUnit CI checks. * Removes the, now redundant, `.travis.yml` configuration. * Updates the `.gitattributes` file to reflect the removal of the `.travis.yml` file. * Adds a "Build Status" badge to the Readme to use the results from this particular GH Actions run. Notes: 1. Builds will run on: - All pushes and all pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR files for Parallel Lint and PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `test.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. The Composer install step for PHP 8.2 is special cased for now as PHPUnit 9.x does not allow for installation on PHP 8.2 yet. 7. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * Linting and running of the tests will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Builds against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
jrfnl
added a commit
to Yoast/yoast-test-helper
that referenced
this issue
Oct 11, 2021
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes all references to that action from the `.travis.yml` configuration, as well as does some additional clean up of parts of the Travis script which have now become unused. * Updates the "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * The tag-based branch filter has not been added as that filter was only intended for use with the `deploy` stage. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Linting runs against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
jrfnl
added a commit
to Emilia-Capital/comment-hacks
that referenced
this issue
Oct 11, 2021
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes all references to that action from the `.travis.yml` configuration, as well as does some additional clean up of parts of the Travis script which have now become unused. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly), neither of which has yet been released. Linting runs against either of these two PHP versions will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successfull, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 7, 2022
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes all references to that action from the `.travis.yml` configuration. * Add a "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs. * Adds two small tweaks to the `lint` script in the `composer.json`. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: this only needs to be done for the PHP version used for the actual linting. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow. The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory. The second time, it sets up the PHP version on which we want to do the actual linting. Note: within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. 5. As the `composer.lock` file for this plugin contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the `composer install` to work on all supported PHP versions. Previously in Travis, an uncontrolled `composer update` would be run to get round this. Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail. So, for this workflow, I've chosen to approach this slightly differently (compared to Travis) by doing a `no-dev` install + moving the linting tools to `no-dev`. That way, we avoid the uncontrolled update, can still install on all supported PHP versions and can run the lint on these without interference. 6. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 7. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 8. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The tag-based branch filter has not been added as that filter was only intended for use with the `deploy` stage. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly). Linting runs against PHP 8.2 will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late. Regarding the changes to the Composer `lint` scripts: * For the generic `lint` command, the `vendor_prefixed` directory is no longer excluded from the linting as the adjusted code does need to be cross-version compatible and safeguarding that PHP_Scoper doesn't introduce cross-version compatibility issues is a good thing. * The `--show-deprecated` option was added to PHP Parallel Lint some years ago, but never documented, so I never knew it existed. Enabling this option allows for deprecations which can be detected at compile time to be shown (in contrast to deprecations which can only be detected at run-time).
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 7, 2022
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes all references to that action from the `.travis.yml` configuration. * Add a "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs. * Adds two small tweaks to the `lint` script in the `composer.json`. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: this only needs to be done for the PHP version used for the actual linting. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow. The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory. The second time, it sets up the PHP version on which we want to do the actual linting. Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time) 5. As the `composer.lock` file for this plugin contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the linting to work (on all supported PHP versions). Previously in Travis, an uncontrolled `composer update` would be run to get round this. Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail. So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible. - First a full install is done on PHP 7.2. This will generate the `vendor_prefixed` directory, as well as set up all dependencies. - Next, the dependencies which are only needed for the prefixing are removed. This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of `autoload.files` in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code. - Now those incompatible dependencies are removed, we **_don't_** actually need to do a re-`install` of the dependencies on the target PHP version as all we'll be using is PHP-Parallel-Lint and PHP-Parallel-Lint is compatible with PHP 5.3 and higher, so the version installed on PHP 7.2 will work on all PHP versions on which we need to do the linting. It may be a bit counter-intuitive to do it like that, but it works and keeps the workflow as simple as possible, while still achieving the goal of the workflow. And we also avoid the uncontrolled update. 6. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 7. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 8. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The tag-based branch filter has not been added as that filter was only intended for use with the `deploy` stage. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly). Linting runs against PHP 8.2 will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late. Regarding the changes to the Composer `lint` scripts: * For the generic `lint` command, the `vendor_prefixed` directory is no longer excluded from the linting as the adjusted code **does** need to be cross-version compatible and safeguarding that PHP_Scoper doesn't introduce cross-version compatibility issues is a good thing.
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 7, 2022
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes all references to that action from the `.travis.yml` configuration. * Add a "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs. * Adds two small tweaks to the `lint` script in the `composer.json`. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: this only needs to be done for the PHP version used for the actual linting. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow. The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory. The second time, it sets up the PHP version on which we want to do the actual linting. Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time) 5. As the `composer.lock` file for this plugin contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the linting to work (on all supported PHP versions). Previously in Travis, an uncontrolled `composer update` would be run to get round this. Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail. So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible. - First a full install is done on PHP 7.2. This will generate the `vendor_prefixed` directory, as well as set up all dependencies. - Next, the dependencies which are only needed for the prefixing are removed. This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of `autoload.files` in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code. - Now those incompatible dependencies are removed, we **_don't_** actually need to do a re-`install` of the dependencies on the target PHP version as all we'll be using is PHP-Parallel-Lint and PHP-Parallel-Lint is compatible with PHP 5.3 and higher, so the version installed on PHP 7.2 will work on all PHP versions on which we need to do the linting. It may be a bit counter-intuitive to do it like that, but it works and keeps the workflow as simple as possible, while still achieving the goal of the workflow. And we also avoid the uncontrolled update. 6. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 7. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 8. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The tag-based branch filter has not been added as that filter was only intended for use with the `deploy` stage. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly). Linting runs against PHP 8.2 will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late. Regarding the changes to the Composer `lint` scripts: * For the generic `lint` command, the `vendor_prefixed` directory is no longer excluded from the linting as the adjusted code **does** need to be cross-version compatible and safeguarding that PHP_Scoper doesn't introduce cross-version compatibility issues is a good thing.
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 7, 2022
This commit: * Adds a GH Actions workflow for the PHP lint CI check. * Removes all references to that action from the `.travis.yml` configuration. * Add a "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs. * Adds two small tweaks to the `lint` script in the `composer.json`. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: this only needs to be done for the PHP version used for the actual linting. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow. The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory. The second time, it sets up the PHP version on which we want to do the actual linting. Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time) 5. As the `composer.lock` file for this plugin contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the linting to work (on all supported PHP versions). Previously in Travis, an uncontrolled `composer update` would be run to get round this. Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail. So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible. - First a full install is done on PHP 7.2. This will generate the `vendor_prefixed` directory, as well as set up all dependencies. - Next, the dependencies which are only needed for the prefixing are removed. This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of `autoload.files` in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code. - Now those incompatible dependencies are removed, we **_don't_** actually need to do a re-`install` of the dependencies on the target PHP version as all we'll be using is PHP-Parallel-Lint and PHP-Parallel-Lint is compatible with PHP 5.3 and higher, so the version installed on PHP 7.2 will work on all PHP versions on which we need to do the linting. It may be a bit counter-intuitive to do it like that, but it works and keeps the workflow as simple as possible, while still achieving the goal of the workflow. And we also avoid the uncontrolled update. 6. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file. IMO, that would make this a typical point of failure where updates would be done in one, but not the other. If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited. 7. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 8. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool. Differences with the Travis implementation: * There is a minor difference in the branch filtering being done for `push` events. Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though. * The tag-based branch filter has not been added as that filter was only intended for use with the `deploy` stage. * Linting will now also be executed against PHP 8.1 and 8.2 (nightly). Linting runs against PHP 8.2 will be "allowed to fail" for the time being. Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late. Regarding the changes to the Composer `lint` scripts: * For the generic `lint` command, the `vendor_prefixed` directory is no longer excluded from the linting as the adjusted code **does** need to be cross-version compatible and safeguarding that PHP_Scoper doesn't introduce cross-version compatibility issues is a good thing.
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 12, 2022
This commit: * Adds a GH Actions workflow for the PHPUnit CI check. * Removes all references to that check from the `.travis.yml` configuration. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as we need to do a `composer install` anyway to get the other dependencies, like WP Test Utils. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow. The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory. The second time, it sets up the PHP version on which we want to do the actual linting. Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time) 7. For these BrainMonkey based unit tests, we can (and should) use the most appropriate PHPUnit/BrainMonkey/Mockery/etc version for the PHP version the tests are being run on. Most notably, this is actually needed to get the tests running on PHP 8.0 and higher. As there is a committed `composer.lock` file and the PHPUnit version is locked at PHPUnit 5.x, this means we have to do an on-the-fly update of the PHPUnit version depending on the PHP version of the current build. As the `composer.lock` file for this plugin also contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the testing to work (on all supported PHP versions). Previously in Travis, an uncontrolled `composer update` would be run to get round this. Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail. So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible. - First a full install is done on PHP 7.2. This will generate the `vendor_prefixed` directory and create the generated files for the dependency injection, as well as set up all dependencies. - Next, the dependencies which are only needed for the prefixing are removed. This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of `autoload.files` in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code. - Lastly, we remove the `vendor` directory. - Now those incompatible dependencies are removed, we switch to the actual PHP version we want to use for the tests. - Next, we remove the `phpunit/phpunit` root requirement as it is already a dependency of the WP Test Utils and we don't want to abide by the locked version of PHPUnit. - We then run a selective `composer update` to install the locked dependencies and update just and only the test dependencies to match the PHP version on which we will be running the tests. Altogether, this prevents the uncontrolled dependency update. Additionally, the `--no-scripts` addition to the Composer run commands when run on the _real_ PHP version, will prevent the prefixing/dependency injection tasks from being run again on incompatible PHP versions. Note: no need for adding `--no-interaction` to the plain `composer ...` commands as the `setup-php` action already sets an environment variable to ensure Composer will always run with `--no-interaction` in these workflows. Differences with the Travis implementation: * The most appropriate version of the test dependencies will now be used for _all_ builds, not just for builds against PHP 8.0/`nightly`. * In Travis, there was a reference to a `atanamo/php-codeshift` package, which was removed after the prefixing was finished. This package is not a (root) dependency of the WPSEO Free plugin, so removing it doesn't actually have any effect. Instead, the `league/oauth2-client` package is removed, as that package should not be used during the test run as it will be prefixed and the prefixed version should be used instead. * Except for the test dependencies, no other dependencies will be updated during the test run. * In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1. As PHP 8.1 has been released, the (remaining) test failures on this build need to be fixed, though as these are only _deprecation notices_, the fact that these tests are currently failing is nothing to be too concerned about for the time being. To prevent new failures from being introduced in the mean time, I'm not allowing the build to fail, but have instead added expectations for the deprecations to the few tests which still have them.
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 12, 2022
This commit: * Adds a GH Actions workflow for the PHPUnit CI check. * Removes all references to that check from the `.travis.yml` configuration. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as we need to do a `composer install` anyway to get the other dependencies, like WP Test Utils. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow. The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory. The second time, it sets up the PHP version on which we want to do the actual linting. Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time) 7. For these BrainMonkey based unit tests, we can (and should) use the most appropriate PHPUnit/BrainMonkey/Mockery/etc version for the PHP version the tests are being run on. Most notably, this is actually needed to get the tests running on PHP 8.0 and higher. As there is a committed `composer.lock` file and the PHPUnit version is locked at PHPUnit 5.x, this means we have to do an on-the-fly update of the PHPUnit version depending on the PHP version of the current build. As the `composer.lock` file for this plugin also contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the testing to work (on all supported PHP versions). Previously in Travis, an uncontrolled `composer update` would be run to get round this. Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail. So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible. - First a full install is done on PHP 7.2. This will generate the `vendor_prefixed` directory and create the generated files for the dependency injection, as well as set up all dependencies. - Next, the dependencies which are only needed for the prefixing are removed. This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of `autoload.files` in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code. - Lastly, we remove the `vendor` directory. - Now those incompatible dependencies are removed, we switch to the actual PHP version we want to use for the tests. - Next, we remove the `phpunit/phpunit` root requirement as it is already a dependency of the WP Test Utils and we don't want to abide by the locked version of PHPUnit. - We then run a selective `composer update` to install the locked dependencies and update just and only the test dependencies to match the PHP version on which we will be running the tests. Altogether, this prevents the uncontrolled dependency update. Additionally, the `--no-scripts` addition to the Composer run commands when run on the _real_ PHP version, will prevent the prefixing/dependency injection tasks from being run again on incompatible PHP versions. Note: no need for adding `--no-interaction` to the plain `composer ...` commands as the `setup-php` action already sets an environment variable to ensure Composer will always run with `--no-interaction` in these workflows. Differences with the Travis implementation: * The most appropriate version of the test dependencies will now be used for _all_ builds, not just for builds against PHP 8.0/`nightly`. * In Travis, there was a reference to a `atanamo/php-codeshift` package, which was removed after the prefixing was finished. This package is not a (root) dependency of the WPSEO Free plugin, so removing it doesn't actually have any effect. Instead, the `league/oauth2-client` package is removed, as that package should not be used during the test run as it will be prefixed and the prefixed version should be used instead. * Except for the test dependencies, no other dependencies will be updated during the test run. * In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1. As PHP 8.1 has been released, the (remaining) test failures on this build need to be fixed, though as these are only _deprecation notices_, the fact that these tests are currently failing is nothing to be too concerned about for the time being. To prevent new failures from being introduced in the mean time, I'm not allowing the build to fail, but will instead add expectations for the deprecations or skip requirements to the few tests which still have them for the time being. REMOVE
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 14, 2022
This commit: * Adds a GH Actions workflow for the PHPUnit CI check. * Removes all references to that check from the `.travis.yml` configuration. * Adds a "Unit Tests" badge in the Readme using the results from the GH `Unit Test` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as we need to do a `composer install` anyway to get the other dependencies, like WP Test Utils. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow. The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory. The second time, it sets up the PHP version on which we want to do the actual linting. Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time) 7. For these BrainMonkey based unit tests, we can (and should) use the most appropriate PHPUnit/BrainMonkey/Mockery/etc version for the PHP version the tests are being run on. Most notably, this is actually needed to get the tests running on PHP 8.0 and higher. As there is a committed `composer.lock` file and the PHPUnit version is locked at PHPUnit 5.x, this means we have to do an on-the-fly update of the PHPUnit version depending on the PHP version of the current build. As the `composer.lock` file for this plugin also contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the testing to work (on all supported PHP versions). Previously in Travis, an uncontrolled `composer update` would be run to get round this. Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail. So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible. - First a full install is done on PHP 7.2. This will generate the `vendor_prefixed` directory and create the generated files for the dependency injection, as well as set up all dependencies. - Next, the dependencies which are only needed for the prefixing are removed. This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of `autoload.files` in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code. - Lastly, we remove the `vendor` directory. - Now those incompatible dependencies are removed, we switch to the actual PHP version we want to use for the tests. - Next, we remove the `phpunit/phpunit` root requirement as it is already a dependency of the WP Test Utils and we don't want to abide by the locked version of PHPUnit. - We then run a selective `composer update` to install the locked dependencies and update just and only the test dependencies to match the PHP version on which we will be running the tests. Altogether, this prevents the uncontrolled dependency update. Additionally, the `--no-scripts` addition to the Composer run commands when run on the _real_ PHP version, will prevent the prefixing/dependency injection tasks from being run again on incompatible PHP versions. Note: no need for adding `--no-interaction` to the plain `composer ...` commands as the `setup-php` action already sets an environment variable to ensure Composer will always run with `--no-interaction` in these workflows. Differences with the Travis implementation: * The most appropriate version of the test dependencies will now be used for _all_ builds, not just for builds against PHP 8.0/`nightly`. * In Travis, there was a reference to a `atanamo/php-codeshift` package, which was removed after the prefixing was finished. This package is not a (root) dependency of the WPSEO Free plugin, so removing it doesn't actually have any effect. Instead, the `league/oauth2-client` package is removed, as that package should not be used during the test run as it will be prefixed and the prefixed version should be used instead. * Except for the test dependencies, no other dependencies will be updated during the test run. * In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1. As PHP 8.1 has been released, the (remaining) test failures on this build need to be fixed, though as these are only _deprecation notices_, the fact that these tests are currently failing is nothing to be too concerned about for the time being. To prevent new failures from being introduced in the mean time, I'm not allowing the build to fail, but will instead add expectations for the deprecations or skip requirements to the few tests which still have them for the time being. REMOVE ADD TO: unit test
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 14, 2022
This commit: * Adds a GH Actions workflow for the PHPUnit CI check. * Removes all references to that check from the `.travis.yml` configuration. * Adds a "Unit Tests" badge in the Readme using the results from the GH `Unit Test` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as we need to do a `composer install` anyway to get the other dependencies, like WP Test Utils. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow. The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory. The second time, it sets up the PHP version on which we want to do the actual linting. Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time) 7. For these BrainMonkey based unit tests, we can (and should) use the most appropriate PHPUnit/BrainMonkey/Mockery/etc version for the PHP version the tests are being run on. Most notably, this is actually needed to get the tests running on PHP 8.0 and higher. As there is a committed `composer.lock` file and the PHPUnit version is locked at PHPUnit 5.x, this means we have to do an on-the-fly update of the PHPUnit version depending on the PHP version of the current build. As the `composer.lock` file for this plugin also contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the testing to work (on all supported PHP versions). Previously in Travis, an uncontrolled `composer update` would be run to get round this. Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail. So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible. - First a full install is done on PHP 7.2. This will generate the `vendor_prefixed` directory and create the generated files for the dependency injection, as well as set up all dependencies. - Next, the dependencies which are only needed for the prefixing are removed. This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of `autoload.files` in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code. - Lastly, we remove the `vendor` directory. - Now those incompatible dependencies are removed, we switch to the actual PHP version we want to use for the tests. - Next, we remove the `phpunit/phpunit` root requirement as it is already a dependency of the WP Test Utils and we don't want to abide by the locked version of PHPUnit. - We then run a selective `composer update` to install the locked dependencies and update just and only the test dependencies to match the PHP version on which we will be running the tests. Altogether, this prevents the uncontrolled dependency update. Additionally, the `--no-scripts` addition to the Composer run commands when run on the _real_ PHP version, will prevent the prefixing/dependency injection tasks from being run again on incompatible PHP versions. Note: no need for adding `--no-interaction` to the plain `composer ...` commands as the `setup-php` action already sets an environment variable to ensure Composer will always run with `--no-interaction` in these workflows. Differences with the Travis implementation: * The most appropriate version of the test dependencies will now be used for _all_ builds, not just for builds against PHP 8.0/`nightly`. * In Travis, there was a reference to a `atanamo/php-codeshift` package, which was removed after the prefixing was finished. This package is not a (root) dependency of the WPSEO Free plugin, so removing it doesn't actually have any effect. Instead, the `league/oauth2-client` package is removed, as that package should not be used during the test run as it will be prefixed and the prefixed version should be used instead. * Except for the test dependencies, no other dependencies will be updated during the test run. * In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1. As PHP 8.1 has been released, the (remaining) test failures on this build need to be fixed, though as these are only _deprecation notices_, the fact that these tests are currently failing is nothing to be too concerned about for the time being. To prevent new failures from being introduced in the mean time, I'm not allowing the build to fail, but will instead add expectations for the deprecations or skip requirements to the few tests which still have them for the time being. REMOVE ADD TO: unit test
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 14, 2022
This commit: * Adds a GH Actions workflow for the PHPUnit CI check. * Removes all references to that check from the `.travis.yml` configuration. * Adds a "Unit Tests" badge in the Readme using the results from the GH `Unit Test` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as we need to do a `composer install` anyway to get the other dependencies, like WP Test Utils. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow. The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory. The second time, it sets up the PHP version on which we want to do the actual linting. Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time) 7. For these BrainMonkey based unit tests, we can (and should) use the most appropriate PHPUnit/BrainMonkey/Mockery/etc version for the PHP version the tests are being run on. Most notably, this is actually needed to get the tests running on PHP 8.0 and higher. As there is a committed `composer.lock` file and the PHPUnit version is locked at PHPUnit 5.x, this means we have to do an on-the-fly update of the PHPUnit version depending on the PHP version of the current build. As the `composer.lock` file for this plugin also contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the testing to work (on all supported PHP versions). Previously in Travis, an uncontrolled `composer update` would be run to get round this. Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail. So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible. - First a full install is done on PHP 7.2. This will generate the `vendor_prefixed` directory and create the generated files for the dependency injection, as well as set up all dependencies. - Next, the dependencies which are only needed for the prefixing are removed. This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of `autoload.files` in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code. - Lastly, we remove the `vendor` directory. - Now those incompatible dependencies are removed, we switch to the actual PHP version we want to use for the tests. - Next, we remove the `phpunit/phpunit` root requirement as it is already a dependency of the WP Test Utils and we don't want to abide by the locked version of PHPUnit. - We then run a selective `composer update` to install the locked dependencies and update just and only the test dependencies to match the PHP version on which we will be running the tests. Altogether, this prevents the uncontrolled dependency update. Additionally, the `--no-scripts` addition to the Composer run commands when run on the _real_ PHP version, will prevent the prefixing/dependency injection tasks from being run again on incompatible PHP versions. Note: no need for adding `--no-interaction` to the plain `composer ...` commands as the `setup-php` action already sets an environment variable to ensure Composer will always run with `--no-interaction` in these workflows. Differences with the Travis implementation: * The most appropriate version of the test dependencies will now be used for _all_ builds, not just for builds against PHP 8.0/`nightly`. * In Travis, there was a reference to a `atanamo/php-codeshift` package, which was removed after the prefixing was finished. This package is not a (root) dependency of the WPSEO Free plugin, so removing it doesn't actually have any effect. Instead, the `league/oauth2-client` package is removed, as that package should not be used during the test run as it will be prefixed and the prefixed version should be used instead. * Except for the test dependencies, no other dependencies will be updated during the test run. * In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1. As PHP 8.1 has been released, the (remaining) test failures on this build need to be fixed, though as these are only _deprecation notices_, the fact that these tests are currently failing is nothing to be too concerned about for the time being. To prevent new failures from being introduced in the mean time, I'm not allowing the build to fail, but will instead add expectations for the deprecations or skip requirements to the few tests which still have them for the time being. REMOVE ADD TO: unit test
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 14, 2022
This commit: * Adds a GH Actions workflow for the PHPUnit CI check. * Removes all references to that check from the `.travis.yml` configuration. * Adds a "Unit Tests" badge in the Readme using the results from the GH `Unit Test` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as we need to do a `composer install` anyway to get the other dependencies, like WP Test Utils. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow. The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory. The second time, it sets up the PHP version on which we want to do the actual linting. Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time) 7. For these BrainMonkey based unit tests, we can (and should) use the most appropriate PHPUnit/BrainMonkey/Mockery/etc version for the PHP version the tests are being run on. Most notably, this is actually needed to get the tests running on PHP 8.0 and higher. As there is a committed `composer.lock` file and the PHPUnit version is locked at PHPUnit 5.x, this means we have to do an on-the-fly update of the PHPUnit version depending on the PHP version of the current build. As the `composer.lock` file for this plugin also contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the testing to work (on all supported PHP versions). Previously in Travis, an uncontrolled `composer update` would be run to get round this. Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail. So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible. - First a full install is done on PHP 7.2. This will generate the `vendor_prefixed` directory and create the generated files for the dependency injection, as well as set up all dependencies. - Next, the dependencies which are only needed for the prefixing are removed. This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of `autoload.files` in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code. - Lastly, we remove the `vendor` directory. - Now those incompatible dependencies are removed, we switch to the actual PHP version we want to use for the tests. - Next, we remove the `phpunit/phpunit` root requirement as it is already a dependency of the WP Test Utils and we don't want to abide by the locked version of PHPUnit. - We then run a selective `composer update` to install the locked dependencies and update just and only the test dependencies to match the PHP version on which we will be running the tests. Altogether, this prevents the uncontrolled dependency update. Additionally, the `--no-scripts` addition to the Composer run commands when run on the _real_ PHP version, will prevent the prefixing/dependency injection tasks from being run again on incompatible PHP versions. Note: no need for adding `--no-interaction` to the plain `composer ...` commands as the `setup-php` action already sets an environment variable to ensure Composer will always run with `--no-interaction` in these workflows. Differences with the Travis implementation: * The most appropriate version of the test dependencies will now be used for _all_ builds, not just for builds against PHP 8.0/`nightly`. * In Travis, there was a reference to a `atanamo/php-codeshift` package, which was removed after the prefixing was finished. This package is not a (root) dependency of the WPSEO Free plugin, so removing it doesn't actually have any effect. Instead, the `league/oauth2-client` package is removed, as that package should not be used during the test run as it will be prefixed and the prefixed version should be used instead. * Except for the test dependencies, no other dependencies will be updated during the test run. * In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1. As PHP 8.1 has been released, the (remaining) test failures on this build need to be fixed, though as these are only _deprecation notices_, the fact that these tests are currently failing is nothing to be too concerned about for the time being. To prevent new failures from being introduced in the mean time, I'm not allowing the build to fail, but will instead add expectations for the deprecations or skip requirements to the few tests which still have them for the time being.
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 15, 2022
This commit: * Adds a GH Actions workflow for the PHPUnit CI check. * Removes all references to that check from the `.travis.yml` configuration. * Adds a "Unit Tests" badge in the Readme using the results from the GH `Unit Test` Action runs. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 4. While the `setup-php` action offers the ability to [install the PHAR file for PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as we need to do a `composer install` anyway to get the other dependencies, like WP Test Utils. 5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 6. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow. The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory. The second time, it sets up the PHP version on which we want to do the actual linting. Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time) 7. For these BrainMonkey based unit tests, we can (and should) use the most appropriate PHPUnit/BrainMonkey/Mockery/etc version for the PHP version the tests are being run on. Most notably, this is actually needed to get the tests running on PHP 8.0 and higher. As there is a committed `composer.lock` file and the PHPUnit version is locked at PHPUnit 5.x, this means we have to do an on-the-fly update of the PHPUnit version depending on the PHP version of the current build. As the `composer.lock` file for this plugin also contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the testing to work (on all supported PHP versions). Previously in Travis, an uncontrolled `composer update` would be run to get round this. Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail. So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible. - First a full install is done on PHP 7.2. This will generate the `vendor_prefixed` directory and create the generated files for the dependency injection, as well as set up all dependencies. - Next, the dependencies which are only needed for the prefixing are removed. This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of `autoload.files` in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code. - Lastly, we remove the `vendor` directory. - Now those incompatible dependencies are removed, we switch to the actual PHP version we want to use for the tests. - Next, we remove the `phpunit/phpunit` root requirement as it is already a dependency of the WP Test Utils and we don't want to abide by the locked version of PHPUnit. - We then run a selective `composer update` to install the locked dependencies and update just and only the test dependencies to match the PHP version on which we will be running the tests. Altogether, this prevents the uncontrolled dependency update. Additionally, the `--no-scripts` addition to the Composer run commands when run on the _real_ PHP version, will prevent the prefixing/dependency injection tasks from being run again on incompatible PHP versions. Note: no need for adding `--no-interaction` to the plain `composer ...` commands as the `setup-php` action already sets an environment variable to ensure Composer will always run with `--no-interaction` in these workflows. Differences with the Travis implementation: * The most appropriate version of the test dependencies will now be used for _all_ builds, not just for builds against PHP 8.0/`nightly`. * In Travis, there was a reference to a `atanamo/php-codeshift` package, which was removed after the prefixing was finished. This package is not a (root) dependency of the WPSEO Free plugin, so removing it doesn't actually have any effect. Instead, the `league/oauth2-client` package is removed, as that package should not be used during the test run as it will be prefixed and the prefixed version should be used instead. * Except for the test dependencies, no other dependencies will be updated during the test run. * In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1. As PHP 8.1 has been released, the (remaining) test failures on this build need to be fixed, though as these are only _deprecation notices_, the fact that these tests are currently failing is nothing to be too concerned about for the time being. To prevent new failures from being introduced in the mean time, I'm not allowing the build to fail, but will instead add expectations for the deprecations or skip requirements to the few tests which still have them for the time being.
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 17, 2022
This commit: * Adds a GH Actions workflow for the PHPUnit Integration test CI check. * Removes all references to that check from the `.travis.yml` configuration. * Replaces the "Build Status" badge in the Readme with a badge using the results from the GH `IntegrationTest` Action runs. All other workflows have their own status badges, so the Travis badge can now be safely removed. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. In contrast to various other scripts, the matrix for these integration tests is explicitly set with all variables explicitly defined for each build, as we want full control over the three variables (`php_version`, `wp_version` and `multisite`) used for each run. 4. As a MySQL database is needed, we need to explicitly add that service. The MySQL version used varies based on the PHP version against which the tests are run for two reasons: 1. It's good practice to test against multiple different MySQL versions to verify that any queries run are MySQL cross-version compatible. 2. While WP is _supposed to_ support MySQL 8.0 since WP 5.4, in actual fact, WordPress doesn't fully support MySQL 8.0 on PHP < 7.4. Also see: https://core.trac.wordpress.org/ticket/52496 Note: in the Travis script, the PHP 5.6 build was explicitly set to use the `trusty` dist to force the use of MySQL 5.6. In GH Actions, this is not directly linked to the Ubuntu image used. All the same, the script has been set up to explicitly use MySQL 5.6 for PHP 5.6, same as before. 5. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 6. While the `setup-php` action offers the ability to [install the PHAR file for PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as we need to do a `composer install` anyway to get the other dependencies, like WP Test Utils. 7. The PHPUnit version(s) supported and needed to run the tests depend on **both** the WP version used as well as the PHP version used. As an `if` condition on a step would get pretty complicated to handle this, I've introduced a separate - fully documented - step to determine the "type" of install we need. The output of that step is subsequently used in the `if` condition for follow-on steps to make sure the correct PHPUnit version will be installed by Composer. 8. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 9. As there is a committed `composer.lock` file and the PHPUnit version is locked at PHPUnit 5.x, we have to do an on-the-fly update of the PHPUnit version depending on the WP and PHP version used for the current build. As we still also want to benefit from the Composer caching which the `ramsey/composer-install` action sets up, I've done some nifty trickery with the options passed to the `composer-install` action to get that working. - The `dependency-versions: "highest"` basically changes the command used by the action from `composer install` to `composer update`, however we don't want to update _all_ dependencies as run-time dependencies should remain locked at their original version. - To that end, I'm passing additional `composer-options` which will limit the update to just and only the test dependencies. - These type of updates are only run when needed based on the "install type" determination as described in (7). 10. Installing WordPress was previously done via inline commands in the Travis script. This has now been replaced by the more comprehensive `tests/bin/install-wp-tests.sh` script as made available by the WP CLI `scaffold` command. As WPCLI is not used anywhere else in the workflow, a copy of the script, with source annotations, is being introduced into the plugin codebase. Note: this script needs to have the `x` flag set to allow it to be executed. Ref: https://github.com/wp-cli/scaffold-command/blob/master/templates/install-wp-tests.sh Some differences between the scripts: - The plugin will no longer be copied into the WordPress install, as, to be honest, that's just not necessary with the current test setup/test bootstrap. - The location to which WP is downloaded and installed is different between the scripts, however, this makes no difference for our purposes as the WP Test Utils library already takes the default install locations from the `install-wp-tests.sh` script into account. - To test against the "nightly" of WordPress, the matrix needs to set the `wp_version` to `trunk` or `nightly` instead of `master` (as was previously used in some scripts). - WordPress will be set up to always use `mysqli` for database commands, even on PHP 5.6. Differences with the Travis implementation: * The most appropriate version of the test dependencies will now be used for _all_ builds, not just for builds against PHP 8.0/`nightly`. * Except for the test dependencies, no other dependencies will be updated during the test run. * In Travis, there was a reference to a `atanamo/php-codeshift` package, which was removed after the prefixing was finished. This package is not a (root) dependency of the WPSEO Free plugin, so removing it doesn't actually have any effect. Instead, the `league/oauth2-client` package is removed, as that package should not be used during the test run as it will be prefixed and the prefixed version should be used instead. * The "mix" of PHP/WP versions + Multisite used in the matrix has been adjusted slightly compared to Travis. The current mix in GHA ensure there is a run against high + low WP on low PHP, a run against high + low WP on high PHP and a run against mulisite for all supported WP versions. * In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1. As PHP 8.1 has been released, the (remaining) test failures on this build need to be fixed, though as these are only _deprecation notices_, the fact that these tests are currently failing is nothing to be too concerned about for the time being. To prevent new failures from being introduced in the mean time, I'm not allowing the build to fail, but will instead add expectations for the deprecations or skip requirements to the few tests which still have them for the time being. Builds against WP `trunk`, will be "allowed to fail". Note: if a "allowed to fail" job actually fails, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late. * Instead of using Git to clone WP, the WP-CLI script is now used to retrieve the relevant parts of WP and initiate the database and the `wp-config` settings. * There are a number of files which are usually created by Grunt and are needed during the test run. In Travis, these files were created on the fly from within the Travis script to ensure tests would not error out on these files missing.. As this was only done in Travis, PHP focussed plugin developers who rarely use Grunt locally, would still end up with these errors when they would run the tests locally. As the need for these files is inherent to the integration tests, creating these files from within the integration test bootstrap file will make this more stable and will allow this to work both in CI as for dev-local test runs. * In Travis, test runs would run either as single site or as multisite. In GH Actions, each build will always run the tests against WP in single site mode and for those builds with the `multisite` flag set to `true` in the matrix, the test will **_also_** be run in multisite mode. * In Travis, for select test runs, code coverage would be calculated and uploaded to CodeClimate. As CodeClimate did not signal back to PRs if a PR changed the code coverage and by how much, this was not very effective, other than to keep track over time, though in practice, nobody seemed to even check on this or check on the reports at all. With this in mind, code coverage will no longer be run, nor uploaded to CodeClimate. The intention is to eventually run code coverage on the Jenkins server.
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 17, 2022
This commit: * Adds a GH Actions workflow for the PHPUnit Integration test CI check. * Removes all references to that check from the `.travis.yml` configuration. * Replaces the "Build Status" badge in the Readme with a badge using the results from the GH `IntegrationTest` Action runs. All other workflows have their own status badges, so the Travis badge can now be safely removed. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. In contrast to various other scripts, the matrix for these integration tests is explicitly set with all variables explicitly defined for each build, as we want full control over the three variables (`php_version`, `wp_version` and `multisite`) used for each run. 4. As a MySQL database is needed, we need to explicitly add that service. The MySQL version used varies based on the PHP version against which the tests are run for two reasons: 1. It's good practice to test against multiple different MySQL versions to verify that any queries run are MySQL cross-version compatible. 2. While WP is _supposed to_ support MySQL 8.0 since WP 5.4, in actual fact, WordPress doesn't fully support MySQL 8.0 on PHP < 7.4. Also see: https://core.trac.wordpress.org/ticket/52496 Note: in the Travis script, the PHP 5.6 build was explicitly set to use the `trusty` dist to force the use of MySQL 5.6. In GH Actions, this is not directly linked to the Ubuntu image used. All the same, the script has been set up to explicitly use MySQL 5.6 for PHP 5.6, same as before. 5. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 6. While the `setup-php` action offers the ability to [install the PHAR file for PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as we need to do a `composer install` anyway to get the other dependencies, like WP Test Utils. 7. The PHPUnit version(s) supported and needed to run the tests depend on **both** the WP version used as well as the PHP version used. As an `if` condition on a step would get pretty complicated to handle this, I've introduced a separate - fully documented - step to determine the "type" of install we need. The output of that step is subsequently used in the `if` condition for follow-on steps to make sure the correct PHPUnit version will be installed by Composer. 8. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 9. As there is a committed `composer.lock` file and the PHPUnit version is locked at PHPUnit 5.x, we have to do an on-the-fly update of the PHPUnit version depending on the WP and PHP version used for the current build. As we still also want to benefit from the Composer caching which the `ramsey/composer-install` action sets up, I've done some nifty trickery with the options passed to the `composer-install` action to get that working. - The `dependency-versions: "highest"` basically changes the command used by the action from `composer install` to `composer update`, however we don't want to update _all_ dependencies as run-time dependencies should remain locked at their original version. - To that end, I'm passing additional `composer-options` which will limit the update to just and only the test dependencies. - These type of updates are only run when needed based on the "install type" determination as described in (7). 10. Installing WordPress was previously done via inline commands in the Travis script. This has now been replaced by the more comprehensive `tests/bin/install-wp-tests.sh` script as made available by the WP CLI `scaffold` command. As WPCLI is not used anywhere else in the workflow, a copy of the script, with source annotations, is being introduced into the plugin codebase. Note: this script needs to have the `x` flag set to allow it to be executed. Ref: https://github.com/wp-cli/scaffold-command/blob/master/templates/install-wp-tests.sh Some differences between the scripts: - The plugin will no longer be copied into the WordPress install, as, to be honest, that's just not necessary with the current test setup/test bootstrap. - The location to which WP is downloaded and installed is different between the scripts, however, this makes no difference for our purposes as the WP Test Utils library already takes the default install locations from the `install-wp-tests.sh` script into account. - To test against the "nightly" of WordPress, the matrix needs to set the `wp_version` to `trunk` or `nightly` instead of `master` (as was previously used in some scripts). - WordPress will be set up to always use `mysqli` for database commands, even on PHP 5.6. Differences with the Travis implementation: * The most appropriate version of the test dependencies will now be used for _all_ builds, not just for builds against PHP 8.0/`nightly`. * Except for the test dependencies, no other dependencies will be updated during the test run. * In Travis, there was a reference to a `atanamo/php-codeshift` package, which was removed after the prefixing was finished. This package is not a (root) dependency of the WPSEO Free plugin, so removing it doesn't actually have any effect. Instead, the `league/oauth2-client` package is removed, as that package should not be used during the test run as it will be prefixed and the prefixed version should be used instead. * The "mix" of PHP/WP versions + Multisite used in the matrix has been adjusted slightly compared to Travis. The current mix in GHA ensure there is a run against high + low WP on low PHP, a run against high + low WP on high PHP and a run against mulisite for all supported WP versions. * In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1. As PHP 8.1 has been released, the (remaining) test failures on this build need to be fixed, though as these are only _deprecation notices_, the fact that these tests are currently failing is nothing to be too concerned about for the time being. To prevent new failures from being introduced in the mean time, I'm not allowing the build to fail, but will instead add expectations for the deprecations or skip requirements to the few tests which still have them for the time being. Builds against WP `trunk`, will be "allowed to fail". Note: if a "allowed to fail" job actually fails, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late. * Instead of using Git to clone WP, the WP-CLI script is now used to retrieve the relevant parts of WP and initiate the database and the `wp-config` settings. * There are a number of files which are usually created by Grunt and are needed during the test run. In Travis, these files were created on the fly from within the Travis script to ensure tests would not error out on these files missing.. As this was only done in Travis, PHP focussed plugin developers who rarely use Grunt locally, would still end up with these errors when they would run the tests locally. As the need for these files is inherent to the integration tests, creating these files from within the integration test bootstrap file will make this more stable and will allow this to work both in CI as for dev-local test runs. * In Travis, test runs would run either as single site or as multisite. In GH Actions, each build will always run the tests against WP in single site mode and for those builds with the `multisite` flag set to `true` in the matrix, the test will **_also_** be run in multisite mode. * In Travis, for select test runs, code coverage would be calculated and uploaded to CodeClimate. As CodeClimate did not signal back to PRs if a PR changed the code coverage and by how much, this was not very effective, other than to keep track over time, though in practice, nobody seemed to even check on this or check on the reports at all. With this in mind, code coverage will no longer be run, nor uploaded to CodeClimate. The intention is to eventually run code coverage on the Jenkins server.
jrfnl
added a commit
to Yoast/wordpress-seo
that referenced
this issue
Mar 17, 2022
This commit: * Adds a GH Actions workflow for the PHPUnit Integration test CI check. * Removes all references to that check from the `.travis.yml` configuration. * Replaces the "Build Status" badge in the Readme with a badge using the results from the GH `IntegrationTest` Action runs. All other workflows have their own status badges, so the Travis badge can now be safely removed. Notes: 1. Builds will run on: - Select pushes using branch filtering similar to before. - All pull requests. - When manually triggered. Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default. 2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled. In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour. 3. In contrast to various other scripts, the matrix for these integration tests is explicitly set with all variables explicitly defined for each build, as we want full control over the three variables (`php_version`, `wp_version` and `multisite`) used for each run. 4. As a MySQL database is needed, we need to explicitly add that service. The MySQL version used varies based on the PHP version against which the tests are run for two reasons: 1. It's good practice to test against multiple different MySQL versions to verify that any queries run are MySQL cross-version compatible. 2. While WP is _supposed to_ support MySQL 8.0 since WP 5.4, in actual fact, WordPress doesn't fully support MySQL 8.0 on PHP < 7.4. Also see: https://core.trac.wordpress.org/ticket/52496 Note: in the Travis script, the PHP 5.6 build was explicitly set to use the `trusty` dist to force the use of MySQL 5.6. In GH Actions, this is not directly linked to the Ubuntu image used. All the same, the script has been set up to explicitly use MySQL 5.6 for PHP 5.6, same as before. 5. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration. This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile). For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown. Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off. Refs: * shivammathur/setup-php#450 * shivammathur/setup-php#469 6. While the `setup-php` action offers the ability to [install the PHAR file for PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as we need to do a `composer install` anyway to get the other dependencies, like WP Test Utils. 7. The PHPUnit version(s) supported and needed to run the tests depend on **both** the WP version used as well as the PHP version used. As an `if` condition on a step would get pretty complicated to handle this, I've introduced a separate - fully documented - step to determine the "type" of install we need. The output of that step is subsequently used in the `if` condition for follow-on steps to make sure the correct PHPUnit version will be installed by Composer. 8. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose. The alternative would be to handle the caching manually, which would add three extra steps to the script. Note: Caching works differently between Travis and GH Actions. On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key. As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact. Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows 9. As there is a committed `composer.lock` file and the PHPUnit version is locked at PHPUnit 5.x, we have to do an on-the-fly update of the PHPUnit version depending on the WP and PHP version used for the current build. As we still also want to benefit from the Composer caching which the `ramsey/composer-install` action sets up, I've done some nifty trickery with the options passed to the `composer-install` action to get that working. - The `dependency-versions: "highest"` basically changes the command used by the action from `composer install` to `composer update`, however we don't want to update _all_ dependencies as run-time dependencies should remain locked at their original version. - To that end, I'm passing additional `composer-options` which will limit the update to just and only the test dependencies. - These type of updates are only run when needed based on the "install type" determination as described in (7). 10. Installing WordPress was previously done via inline commands in the Travis script. This has now been replaced by the more comprehensive `tests/bin/install-wp-tests.sh` script as made available by the WP CLI `scaffold` command. As WPCLI is not used anywhere else in the workflow, a copy of the script, with source annotations, is being introduced into the plugin codebase. Note: this script needs to have the `x` flag set to allow it to be executed. Ref: https://github.com/wp-cli/scaffold-command/blob/master/templates/install-wp-tests.sh Some differences between the scripts: - The plugin will no longer be copied into the WordPress install, as, to be honest, that's just not necessary with the current test setup/test bootstrap. - The location to which WP is downloaded and installed is different between the scripts, however, this makes no difference for our purposes as the WP Test Utils library already takes the default install locations from the `install-wp-tests.sh` script into account. - To test against the "nightly" of WordPress, the matrix needs to set the `wp_version` to `trunk` or `nightly` instead of `master` (as was previously used in some scripts). - WordPress will be set up to always use `mysqli` for database commands, even on PHP 5.6. Differences with the Travis implementation: * The most appropriate version of the test dependencies will now be used for _all_ builds, not just for builds against PHP 8.0/`nightly`. * Except for the test dependencies, no other dependencies will be updated during the test run. * In Travis, there was a reference to a `atanamo/php-codeshift` package, which was removed after the prefixing was finished. This package is not a (root) dependency of the WPSEO Free plugin, so removing it doesn't actually have any effect. Instead, the `league/oauth2-client` package is removed, as that package should not be used during the test run as it will be prefixed and the prefixed version should be used instead. * The "mix" of PHP/WP versions + Multisite used in the matrix has been adjusted slightly compared to Travis. The current mix in GHA ensure there is a run against high + low WP on low PHP, a run against high + low WP on high PHP and a run against mulisite for all supported WP versions. * In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1. As PHP 8.1 has been released, the (remaining) test failures on this build need to be fixed, though as these are only _deprecation notices_, the fact that these tests are currently failing is nothing to be too concerned about for the time being. To prevent new failures from being introduced in the mean time, I'm not allowing the build to fail, but will instead add expectations for the deprecations or skip requirements to the few tests which still have them for the time being. Builds against WP `trunk`, will be "allowed to fail". Note: if a "allowed to fail" job actually fails, the workflow will show as successful, but there will still be a red `x` next to a PR. This is a known issue in GHA: https://github.com/actions/toolkit/issues/399 There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late. * Instead of using Git to clone WP, the WP-CLI script is now used to retrieve the relevant parts of WP and initiate the database and the `wp-config` settings. * There are a number of files which are usually created by Grunt and are needed during the test run. In Travis, these files were created on the fly from within the Travis script to ensure tests would not error out on these files missing.. As this was only done in Travis, PHP focussed plugin developers who rarely use Grunt locally, would still end up with these errors when they would run the tests locally. As the need for these files is inherent to the integration tests, creating these files from within the integration test bootstrap file will make this more stable and will allow this to work both in CI as for dev-local test runs. * In Travis, test runs would run either as single site or as multisite. In GH Actions, each build will always run the tests against WP in single site mode and for those builds with the `multisite` flag set to `true` in the matrix, the test will **_also_** be run in multisite mode. * In Travis, for select test runs, code coverage would be calculated and uploaded to CodeClimate. As CodeClimate did not signal back to PRs if a PR changed the code coverage and by how much, this was not very effective, other than to keep track over time, though in practice, nobody seemed to even check on this or check on the reports at all. With this in mind, code coverage will no longer be run, nor uploaded to CodeClimate. The intention is to eventually run code coverage on the Jenkins server.
This was referenced Oct 22, 2022
stronk7
added a commit
to stronk7/moodle-cs
that referenced
this issue
Jan 12, 2023
As far as unit tests don't use the command line tools (phpcs, phpcbf) they can be passing ok while the tools have some other problem (say php X.Y compatibility or whatever). These tests just ensure that the command line tools are executed once, against a simple fixture file and results are the expected ones (we have used very simple shell exit codes checks, that's enough for now). Also note that, iby default, the php action comes with production php.ini settings, so error levels and output are not the best for CIs. Hence, let's enable them here. Reference: shivammathur/setup-php#450
stronk7
added a commit
to stronk7/moodle-cs
that referenced
this issue
Jan 12, 2023
As far as unit tests don't use the command line tools (phpcs, phpcbf) they can be passing ok while the tools have some other problem (say php X.Y compatibility or whatever). These tests just ensure that the command line tools are executed once, against a simple fixture file and results are the expected ones (we have used very simple shell exit codes checks, that's enough for now). Also note that, iby default, the php action comes with production php.ini settings, so error levels and output are not the best for CIs. Hence, let's enable them here. Reference: shivammathur/setup-php#450
stronk7
added a commit
to stronk7/moodle-cs
that referenced
this issue
Jan 12, 2023
As far as unit tests don't use the command line tools (phpcs, phpcbf) they can be passing ok while the tools have some other problem (say php X.Y compatibility or whatever). These tests just ensure that the command line tools are executed once, against a simple fixture file and results are the expected ones (we have used very simple shell exit codes checks, that's enough for now). Also note that, iby default, the php action comes with production php.ini settings, so error levels and output are not the best for CIs. Hence, let's enable them here. Reference: shivammathur/setup-php#450
stronk7
added a commit
to stronk7/moodle-cs
that referenced
this issue
Jan 12, 2023
As far as unit tests don't use the command line tools (phpcs, phpcbf) they can be passing ok while the tools have some other problem (say php X.Y compatibility or whatever). These tests just ensure that the command line tools are executed once, against a simple fixture file and results are the expected ones (we have used very simple shell exit codes checks, that's enough for now). Also note that, iby default, the php action comes with production php.ini settings, so error levels and output are not the best for CIs. Hence, let's enable them here. Reference: shivammathur/setup-php#450
stronk7
added a commit
to stronk7/moodle-cs
that referenced
this issue
Jan 12, 2023
As far as unit tests don't use the command line tools (phpcs, phpcbf) they can be passing ok while the tools have some other problem (say php X.Y compatibility or whatever). These tests just ensure that the command line tools are executed once, against a simple fixture file and results are the expected ones (we have used very simple shell exit codes checks, that's enough for now). Also note that, iby default, the php action comes with production php.ini settings, so error levels and output are not the best for CIs. Hence, let's enable them here. Reference: shivammathur/setup-php#450
stronk7
added a commit
to stronk7/moodle-cs
that referenced
this issue
Jan 12, 2023
As far as unit tests don't use the command line tools (phpcs, phpcbf) they can be passing ok while the tools have some other problem (say php X.Y compatibility or whatever). These tests just ensure that the command line tools are executed once, against a simple fixture file and results are the expected ones (we have used very simple shell exit codes checks, that's enough for now). Also note that, iby default, the php action comes with production php.ini settings, so error levels and output are not the best for CIs. Hence, let's enable them here. Reference: shivammathur/setup-php#450
stronk7
added a commit
to stronk7/moodle-cs
that referenced
this issue
Jan 12, 2023
As far as unit tests don't use the command line tools (phpcs, phpcbf) they can be passing ok while the tools have some other problem (say php X.Y compatibility or whatever). These tests just ensure that the command line tools are executed once, against a simple fixture file and results are the expected ones (we have used very simple shell exit codes checks, that's enough for now). Also note that, iby default, the php action comes with production php.ini settings, so error levels and output are not the best for CIs. Hence, let's enable them here. Reference: shivammathur/setup-php#450
stronk7
added a commit
to stronk7/moodle-cs
that referenced
this issue
Jan 12, 2023
As far as unit tests don't use the command line tools (phpcs, phpcbf) they can be passing ok while the tools have some other problem (say php X.Y compatibility or whatever). These tests just ensure that the command line tools are executed once, against a simple fixture file and results are the expected ones (we have used very simple shell exit codes checks, that's enough for now). Also note that, iby default, the php action comes with production php.ini settings, so error levels and output are not the best for CIs. Hence, let's enable them here. Reference: shivammathur/setup-php#450
AnaelMobilia
added a commit
to AnaelMobilia/image-heberg.fr
that referenced
this issue
Dec 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi, currently this GA installs PHP with its default
php.ini
, which is for Linux the one taken from Ondrej repositories (https://deb.sury.org/) as far as I can tell.Those packages install PHP selecting the
php.ini-production
config fromphp-src
, which sets for exampleerror_reporting
toE_ALL & ~E_DEPRECATED & ~E_STRICT
instead ofE_ALL
.But as a Continuous Integration services, it would be preferable to have the
php.ini-development
one.I in fact noticed this because a test of mine was failing locally but not on GA, which rang a huge bell into my mind.
Here's the full diff for PHP 8.0 between the two. You can see that all the differences are worthy for a CI environment:
The text was updated successfully, but these errors were encountered: