Skip to content

phpcs: Fix and enable Jetpack.PHPUnit.Attributes sniff #43175

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.

Sign up for GitHub

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

Jump to bottom
Merged
anomiex merged 4 commits into from
Apr 22, 2025
Merged

phpcs: Fix and enable Jetpack.PHPUnit.Attributes sniff #43175

anomiex merged 4 commits into from
Apr 22, 2025

Conversation

@anomiex
Copy link
Contributor

@anomiex anomiex commented Apr 21, 2025

Proposed changes:

Mostly this is the result of running phpcbf on files failing the sniff.

Things needing manual cleanup were:

  • projects/packages/image-cdn/tests/php/Image_CDN_Test.php: Remove a broken @covers that had no space between the tag and class name.

  • projects/plugins/jetpack/tests/php/core-api/WPCOM_REST_API_V2_Field_Controller_Test.php: Fix a @group that was accidentally "rest-api\n\nphpcs:disable Generic.Files.OneObjectStructurePerFile.MultipleFound".

  • projects/plugins/jetpack/tests/php/core-api/wpcom-fields/WPCOM_REST_API_V2_Attachment_VideoPress_Data_Test.php:

  • projects/plugins/jetpack/tests/php/sync/Jetpack_Sync_Queue_TestBase.php: Move some @groups from the file comment to class comment.

  • projects/plugins/jetpack/tests/php/Functions_OpenGraph_Test.php:

  • projects/plugins/jetpack/tests/php/general/Jetpack_Client_Server_Test.php: Fix where it got confused about whether a comment was a file or class comment.

  • projects/plugins/jetpack/tests/php/json-api/Jetpack_Json_Api_Plugins_Endpoints_Test.php: Fix @covers for nonexistent class.

  • projects/plugins/jetpack/tests/php/modules/shortcodes/trait.http-request-cache.php:

  • projects/plugins/jetpack/tests/php/trait-woo-tests.php: Manually fix, since the sniff doesn't recognize the trait as a test class.

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

pf4qpu-Fo-p2
p1745258954968799/1745251420.256849-slack-C02LK1W8T4Z

Does this pull request change what data or activity we track or use?

No

Testing instructions:

  • CI happy?

@anomiex anomiex requested review from a team April 21, 2025 18:24
@anomiex anomiex self-assigned this Apr 21, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 21, 2025 •
edited
Loading

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack or WordPress.com Site Helper), and enable the fix/phpcs-phpunit-attributes-sniff branch.
  • To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack fix/phpcs-phpunit-attributes-sniff

bin/jetpack-downloader test jetpack-mu-wpcom-plugin fix/phpcs-phpunit-attributes-sniff

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions github-actions bot added [Package] Subscribers Dashboard [Package] Sync [Package] VideoPress [Package] WAF [Plugin] Boost A feature to speed up the site and improve performance. [Plugin] CRM Issues about the Jetpack CRM plugin [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Plugin] Social Issues about the Jetpack Social plugin [Plugin] Wpcomsh [Tests] Includes Tests labels Apr 21, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 21, 2025 •
edited
Loading

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  • ✅ Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

Jetpack plugin:

The Jetpack plugin has different release cadences depending on the platform:

  • WordPress.com Simple releases happen as soon as you deploy your changes after merging this PR (PCYsg-Jjm-p2).
  • WoA releases happen weekly.
  • Releases to self-hosted sites happen monthly:
    • Scheduled release: May 6, 2025
    • Code freeze: May 5, 2025

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Boost plugin:

  • Next scheduled release: April 30, 2025

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Social plugin:

No scheduled milestone found for this plugin.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

Wpcomsh plugin:

  • Next scheduled release: Atomic deploys happen twice daily on weekdays (p9o2xV-2EN-p2)

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

@anomiex
Copy link
Contributor Author

anomiex commented Apr 21, 2025

This is one that I'll want to at least locally rebase and re-run phpcs before actually merging, to try to make sure other PRs didn't add new things needing fixing.

@jp-launch-control
Copy link

jp-launch-control bot commented Apr 21, 2025 •
edited
Loading

Code Coverage Summary

Coverage changed in 11 files. Only the first 5 are listed here.

File Coverage Δ% Δ Uncovered
projects/packages/masterbar/src/admin-menu/class-admin-menu.php 165/216 (76.39%) 0.93% -2 💚
projects/plugins/jetpack/json-endpoints/class.wpcom-json-api-taxonomy-endpoint.php 3/4 (75.00%) 75.00% -3 💚
projects/plugins/crm/src/automation/class-base-trigger.php 7/14 (50.00%) 50.00% -7 💚
projects/plugins/crm/src/rest-api/v4/class-rest-base-controller.php 8/11 (72.73%) 72.73% -8 💚
projects/plugins/crm/src/automation/class-base-condition.php 10/11 (90.91%) 90.91% -10 💚

Full summary · PHP report · JS report

@github-actions github-actions bot added the Actions GitHub actions used to automate some of the work around releases and repository management label Apr 21, 2025
Base automatically changed from add/phpcs-phpunit-attributes-sniff to trunk April 21, 2025 19:56
anomiex added 2 commits April 21, 2025 15:59
@anomiex
phpcs: Fix and enable Jetpack.PHPUnit.Attributes sniff
f6883b7 
Mostly this is the result of running phpcbf on files failing the sniff.

Things needing manual cleanup were:

* `projects/packages/image-cdn/tests/php/Image_CDN_Test.php`:
  Remove a broken `@covers` that had no space between the tag and class
  name.

* `projects/plugins/jetpack/tests/php/core-api/WPCOM_REST_API_V2_Field_Controller_Test.php`:
  Fix a `@group` that was accidentally
  "rest-api\n\nphpcs:disable Generic.Files.OneObjectStructurePerFile.MultipleFound".

* `projects/plugins/jetpack/tests/php/core-api/wpcom-fields/WPCOM_REST_API_V2_Attachment_VideoPress_Data_Test.php`:
* `projects/plugins/jetpack/tests/php/sync/Jetpack_Sync_Queue_TestBase.php`:
  Move some `@group`s from the file comment to class comment.

* `projects/plugins/jetpack/tests/php/Functions_OpenGraph_Test.php`:
* `projects/plugins/jetpack/tests/php/general/Jetpack_Client_Server_Test.php`:
  Fix where it got confused about whether a comment was a file or class
  comment.

* `projects/plugins/jetpack/tests/php/json-api/Jetpack_Json_Api_Plugins_Endpoints_Test.php`:
  Fix `@covers` for nonexistent class.

* `projects/plugins/jetpack/tests/php/modules/shortcodes/trait.http-request-cache.php`:
* `projects/plugins/jetpack/tests/php/trait-woo-tests.php`:
  Manually fix, since the sniff doesn't recognize the trait as a test
  class.
@anomiex
Disable `PHPCompatibility.Attributes.NewAttributes.PHPUnitAttributeFo…
1328706 
…und` in phpcompatibility-dev run.

`PHPCompatibility.Attributes.NewAttributes.Found` was already disabled.
@anomiex anomiex force-pushed the fix/phpcs-phpunit-attributes-sniff branch from fc6607e to 1328706 Compare April 21, 2025 19:59
@anomiex
Copy link
Contributor Author

anomiex commented Apr 21, 2025

Surprising that this PR somehow changed code coverage on anything.

On investigation, it looks like @covers SomeClass will cover only SomeClass while #[CoversClass( SomeClass::class )] will cover SomeClass and any of its parent classes. The latter is apparently an intentional change in PHPUnit 11.1 which was not made to apply to the equivalent annotation.

tbradsha
tbradsha reviewed Apr 22, 2025
View reviewed changes
Copy link
Contributor

@tbradsha tbradsha left a comment

Choose a reason for hiding this comment

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

I've reviewed through plugins/jetpack so far. It looks like GH's interface only auto-loads diff content for the first 200 files, so I'm submitting this portion in case what I do next would lose those comments.

tbradsha
tbradsha approved these changes Apr 22, 2025
View reviewed changes
Copy link
Contributor

@tbradsha tbradsha left a comment

Choose a reason for hiding this comment

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

A few random inline comments (mostly to satisfy my own curiosity and keep me engaged), but it looks good to me.

@anomiex anomiex merged commit c49fe68 into trunk Apr 22, 2025
85 checks passed
@anomiex anomiex deleted the fix/phpcs-phpunit-attributes-sniff branch April 22, 2025 19:45
@github-actions github-actions bot removed the [Status] Needs Review This PR is ready for review. label Apr 22, 2025
haqadn pushed a commit that referenced this pull request Apr 23, 2025
@anomiex @haqadn
phpcs: Fix and enable Jetpack.PHPUnit.Attributes sniff (#43175)
ba65c93 
Mostly this is the result of running phpcbf on files failing the sniff.

Things needing manual cleanup were:

* `projects/packages/image-cdn/tests/php/Image_CDN_Test.php`:
  Remove a broken `@covers` that had no space between the tag and class
  name.

* `projects/plugins/jetpack/tests/php/core-api/WPCOM_REST_API_V2_Field_Controller_Test.php`:
  Fix a `@group` that was accidentally
  "rest-api\n\nphpcs:disable Generic.Files.OneObjectStructurePerFile.MultipleFound".

* `projects/plugins/jetpack/tests/php/core-api/wpcom-fields/WPCOM_REST_API_V2_Attachment_VideoPress_Data_Test.php`:
* `projects/plugins/jetpack/tests/php/sync/Jetpack_Sync_Queue_TestBase.php`:
  Move some `@group`s from the file comment to class comment.

* `projects/plugins/jetpack/tests/php/Functions_OpenGraph_Test.php`:
* `projects/plugins/jetpack/tests/php/general/Jetpack_Client_Server_Test.php`:
  Fix where it got confused about whether a comment was a file or class
  comment.

* `projects/plugins/jetpack/tests/php/json-api/Jetpack_Json_Api_Plugins_Endpoints_Test.php`:
  Fix `@covers` for nonexistent class.

* `projects/plugins/jetpack/tests/php/modules/shortcodes/trait.http-request-cache.php`:
* `projects/plugins/jetpack/tests/php/trait-woo-tests.php`:
  Manually fix, since the sniff doesn't recognize the trait as a test
  class.

* Disable `PHPCompatibility.Attributes.NewAttributes.PHPUnitAttributeFound` in phpcompatibility-dev run.

`PHPCompatibility.Attributes.NewAttributes.Found` was already disabled.
haqadn added a commit that referenced this pull request Apr 24, 2025
@haqadn @donnchawp
@LiamSarsfield @kraftbj @matticbot @renovate-bot @anomiex @edanzer @johngodley @fgiannar @enejb @coder-karen @josephscott @jeherve
Boost: Handle cache gc in a scheduled event (#42954)
ed36f8f 
* We need to remove directories when rebuilding too

GC can delete rebuild files, leaving empty directories. Removing this
check makes sure those directories are deleted.

* Return directoroes in iterator too.

* changelog

* Add tests for Filesyste_Utils.php

* Fix the type of the $type parameter as static analysis checks failed

* Checks seem to be stalled. Kick them?

* Updating PHAN baseline

* Delete index.html files when deleting everything

If that file is left over, then the directory can't be deleted

* Refactor walk_directory and garbage collection

* Decouple file-storage from cache

* Reuse delete and rebuild path actions

* Fix this typo and it's one file

* Delete the cache file for this user as they left a comment

We need to delete this file instead of rebuild as the cache file is only
used by that particular user because of their cookies.

* Add $parameters parameter to rebuild and delete page functions

This allows those functions to delete a single page visited by a
particular visitor with cookies.

* Apply the action to the root $path too.

* Re-use some code

* Remove unused method

* Refactor the storage interface

* Refactor rebuilding

* Schedule a garbage collection when page output changes

* Fix comment approval check

* Update changelog

* Fix single garbage collection

* Finish cache to account for cache preloading

* Update dependency @mdn/browser-compat-data to v6 (#43199)

Co-authored-by: Renovate Bot <bot@renovateapp.com>

* Add support for sqlite in wordbless (#43197)

* phpcs: Fix and enable `Jetpack.PHPUnit.Attributes` sniff (#43175)

Mostly this is the result of running phpcbf on files failing the sniff.

Things needing manual cleanup were:

* `projects/packages/image-cdn/tests/php/Image_CDN_Test.php`:
  Remove a broken `@covers` that had no space between the tag and class
  name.

* `projects/plugins/jetpack/tests/php/core-api/WPCOM_REST_API_V2_Field_Controller_Test.php`:
  Fix a `@group` that was accidentally
  "rest-api\n\nphpcs:disable Generic.Files.OneObjectStructurePerFile.MultipleFound".

* `projects/plugins/jetpack/tests/php/core-api/wpcom-fields/WPCOM_REST_API_V2_Attachment_VideoPress_Data_Test.php`:
* `projects/plugins/jetpack/tests/php/sync/Jetpack_Sync_Queue_TestBase.php`:
  Move some `@group`s from the file comment to class comment.

* `projects/plugins/jetpack/tests/php/Functions_OpenGraph_Test.php`:
* `projects/plugins/jetpack/tests/php/general/Jetpack_Client_Server_Test.php`:
  Fix where it got confused about whether a comment was a file or class
  comment.

* `projects/plugins/jetpack/tests/php/json-api/Jetpack_Json_Api_Plugins_Endpoints_Test.php`:
  Fix `@covers` for nonexistent class.

* `projects/plugins/jetpack/tests/php/modules/shortcodes/trait.http-request-cache.php`:
* `projects/plugins/jetpack/tests/php/trait-woo-tests.php`:
  Manually fix, since the sniff doesn't recognize the trait as a test
  class.

* Disable `PHPCompatibility.Attributes.NewAttributes.PHPUnitAttributeFound` in phpcompatibility-dev run.

`PHPCompatibility.Attributes.NewAttributes.Found` was already disabled.

* phpunit: Enable reporting and failing on PHPUnit 11 deprecations (#43179)

Now that we've fixed all the PHPUnit 11 deprecations, adjust the configs
to enable `displayDetailsOnPhpunitDeprecations` and
`failOnPhpunitDeprecation`.

For plugins/crm, plugins/jetpack, and plugins/wpcomsh, there's one more
deprecation left that's also fixed in this PR: WP Core's obsolete
`WP_UnitTestCase_Base::checkRequirements()` function has a doc comment
that PHPUnit is mis-parsing to see a `@group` annotation that's not
actually intended. We can override it to stop PHPUnit from seeing that
doc comment.

* Update Eslint packages (#43200)

* Update Eslint packages
* Ignore another package with a build dep trying to `npm install` something
* Remove no-longer-needed eslint-disable directives

---------

Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Brad Jorsch <brad.jorsch@automattic.com>

* Forms: Change data structure for integrations endpoint (#43183)

* Gravatar: add Gravatar profiles to comments form (#42458)

* Add initial Gravatar comments

* Add changelog

* Revert webpack changes

Problem fixed in hovercard package

* Add i18n

* Use latest hovercard library

* Fix flex on email field

On simple sites it seems to be different

* Fix changelog?

* Bump to 0.8.0 Quick Editor

Track where interaction is coming from

* Lazy load Gravatar profile

* CSS lint fix

* Jetpack Sync: Switch 'jetpack_package_version' to a callable (#43188)

* Jetpack Sync: Switch 'jetpack_package_version' to a callable

* Gravatar comments: fix hovercard style (#43207)

* Gravatar: fix hovercard style

Ensure the hovercard style is loaded in the main CSS

* Add changelog

* Update/file field interactivity fixes (#43201)

* Image Compare block: Ensure compare line is not visible on top of a sticky menu (#43187)

* General: Bump minimum WordPress version to 6.7 (#43192)

* API Update Post: Fix warning when `$input['type']` does not exist (#43180)

* e2e Slack notifications: update file upload logic (#43196)

* e2e Slack notifications: update file upload logic

Slack will soon deprecate the files.upload API:
https://docs.slack.dev/changelog/2024-04-a-better-way-to-upload-files-is-here-to-stay/

We are encouraged to move to files.getUploadURLExternal and files.completeUploadExternal instead.
https://api.slack.com/methods/files.getUploadURLExternal
https://api.slack.com/methods/files.completeUploadExternal

* changelog

* Expand the tests to avoid coverage failure

* changelog: update wording.

Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>

* Update implementation to use @slack/web-api

See #43196 (review)

* Additional error check

See https://github.com/Automattic/jetpack/pull/43196/files#r2054583550

Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>

* Bring filesUploadV2 back into postOrUpdateMessage

We don't need to extract things anymore, since we simplified the upload in bf38b5f

Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>

* Update test

* Remove extra handling of errors

---------

Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>

* Fix extra parameter issue

* Remove unused const

---------

Co-authored-by: Donncha O Caoimh <5656673+donnchawp@users.noreply.github.com>
Co-authored-by: Liam Sarsfield <43409125+LiamSarsfield@users.noreply.github.com>
Co-authored-by: Brandon Kraft <public@brandonkraft.com>
Co-authored-by: Automattic Bot <sysops+ghmatticbot@automattic.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>
Co-authored-by: Brad Jorsch <brad.jorsch@automattic.com>
Co-authored-by: Erick Danzer <edanzer@gmail.com>
Co-authored-by: John <johng75@gmail.com>
Co-authored-by: Foteini Giannaropoulou <giannaropoulou.foteini@gmail.com>
Co-authored-by: Enej Bajgoric <enej.bajgoric@automattic.com>
Co-authored-by: Karen Attfield <karenlattfield@gmail.com>
Co-authored-by: Joseph Scott <joseph@josephscott.org>
Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tbradsha tbradsha tbradsha approved these changes

Assignees

@anomiex anomiex

Labels

Actions GitHub actions used to automate some of the work around releases and repository management Docs [Feature] Contact Form [Feature] Masterbar WordPress.com Toolbar and Dashboard customizations [Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins [Feature] Publicize Now Jetpack Social, auto-sharing [Package] Account Protection [Package] Admin Ui [Package] Assets [Package] Autoloader [Package] Backup [Package] Blaze [Package] Blocks [Package] Boost Speed Score [Package] Changelogger [Package] Codesniffer [Package] Connection [Package] Constants [Package] Device Detection [Package] Forms [Package] Ignorefile [Package] Image CDN [Package] IP [Package] Jetpack mu wpcom WordPress.com Features [Package] Jitm [Package] Masterbar [Package] My Jetpack [Package] Password Checker [Package] Patchwork Redefine Exit [Package] Phan Plugins [Package] Phpcs Filter [Package] Plans [Package] Post List [Package] Protect Status [Package] Publicize [Package] Roles [Package] Scheduled Updates [Package] Search Contains core Search functionality for Jetpack and Search plugins [Package] Stats Data [Package] Status [Package] Stub Generator [Package] Subscribers Dashboard [Package] Sync [Package] VideoPress [Package] WAF [Plugin] Boost A feature to speed up the site and improve performance. [Plugin] CRM Issues about the Jetpack CRM plugin [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Plugin] Social Issues about the Jetpack Social plugin [Plugin] Wpcomsh [Pri] Normal [Tests] Includes Tests [Type] Janitorial

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@anomiex @tbradsha