-
Notifications
You must be signed in to change notification settings - Fork 1
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
Changes from 8 commits
04ae20f
f6f4291
34f8e37
6b2758c
87e8427
3cd35dc
a6e9ee5
ac3cafa
c7357f1
cf05a2a
fa6c5ca
7f8c8c3
0df79ca
e9e8959
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,28 @@ | ||||||||||||||||||||||||||||||||
name: Setup PHP Scoper | ||||||||||||||||||||||||||||||||
description: Setup PHP Scoper | ||||||||||||||||||||||||||||||||
inputs: | ||||||||||||||||||||||||||||||||
scoper: | ||||||||||||||||||||||||||||||||
description: Whether to do PHP Scoper | ||||||||||||||||||||||||||||||||
required: true | ||||||||||||||||||||||||||||||||
Comment on lines
+1
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI The 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI The 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI The
Comment on lines
+1
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||||||||||||||||||||||||||||
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 | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI The current implementation assumes that the
Suggested change
|
||||||||||||||||||||||||||||||||
- name: Cleanup Vendor | ||||||||||||||||||||||||||||||||
if: inputs.scoper | ||||||||||||||||||||||||||||||||
run: rm -rf vendor | ||||||||||||||||||||||||||||||||
shell: bash | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI The cleanup of the
Suggested change
|
||||||||||||||||||||||||||||||||
- name: Skip PHP Scoper | ||||||||||||||||||||||||||||||||
if: inputs.scoper != true | ||||||||||||||||||||||||||||||||
run: echo "Skipping Scoper" | ||||||||||||||||||||||||||||||||
shell: bash | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI The condition on line 26 should be modified to check if the
Suggested change
Overall, the changes made to 1: name: Setup PHP Scoper
2: description: Setup PHP Scoper
3: inputs:
4: scoper:
5: description: Whether to do PHP Scoper
6: required: false
7:
8: runs:
9: using: "composite"
10: steps:
11: - name: Add Prefix
12: if: inputs.scoper
13: run: |
14: composer require humbug/php-scoper --dev
15: composer exec -- php-scoper add-prefix --output-dir=vendor-prefixed --force
16: composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative
17: composer scoper-patch
18:
19: - name: Cleanup Vendor
20: if: inputs.scoper
21: run: rm -rf vendor
22:
23: - name: Skip PHP Scoper
24: if: inputs.scoper == 'false'
25: run: echo "Skipping Scoper" Note: Please ensure that the |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI The |
||
|
@@ -31,7 +31,7 @@ jobs: | |
MYSQL_AUTHENTICATION_PLUGIN: mysql_native_password | ||
name: UnitTest on WordPress ${{ matrix.wp }} in PHP ${{ matrix.php }} | ||
steps: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI A new step named "Setup PHP with composer v2" has been added to the workflow. It uses the |
||
- uses: actions/checkout@master | ||
- uses: actions/checkout@v3 | ||
|
||
- name: Setup PHP with composer v2 | ||
uses: shivammathur/setup-php@v2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI A new step named "Setup PHP with composer v2" has been added to the workflow file. It uses the action |
||
|
@@ -47,12 +47,20 @@ jobs: | |
- name: Install dependencies | ||
run: composer install --prefer-dist --no-progress --no-suggest | ||
Comment on lines
46
to
47
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: 53: 57: |
||
|
||
Comment on lines
47
to
48
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
name: Do PHP Scoper if possible | ||
with: | ||
scoper: ${{ matrix.scoper }} | ||
|
||
- name: Make Binaries Executionable | ||
if: matrix.scoper | ||
run: chmod +x vendor-prefixed/vendor/bin/* | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
- 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: 115: 118: 120: 123:
|
||
|
||
|
||
- 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
|
||
Comment on lines
58
to
65
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI Several steps have been added to the workflow. Here are the key changes:
|
||
|
@@ -66,7 +74,7 @@ jobs: | |
name: Check Assets | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@master | ||
- uses: actions/checkout@v3 | ||
|
||
- name: Install Node | ||
uses: actions/setup-node@v3 | ||
Comment on lines
98
to
104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI A new job named "Check Assets" has been added to the workflow. It runs on
|
||
|
@@ -93,7 +101,7 @@ jobs: | |
if: contains(github.ref, 'tags/') | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@master | ||
- uses: actions/checkout@v3 | ||
|
||
- name: Setup PHP with composer v2 | ||
uses: shivammathur/setup-php@v2 | ||
Comment on lines
125
to
131
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI A new job named "Build on Tags" has been added to the workflow. It runs only if the GitHub ref contains
|
||
|
@@ -103,15 +111,13 @@ jobs: | |
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
||
Comment on lines
135
to
137
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
name: Do PHP Scoper | ||
with: | ||
scoper: true | ||
|
||
- name: Clean Unwanted files | ||
run: bash bin/clean.sh | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI Several steps have been added to the "Build on Tags" job. Here are the key changes:
Overall, the changes in the workflow file seem fine. However, it's important to review the contents of the custom actions ( |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,14 +36,6 @@ | |
] ), | ||
], | ||
|
||
// List of excluded files, i.e. files for which the content will be left untouched. | ||
// Paths are relative to the configuration file unless if they are already absolute | ||
// | ||
// For more see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#patchers | ||
'exclude-files' => [ | ||
'src/a-whitelisted-file.php', | ||
], | ||
|
||
// When scoping PHP files, there will be scenarios where some of the code being scoped indirectly references the | ||
// original namespace. These will include, for example, strings or string manipulations. PHP-Scoper has limited | ||
// support for prefixing such strings. To circumvent that, you can define patchers to manipulate the file to your | ||
Comment on lines
36
to
41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI The comment in the new hunk provides a good explanation of the purpose of the patchers. However, it seems that the old hunk is missing some lines of code that were removed. Without the complete context, it's difficult to determine if there are any issues with this change. |
||
|
@@ -60,14 +52,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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI The |
||
'exclude-classes' => [ | ||
'/^WP_/', | ||
'/^Composer/' | ||
'/^Composer/', | ||
'/^PHPUnit/', | ||
], | ||
Comment on lines
70
to
74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI The |
||
'exclude-functions' => [ | ||
// 'mb_str_split', | ||
Comment on lines
52
to
76
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
|
@@ -80,7 +83,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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,13 @@ | |
// Give access to tests_add_filter() function. | ||
require_once $_tests_dir . '/includes/functions.php'; | ||
|
||
Comment on lines
19
to
21
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 OpenAI The function /**
* Manually load the plugin being tested.
*/
function _manually_load_plugin() {
require dirname(dirname(__FILE__)) . '/ga-communicator.php';
} 28: // 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'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 OpenAI
The
name
anddescription
fields in the GitHub Actions workflow file should be more descriptive. Consider updating them to provide clearer information about the purpose of the workflow.