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

Move auto-sizes to plugins folder #972

Merged
merged 41 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
aa2437d
Add plugins folder
mukeshpanchal27 Jan 18, 2024
c366c91
ignore plugins folder
mukeshpanchal27 Jan 18, 2024
2ebdda4
Restructure the plugins.json file
mukeshpanchal27 Jan 18, 2024
c59f9f9
Update build-plugins script
mukeshpanchal27 Jan 18, 2024
5ab7268
Update test-plugins script
mukeshpanchal27 Jan 18, 2024
177b2e9
Update Deploy standalone plugins workflow
mukeshpanchal27 Jan 18, 2024
c55b3e6
Add plugins in tooling
mukeshpanchal27 Jan 18, 2024
7c3d1d1
Merge branch 'feature/modules-to-plugins' into add/934-plugins-folder
mukeshpanchal27 Jan 18, 2024
7df015d
Apply suggestions from code review
mukeshpanchal27 Jan 18, 2024
643d78b
Update variable names
mukeshpanchal27 Jan 19, 2024
b0bad91
Remove the temporary changes
mukeshpanchal27 Jan 19, 2024
16fda54
Merge pull request #948 from WordPress/add/934-plugins-folder
felixarntz Jan 19, 2024
dbe1a99
Plugin test suite
mukeshpanchal27 Jan 29, 2024
cef64a3
Add test file
mukeshpanchal27 Jan 29, 2024
8a40b2c
Run test for branch
mukeshpanchal27 Jan 29, 2024
43b630a
Update test command
mukeshpanchal27 Jan 30, 2024
54ca2e6
Update unit test command
mukeshpanchal27 Jan 30, 2024
218a69d
Merge pull request #959 from WordPress/trunk
westonruter Jan 30, 2024
7c92bc0
Merge branch 'feature/modules-to-plugins' into add/plugins-test-suite
mukeshpanchal27 Jan 31, 2024
4d18663
Add another test
mukeshpanchal27 Feb 5, 2024
2b9693f
Merge branch 'add/plugins-test-suite' of github.com:WordPress/perform…
mukeshpanchal27 Feb 5, 2024
8c04816
Merge pull request #968 from WordPress/trunk
mukeshpanchal27 Feb 5, 2024
f47b1f8
Merge branch 'feature/modules-to-plugins' into add/plugins-test-suite
mukeshpanchal27 Feb 5, 2024
c75b3dc
Revert changes
mukeshpanchal27 Feb 5, 2024
ecd9d21
Adjust composer update
mukeshpanchal27 Feb 5, 2024
500fb91
Update plugin name to Test Plugin
mukeshpanchal27 Feb 6, 2024
4c21e8f
Merge pull request #956 from WordPress/add/plugins-test-suite
mukeshpanchal27 Feb 6, 2024
161a738
Move plugin to plugins folder
mukeshpanchal27 Feb 6, 2024
d10052f
Merge branch 'feature/auto-sizes' into move/auto-sizes
mukeshpanchal27 Feb 6, 2024
23bcfc2
Unit test coverage setup
mukeshpanchal27 Feb 6, 2024
edba918
Add new line at end
mukeshpanchal27 Feb 6, 2024
45a8b57
Add new line at end
mukeshpanchal27 Feb 6, 2024
2daf975
Merge branch 'feature/modules-to-plugins' into move/auto-sizes
mukeshpanchal27 Feb 6, 2024
88040f2
Use single unit test file for plugins
mukeshpanchal27 Feb 7, 2024
14c4776
Adjust spacing
mukeshpanchal27 Feb 7, 2024
87f3a04
Added support for plugin/plugin.php
mukeshpanchal27 Feb 7, 2024
852775d
Remove test plugin
mukeshpanchal27 Feb 9, 2024
bc2884f
Adjust bootstrap file code
mukeshpanchal27 Feb 9, 2024
4b0974e
Add plugin into plugins.json
mukeshpanchal27 Feb 12, 2024
8cfded4
Address review feedback
mukeshpanchal27 Feb 13, 2024
7f3b13b
Address review feedback
mukeshpanchal27 Feb 15, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
/docs export-ignore
/tests export-ignore
/plugin-tests export-ignore
/plugins export-ignore

/*.DS_store export-ignore
/.DS_store? export-ignore
Expand Down
7 changes: 3 additions & 4 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
/tests/modules/images/dominant-color-images @pbearne @spacedmonkey
/tests/testdata/modules/images/dominant-color-images @pbearne @spacedmonkey

# Module: Auto-sizes for Lazy-Loaded Images
/modules/images/auto-sizes @joemcgill
/tests/modules/images/auto-sizes @joemcgill
/tests/testdata/modules/images/auto-sizes @joemcgill
# Plugin: Auto-sizes for Lazy-Loaded Images
/plugins/auto-sizes @joemcgill
/tests/plugins/auto-sizes @joemcgill
2 changes: 1 addition & 1 deletion .github/workflows/deploy-standalone-plugins.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
# for use in the matrix.
# The "dry-run" parameter is included here to set the deployment mode.
# When running the manual (workflow_dispatch) workflow, this value will be set from manual input type.
echo "matrix="$(jq -c '{include:[keys[] as $k | {name:$k,slug:.[$k].slug,version:.[$k].version,"dry-run":false }]}' plugins.json) >> $GITHUB_OUTPUT
echo "matrix="$(jq -c '{include:[.modules | to_entries[] | {name:.key,slug:.value.slug,version:.value.version,"dry-run":false }]}' plugins.json) >> $GITHUB_OUTPUT
fi
deploy:
name: Deploy Plugin
Expand Down
78 changes: 78 additions & 0 deletions .github/workflows/php-test-plugins.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
name: Unit Testing for Plugins

on:
push:
branches:
- trunk
- 'release/**'
# Only run if PHP-related files changed.
paths:
- '.github/workflows/php-test-plugins.yml'
- 'plugins/**.php'
- '.wp-env.json'
- '**/package.json'
- 'package-lock.json'
- 'phpunit.xml.dist'
- 'tests/multisite.xml'
- 'composer.json'
- 'composer.lock'
pull_request:
branches:
- trunk
- 'release/**'
- 'feature/**'
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
# Only run if PHP-related files changed.
paths:
- '.github/workflows/php-test-plugins.yml'
- 'plugins/**.php'
- '.wp-env.json'
- '**/package.json'
- 'package-lock.json'
- 'phpunit.xml.dist'
- 'tests/multisite.xml'
- 'composer.json'
- 'composer.lock'
types:
- opened
- reopened
- synchronize

jobs:
php-test-plugins:
name: "PHP ${{ matrix.php }} / WP ${{ matrix.wp }}"
runs-on: ubuntu-latest
timeout-minutes: 20
strategy:
fail-fast: false
matrix:
php: ['8.2', '8.1', '8.0', '7.4', '7.3', '7.2', '7.1', '7.0']
wp: [ 'latest' ]
include:
- php: '7.4'
wp: '6.3'
- php: '8.3'
wp: 'trunk'
env:
WP_ENV_PHP_VERSION: ${{ matrix.php }}
WP_ENV_CORE: ${{ matrix.wp == 'trunk' && 'WordPress/WordPress' || format( 'https://wordpress.org/wordpress-{0}.zip', matrix.wp ) }}
Copy link
Member

Choose a reason for hiding this comment

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

Question: Why not use ternary expression here? Is it allowed?

Suggested change
WP_ENV_CORE: ${{ matrix.wp == 'trunk' && 'WordPress/WordPress' || format( 'https://wordpress.org/wordpress-{0}.zip', matrix.wp ) }}
WP_ENV_CORE: ${{ matrix.wp == 'trunk' ? 'WordPress/WordPress' : format( 'https://wordpress.org/wordpress-{0}.zip', matrix.wp ) }}

Copy link
Member Author

Choose a reason for hiding this comment

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

This was added as part of #544. If we need to update this, then we also have to update similar settings for other workflows (php-test.yml and php-test-standalone-plugins.yml).

Shall we open a follow-up PR to make these changes?

Copy link
Member

Choose a reason for hiding this comment

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

Follow-up PR is fine. Just seems hard to read right now.

Copy link
Member Author

Choose a reason for hiding this comment

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

@westonruter, GitHub workflow does not allow ${{ x ? 'ifTrue' : 'ifFalse' }} ternary operator. Instead, it uses ${{ x && 'ifTrue' || 'ifFalse' }}. Therefore, we don't need to open a follow-up PR for this work.

steps:
- uses: styfle/cancel-workflow-action@0.11.0
- uses: actions/checkout@v3
- name: Setup Node.js (.nvmrc)
uses: actions/setup-node@v3
with:
node-version-file: '.nvmrc'
cache: npm
- name: npm install
run: npm ci
- name: Install WordPress
run: npm run wp-env start
# Note that `composer update` is required instead of `composer install`
# for the sake of PHP versions older than 8.1, which is the version of
# PHP that the composer.lock was created for.
- name: Composer update
run: npm run wp-env run tests-cli -- --env-cwd="wp-content/plugins/$(basename $(pwd))" composer update --no-interaction
- name: Running single site unit tests for Auto-sizes for Lazy-Loaded Images Plugin
run: npm run test-php -- -- -- --testsuite auto-sizes
- name: Running multisite unit tests for Auto-sizes for Lazy-Loaded Images Plugin
run: npm run test-php-multisite -- -- -- --testsuite auto-sizes
11 changes: 10 additions & 1 deletion bin/plugin/commands/build-plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,16 @@ exports.handler = async () => {
}

try {
const plugins = JSON.parse( jsonString );
const pluginsConfig = JSON.parse( jsonString );
const plugins = pluginsConfig.modules;
if ( ! plugins ) {
log(
formats.error(
'The given module configuration is invalid, the modules are missing, or they are misspelled.'
)
);
return;
}
for ( const moduleDir in plugins ) {
const pluginVersion = plugins[ moduleDir ]?.version;
const pluginSlug = plugins[ moduleDir ]?.slug;
Expand Down
14 changes: 12 additions & 2 deletions bin/plugin/commands/get-plugin-version.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,25 @@ function doRunGetPluginVersion( settings ) {
);
}

const plugins = JSON.parse( pluginsFileContent );
const pluginsConfig = JSON.parse( pluginsFileContent );

// Check for valid and not empty object resulting from plugins JSON file parse.
if ( 'object' !== typeof plugins || 0 === Object.keys( plugins ).length ) {
if (
'object' !== typeof pluginsConfig ||
0 === Object.keys( pluginsConfig ).length
) {
throw Error(
`File at "${ pluginsFile }" parsed, but detected empty/non valid JSON object.`
);
}

const plugins = pluginsConfig.modules;
if ( ! plugins ) {
throw Error(
`File at "${ pluginsFile }" parsed, but the modules are missing, or they are misspelled.`
);
}

for ( const moduleDir in plugins ) {
const pluginVersion = plugins[ moduleDir ]?.version;
const pluginSlug = plugins[ moduleDir ]?.slug;
Expand Down
26 changes: 19 additions & 7 deletions bin/plugin/commands/test-plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -428,12 +428,12 @@ function doRunStandalonePluginTests( settings ) {
);
}

const pluginsJsonFileContentAsJson = JSON.parse( pluginsJsonFileContent );
const pluginsConfig = JSON.parse( pluginsJsonFileContent );

// Check for valid and not empty object resulting from plugins JSON file parse.
if (
'object' !== typeof pluginsJsonFileContentAsJson ||
0 === Object.keys( pluginsJsonFileContentAsJson ).length
'object' !== typeof pluginsConfig ||
0 === Object.keys( pluginsConfig ).length
) {
log(
formats.error(
Expand All @@ -445,24 +445,36 @@ function doRunStandalonePluginTests( settings ) {
process.exit( 1 );
}

const plugins = pluginsConfig.modules;
if ( ! plugins ) {
log(
formats.error(
'The given module configuration is invalid, the modules are missing, or they are misspelled.'
)
);

// Return with exit code 1 to trigger a failure in the test pipeline.
process.exit( 1 );
}

// Create an array of plugins from entries in plugins JSON file.
builtPlugins = Object.keys( pluginsJsonFileContentAsJson )
builtPlugins = Object.keys( plugins )
.filter( ( item ) => {
if (
! fs.pathExistsSync(
`${ settings.builtPluginsDir }${ pluginsJsonFileContentAsJson[ item ].slug }`
`${ settings.builtPluginsDir }${ plugins[ item ].slug }`
)
) {
log(
formats.error(
`Built plugin path "${ settings.builtPluginsDir }${ pluginsJsonFileContentAsJson[ item ].slug }" not found, skipping and removing from plugin list`
`Built plugin path "${ settings.builtPluginsDir }${ plugins[ item ].slug }" not found, skipping and removing from plugin list`
)
);
return false;
}
return true;
} )
.map( ( item ) => pluginsJsonFileContentAsJson[ item ].slug );
.map( ( item ) => plugins[ item ].slug );

// For each built plugin, copy the test assets.
builtPlugins.forEach( ( plugin ) => {
Expand Down
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
"phpstan": "build-cs/vendor/bin/phpstan analyse --memory-limit=2048M -c phpstan.neon.dist",
"format": "build-cs/vendor/bin/phpcbf --standard=phpcs.xml.dist --report-summary --report-source",
"lint": "build-cs/vendor/bin/phpcs --standard=phpcs.xml.dist",
"test": "phpunit -c phpunit.xml.dist --verbose",
"test-multisite": "phpunit -c tests/multisite.xml --verbose"
"test": "phpunit -c phpunit.xml.dist --verbose --testsuite performance-lab",
"test-multisite": "phpunit -c tests/multisite.xml --verbose --testsuite performance-lab"
westonruter marked this conversation as resolved.
Show resolved Hide resolved
},
"config": {
"allow-plugins": {
Expand Down
23 changes: 0 additions & 23 deletions modules/images/auto-sizes/load.php

This file was deleted.

3 changes: 2 additions & 1 deletion phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@
<rule ref="WordPress-Docs"/>
<rule ref="WordPress-Extra" />
<rule ref="WordPress.WP.I18n"/>
<config name="text_domain" value="performance-lab,default"/>
<config name="text_domain" value="performance-lab,default,auto-sizes"/>
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved

<arg value="ps"/>
<arg name="extensions" value="php"/>

<file>./admin</file>
<file>./load.php</file>
<file>./modules</file>
<file>./plugins</file>
<file>./server-timing</file>
<file>./tests</file>

Expand Down
1 change: 1 addition & 0 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ parameters:
- load.php
- module-i18n.php
- modules/
- plugins/
- server-timing/
- tests/
- uninstall.php
Expand Down
8 changes: 6 additions & 2 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@
convertWarningsToExceptions="true"
>
<testsuites>
<testsuite name="default">
<testsuite name="performance-lab">
<directory suffix=".php">./tests</directory>
<exclude>./tests/utils</exclude>
<exclude>./tests/utils</exclude>
<exclude>./tests/plugins</exclude>
</testsuite>
<testsuite name="auto-sizes">
<directory suffix=".php">./tests/plugins/auto-sizes</directory>
</testsuite>
</testsuites>
<groups>
Expand Down
17 changes: 10 additions & 7 deletions plugins.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
{
"images/dominant-color-images": {
"slug": "dominant-color-images",
"version": "1.0.1"
"modules": {
"images/dominant-color-images": {
"slug": "dominant-color-images",
"version": "1.0.1"
},
"images/webp-uploads": {
"slug": "webp-uploads",
"version": "1.0.5"
}
},
"images/webp-uploads": {
"slug": "webp-uploads",
"version": "1.0.5"
}
"plugins": [ "auto-sizes" ]
}
30 changes: 30 additions & 0 deletions plugins/auto-sizes/auto-sizes.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php
/**
* Plugin Name: Auto-sizes for Lazy-loaded Images
* Plugin URI: https://github.com/WordPress/performance/tree/trunk/modules/images/auto-sizes
* Description: This plugin implements the HTML spec for adding `sizes="auto"` to lazy-loaded images.
* Requires at least: 6.3
* Requires PHP: 7.0
* Version: 1.0.0
* Author: WordPress Performance Team
* Author URI: https://make.wordpress.org/performance/
* License: GPLv2 or later
* License URI: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
* Text Domain: auto-sizes
*
* @package auto-sizes
*/

// Exit if accessed directly.
if ( ! defined( 'ABSPATH' ) ) {
exit;
}

// Define the constant.
if ( defined( 'IMAGE_AUTO_SIZES_VERSION' ) ) {
return;
}

define( 'IMAGE_AUTO_SIZES_VERSION', '1.0.0' );

require_once __DIR__ . '/hooks.php';
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
/**
* Hook callbacks used for Auto-sizes for Lazy-loaded Images.
*
* @package performance-lab
* @since n.e.x.t
* @package auto-sizes
* @since 1.0.0
*/

if ( ! defined( 'ABSPATH' ) ) {
Expand All @@ -13,7 +13,7 @@
/**
* Adds auto to the sizes attribute to the image, if applicable.
*
* @since n.e.x.t
* @since 1.0.0
*
* @param array $attr Attributes for the image markup.
* @return array The filtered attributes for the image markup.
Expand Down Expand Up @@ -43,7 +43,7 @@ function auto_sizes_update_image_attributes( $attr ) {
/**
* Adds auto to the sizes attribute to the image, if applicable.
*
* @since n.e.x.t
* @since 1.0.0
*
* @param string $html The HTML image tag markup being filtered.
* @return string The filtered HTML image tag markup.
Expand Down
File renamed without changes.
Loading
Loading