Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#50 PHP-Scoper がテスト可能に #51

Merged
merged 14 commits into from
Jul 20, 2023
28 changes: 28 additions & 0 deletions .github/actions/php-scoper/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: Setup PHP Scoper
description: Setup PHP Scoper
inputs:
Comment on lines +1 to +3

Choose a reason for hiding this comment

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

🤖 OpenAI

The name and description fields in the GitHub Actions workflow file should be more descriptive. Consider updating them to provide clearer information about the purpose of the workflow.

scoper:
description: Whether to do PHP Scoper
required: true
Comment on lines +1 to +6
Copy link

@github-actions github-actions bot Jul 19, 2023

Choose a reason for hiding this comment

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

🤖 OpenAI

The scoper input is marked as required, but it seems that the condition for executing the PHP Scoper steps is based on the value 'yes' instead of 'true'. This could lead to confusion and potential issues. It would be better to align the condition with the input description and make it consistent. Consider changing line 6 to:

required: true
default: 'no'

And changing lines 12 and 21 to:

if: ${{ inputs.scoper == 'yes' }}

This ensures that the condition matches the input value and avoids any ambiguity.


Comment on lines +1 to +7

Choose a reason for hiding this comment

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

🤖 OpenAI

The scoper input parameter should have a default value of false instead of being required. This allows the workflow to run without explicitly specifying the scoper input. Additionally, the description for the scoper input should be updated to indicate that it controls whether PHP Scoper is enabled or not.

3: inputs:
4:   scoper:
5:     description: Whether to enable PHP Scoper
6:     required: false
7:     default: false

runs:
using: "composite"
Comment on lines +8 to +9

Choose a reason for hiding this comment

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

🤖 OpenAI

The runs section of the workflow is using the composite strategy, which allows running multiple steps. However, there is only one step defined in this workflow. If there are no plans to add additional steps in the future, you can remove the using: "composite" line.

Comment on lines +1 to +9

Choose a reason for hiding this comment

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

🤖 OpenAI

The new hunk introduces a GitHub Actions workflow file for setting up PHP Scoper. It includes inputs for the scoper parameter and defines multiple steps to execute based on the value of inputs.scoper. The workflow runs as a composite action.

steps:
- name: Add Prefix
if: inputs.scoper
shell: bash
run: |
composer global require humbug/php-scoper
~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force
composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative
composer scoper-patch

Copy link

@github-actions github-actions bot Jul 19, 2023

Choose a reason for hiding this comment

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

🤖 OpenAI

The current implementation assumes that the php-scoper binary is available globally. However, this may not always be the case. It would be better to use the composer binary to execute the php-scoper command. Replace lines 15-18 with the following code:

Suggested change
- name: Add Prefix
if: inputs.scoper
shell: bash
run: |
composer global require humbug/php-scoper
~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force
composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative
composer scoper-patch
- name: Add Prefix
if: inputs.scoper
run: |
composer require humbug/php-scoper --dev
composer exec -- php-scoper add-prefix --output-dir=vendor-prefixed --force
composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative
composer scoper-patch

- name: Cleanup Vendor
if: inputs.scoper
run: rm -rf vendor
shell: bash

Copy link

@github-actions github-actions bot Jul 19, 2023

Choose a reason for hiding this comment

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

🤖 OpenAI

The cleanup of the vendor directory should only be performed if the PHP Scoper step is enabled. Move the if condition to line 21 and remove the shell: bash attribute from line 23. The updated code should look like this:

Suggested change
- name: Cleanup Vendor
if: inputs.scoper
run: rm -rf vendor
shell: bash
- name: Cleanup Vendor
if: inputs.scoper
run: rm -rf vendor

- name: Skip PHP Scoper
if: ! inputs.scoper
run: echo "Skipping Scoper"
shell: bash
24 changes: 15 additions & 9 deletions .github/workflows/wordpress.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
php: [ '7.2', '7.4', '8.0' ] # PHP versions to check.
wp: [ 'latest', '5.9' ] # WordPress version to check.
tools: [ composer ]

scoper: [ true, false ]
services:
mysql:
image: mysql:8.0
Copy link

@github-actions github-actions bot Jul 19, 2023

Choose a reason for hiding this comment

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

🤖 OpenAI

The services section in the workflow file has been modified. It now includes a mysql service with the image set to mysql:8.0.

Expand Down Expand Up @@ -47,12 +47,20 @@ jobs:
- name: Install dependencies
run: composer install --prefer-dist --no-progress --no-suggest
Comment on lines 46 to 47

Choose a reason for hiding this comment

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

🤖 OpenAI

Add the --no-dev flag to exclude development dependencies if they are not required for testing purposes.

50:
Make sure the ./github/actions/php-scoper/action.yml file exists and contains the necessary configuration.

53:
Ensure that the correct value is being passed to the scoper input.

57:
Review the files being made executable to ensure that only the necessary files are included and there are no security risks associated with this step.


Comment on lines 47 to 48

Choose a reason for hiding this comment

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

🤖 OpenAI

The step installs dependencies using Composer. No issues found.

- uses: ./.github/actions/php-scoper/action.yml
name: Do PHP Scoper if possible
with:
scoper: ${{ matrix.scoper }}

Choose a reason for hiding this comment

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

🤖 OpenAI

A new step named "Do PHP Scoper if possible" has been added. It uses the action defined in .github/actions/php-scoper/action.yml. There are no issues with this change.

- name: Make Binaries Executionable
if: matrix.scoper
run: chmod +x vendor-prefixed/vendor/bin/*

Copy link

@github-actions github-actions bot Jul 19, 2023

Choose a reason for hiding this comment

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

🤖 OpenAI

A new step named "Make Binaries Executionable" has been added. It runs the command chmod +x vendor-prefixed/vendor/bin/* if the scoper matrix variable is true. There are no issues with this change.

- name: Start MySQL
run: |
sudo systemctl start mysql
mysql -h 127.0.0.1 --port 3306 -u root --password=root -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_password BY 'root';"
Comment on lines 59 to 61

Choose a reason for hiding this comment

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

🤖 OpenAI

A step is added to start MySQL and alter the root user's password. No issues found.

Comment on lines 60 to 61

Choose a reason for hiding this comment

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

🤖 OpenAI

Ensure that the workflow environment has the necessary permissions to execute the sudo systemctl start mysql and mysql commands.

65:
Make sure the bash bin/install-wp-tests.sh script exists and is correctly configured.

115:
Make sure the bash bin/build.sh script exists and is correctly configured.

118:
Make sure the ./github/actions/php-scoper/action.yml file exists and contains the necessary configuration.

120:
Ensure that the correct value is being passed to the scoper input.

123:
Make sure the bash bin/clean.sh script exists and is correctly configured.


<!-- This is an auto-generated comment by OpenAI -->



- name: Install WordPress
run: bash bin/install-wp-tests.sh wordpress root root 127.0.0.1:3306 ${{ matrix.wp }}
Comment on lines 63 to 64

Choose a reason for hiding this comment

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

🤖 OpenAI

A step is added to install WordPress for testing purposes. No issues found.


Copy link

@github-actions github-actions bot Jul 19, 2023

Choose a reason for hiding this comment

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

🤖 OpenAI

Several steps have been added to the workflow file. Here are the specific line ranges and their corresponding issues:

Comment on lines 58 to 65

Choose a reason for hiding this comment

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

🤖 OpenAI

Steps related to starting MySQL, installing WordPress, and running tests have been modified. There are no issues with these changes.

Expand Down Expand Up @@ -103,15 +111,13 @@ jobs:
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Comment on lines 135 to 137

Choose a reason for hiding this comment

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

🤖 OpenAI

A new step is added to build the package. No issues found.

- name: Build Package.
- name: Build Package
run: bash bin/build.sh

- name: Do PHP scoper
run: |
composer global require humbug/php-scoper
~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force
composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative
composer help-scoper
- uses: ./.github/actions/php-scoper/action.yml
name: Do PHP Scoper
with:
scoper: true

- name: Clean Unwanted files
run: bash bin/clean.sh

Choose a reason for hiding this comment

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

🤖 OpenAI

More steps have been added to the workflow. Here are the specific issues:

  • Line 115: The bash bin/build.sh command is used to build a package. Make sure this script exists and is correctly configured.
  • Line 118: The ./.github/actions/php-scoper/action.yml file is being used as an action. Make sure this file exists and contains the necessary configuration.
  • Line 120: The scoper input is being passed to the action, but it's not clear where this value is coming from. Ensure that the correct value is being passed here.
  • Line 123: The bash bin/clean.sh command is used to clean unwanted files. Make sure this script exists and is correctly configured.
Suggested change
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Build Package.
- name: Build Package
run: bash bin/build.sh
- name: Do PHP scoper
run: |
composer global require humbug/php-scoper
~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force
composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative
composer help-scoper
- uses: ./.github/actions/php-scoper/action.yml
name: Do PHP Scoper
with:
scoper: true
- name: Clean Unwanted files
run: bash bin/clean.sh
<!-- This is an auto-generated comment by OpenAI -->

Expand Down
17 changes: 11 additions & 6 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,20 @@
"test": "phpunit",
"lint": "phpcs --standard=phpcs.ruleset.xml $(find ./ -name '*.php')",
"fix": "phpcbf --standard=phpcs.ruleset.xml $(find ./ -name '*.php')",
"scoper": "php-scoper",
"prefix": [
"composer install --no-dev",
"scoper": [
"php-scoper add-prefix --output-dir=vendor-prefixed --force",
"composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative",
"composer install",
"@help-scoper"
"@scoper-patch"
],
"scoper-patch": [
"php bin/patcher.php",
"chmod +x vendor-prefixed/vendor/bin/*"
],
"help-scoper": "php bin/patcher.php"
"prefix": [
"composer install --no-dev",
"@scoper",
"composer install"
]
},
"require": {
"php": ">=7.2",
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"update": "wp-env start --update",
"stop": "wp-env stop",
"test": "wp-env run tests-cli ./wp-content/plugins/ga-communicator/vendor/bin/phpunit -c ./wp-content/plugins/ga-communicator/phpunit.xml.dist",
"test:scoper": "wp-env run tests-cli ./wp-content/plugins/ga-communicator/vendor-prefixed/vendor/bin/phpunit -c ./wp-content/plugins/ga-communicator/phpunit.xml.dist",
"env": "wp-env",
"cli": "wp-env run cli wp",
"cli:test": "wp-env run tests-cli wp",
Expand Down
19 changes: 15 additions & 4 deletions scoper.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,25 @@ static function ( string $filePath, string $prefix, string $contents ): string {

// For more information see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#excluded-symbols
'exclude-namespaces' => [
'/^cli/',
'/^Composer/',
'/^DeepCopy/',
'/^Doctrine/',
'/^Kunoichi/',
'/^PharIo/',
'/^PHP_CodeSniffer/',
'/^PHPCompatibility/',
'/^PHPCSStandards/',
'/^PHPUnit/',
'/^Yoast/',
'/^SebastianBergmann/',
'/^TheSeer/',
'/^WP_/',
'/^Composer/',
'/^cli/'
],
Comment on lines 52 to 69

Choose a reason for hiding this comment

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

🤖 OpenAI

The exclude-namespaces array in the PHP-Scoper configuration has been modified. Several namespaces have been added to exclude from scoping. However, it seems that some of the namespaces were already present in the original configuration. Please verify if these additions are intentional and necessary.

'exclude-classes' => [
'/^WP_/',
'/^Composer/'
'/^Composer/',
'/^PHPUnit/',
],
Comment on lines 70 to 74

Choose a reason for hiding this comment

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

🤖 OpenAI

The exclude-classes array in the PHP-Scoper configuration has been modified. Some classes have been added to exclude from scoping. Similar to the previous comment, please ensure that these additions are intentional and necessary.

'exclude-functions' => [
// 'mb_str_split',
Comment on lines 52 to 76

Choose a reason for hiding this comment

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

🤖 OpenAI

The new hunk updates the excluded namespaces, classes, and functions in the PHP-Scoper configuration file. It appears that the changes are intended to exclude additional namespaces, classes, and functions from being scoped. Without the complete context, it's difficult to determine if there are any issues with this change.

Expand All @@ -80,7 +91,7 @@ static function ( string $filePath, string $prefix, string $contents ): string {
//
// For more information see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#exposed-symbols
'expose-global-constants' => true,
'expose-global-classes' => true,
'expose-global-classes' => false,
'expose-global-functions' => false,
'expose-namespaces' => [
],
Comment on lines 83 to 89
Copy link

@github-actions github-actions bot Jul 19, 2023

Choose a reason for hiding this comment

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

🤖 OpenAI

The new hunk modifies the values of 'expose-global-classes' and 'expose-global-functions' in the PHP-Scoper configuration file. The value of 'expose-global-classes' has been changed to false, and 'expose-global-functions' remains false. Without the complete context, it's difficult to determine if there are any issues with this change.

Expand Down
8 changes: 7 additions & 1 deletion tests/bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,13 @@
// Give access to tests_add_filter() function.
require_once $_tests_dir . '/includes/functions.php';

Comment on lines 19 to 21

Choose a reason for hiding this comment

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

🤖 OpenAI

The comment on line 19 is unnecessary and can be removed.

require_once dirname( __DIR__ ) . '/vendor/autoload.php';
/**
* Manually load the plugin being tested.
*/
function _manually_load_plugin() {
require dirname( dirname( __FILE__ ) ) . '/ga-communicator.php';
}
Comment on lines +25 to +27

Choose a reason for hiding this comment

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

🤖 OpenAI

The function _manually_load_plugin() is added to manually load the plugin being tested. This function should be documented with a PHPDoc block to describe its purpose, parameters, and return value.

/**
 * Manually load the plugin being tested.
 */
function _manually_load_plugin() {
    require dirname(dirname(__FILE__)) . '/ga-communicator.php';
}

28:
The tests_add_filter() function is used to add a filter to load the plugin before running the tests. It would be helpful to provide a comment explaining the purpose of this filter.

// Load the plugin before running the tests.
tests_add_filter('muplugins_loaded', '_manually_load_plugin');

Please ensure that the code snippets provided above are placed in the correct location within the file.

tests_add_filter( 'muplugins_loaded', '_manually_load_plugin' );

// Start up the WP testing environment.
require $_tests_dir . '/includes/bootstrap.php';
Loading