Address PHPUnit Deprecations - #637
Draft
desrosj wants to merge 39 commits into
Draft
Conversation
Update dependabot.yml file.
Captures the brainstormed design for a GitHub Actions workflow that opens draft PRs to update package.json gutenberg.sha for trunk (latest public release) and the most recent version branch (daily, from wp/X.Y head). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…kflow. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…decouple trunk job. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…spatch for Gutenberg Sync. - Remove the dry_run input and all DRY_RUN handling. - Replace the auto-detecting setup job with a workflow-level CURRENTLY_SUPPORTED_BRANCH env var (mirrors test-old-branches.yml). - Replace the target choice input with a free-form branch input accepting "trunk" or an X.Y branch name; scheduled runs update trunk plus the currently supported branch, while a manual run targets one branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ch job. - Remove the schedule trigger; the workflow now runs only on workflow_dispatch. - Make the branch input required with a default of "trunk". - Merge the trunk and version-branch jobs into one "sync" job; trunk- and version-specific work is gated on inputs.branch, and the token, checkout, body-building, PR lookup, and PR create/update steps are shared. - Split steps to be atomic: resolve SHA, build PR body, look up the existing PR, and create/update the PR are now separate steps. - Drop the now-unused CURRENTLY_SUPPORTED_BRANCH env var. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… env vars. Define BASE_BRANCH and HEAD_BRANCH as workflow-level environment variables derived from the branch input, and remove the explicit determine/validate step. An invalid branch now fails at the checkout step, which is sufficient. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…upstream. WordPress/wordpress-develop does not accept pushed branches, so the workflow now runs from a fork: - Guard inverted to run only on forks (github.repository != upstream). - Check out the target branch from upstream so the PR is built on current content, push the gutenberg-sync/<branch> branch to the fork, and open the pull request from the fork branch back to upstream. - Drop the canonical-only GitHub App token (its private key cannot live on a fork). Push with the built-in GITHUB_TOKEN; open/label the upstream PR with an optional GUTENBERG_SYNC_TOKEN PAT. Without the PAT, the branch is still pushed and the run prints a link to open the PR manually. - Skip creating a duplicate when an open Gutenberg Sync PR already exists for the target branch from another fork. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…he wp/X.Y existence check. - Replace the manual git clone of WordPress/gutenberg with an actions/checkout step (fetch-depth: 0 so the changelog's git log range has history), and read the new SHA from the checked-out head. - Remove the custom "wp/<branch> does not exist" guard; if the branch is missing, the Gutenberg checkout fails on its own, which is sufficient. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mplate. Move the PR body structure into .github/workflows/gutenberg-sync-pr-body.md with __BRANCH__, __NEW_SHA__, __SOURCE_LINE__, and __CHANGELOG__ placeholders. The resolve steps now only emit the scalar values and write the changelog content to a file; a single awk step renders the template (inserting the changelog file verbatim so untrusted release notes are never re-substituted). The template is sparse-checked-out from the workflow's own ref so it is always available and version-matched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…beling. - Render the PR body by writing each placeholder into a working copy of the template as its value is resolved (sed for scalars, awk to insert the changelog verbatim). This removes the SOURCE_LINE environment variable and the separate body-building step. - Stop applying the label in the sync workflow and identify an existing PR by the gutenberg-sync/<branch> head branch name instead of the label. - Add gutenberg-sync-label.yml: on pull_request_target (opened/reopened) it labels any gutenberg-sync/* PR using upstream's token, which (unlike the fork's token) can label PRs on upstream. It never checks out PR code, so the pull_request_target trigger is safe (zizmor ignore documented inline). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… the template. - Move the body template to .github/template-gutenberg-sync-pr-body.md. - Break each resolve path into one action per step (copy template, fill each placeholder, retrieve release details / determine SHAs, build changelog). - Break the push into create-branch, update-file, stage, commit, add-remote, and push steps. - Split the pull request step into lookup, update-existing, and open-new. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n-branch jobs. Gate the branch selection at the job level (inputs.branch == 'trunk' vs != 'trunk') so the per-step branch conditionals are no longer needed. Each job runs end to end: checkout, resolve, push to the fork, and open/update the PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sable workflow. Add reusable-gutenberg-sync-pr.yml (workflow_call) that pushes the gutenberg-sync/<branch> branch to the fork and opens or updates the upstream pull request. The trunk and version-branch jobs now only resolve the new SHA, title, and body (uploaded as an artifact) and expose them as outputs; the open-pull-request job calls the reusable workflow with those values. This removes the push/PR steps that were duplicated across the two jobs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ranch conditions. Remove the reusable-gutenberg-sync-pr.yml workflow and the resolve/publish job split, returning to one sync job whose trunk- and version-specific steps are gated with inputs.branch conditions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y instead of opening a PR. The compare link cannot carry the generated body, so stop creating/updating the pull request (and drop the GUTENBERG_SYNC_TOKEN PAT). The workflow now pushes the branch to the fork and writes the PR title and body to the job summary, with the body in a fenced block for copy/paste into a manually opened pull request. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The reorganized workflow split resolve, body-building, and push into separate jobs, but the data did not cross the job boundaries. Fix that: - Expose research's resolved values (new/old SHA, title, tag, release URL, proceed) as job outputs and consume them downstream via needs.research.outputs. - Gate build-pr-body and create-update-branch at the job level on the proceed output instead of the per-step env.PROCEED checks, which were empty in those jobs. - Pass the changelog between jobs as an artifact. - Reorder research so the changelog is built after the SHAs are determined. - Build the PR body template from the fork (sparse checkout) and fix the pr_body.md filename and the missing changelog insertion for version branches. - Correct the create-update-branch permission (contents: write) and scope the read-only jobs to contents: read. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…a details block, and fix the SHA placeholder. - Add a list of changes (git log --reverse OLD..NEW, linkified) to every PR body, right after the intro line. Trunk now also checks out Gutenberg (at the release tag) so the list can be built, and the new SHA is read uniformly via git rev-parse. - For trunk, extract only the release's "## Changelog" section (which drops the heading and the First-time contributors/Contributors sections) and place it in a collapsed <details> labeled "Changelog"; remove the "## Changes" heading. - Pass the commit list and release changelog between jobs as artifacts. - Fix the __NEW_SHA__ replacement by giving the step its NEW_SHA env value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… sections. - Add a step to substitute the new __OLD_SHA__ placeholder. - Rename the first details summary to "Changes" (it holds the commit list and source link); the second remains "Changelog" for the release notes. - For version branches, remove the entire changelog <details> block rather than just the placeholder line, so the body has no empty collapsible. - Insert the release changelog directly now that the template supplies the <details> wrapper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ping. - Update the pull request titles and job-summary headings. - Escape the backticks in the PR titles so the shell treats them as literal text instead of command substitution (which blanked the code references). - Restore the blank lines around __CHANGELOG__ in the template so the release changelog renders as Markdown inside the <details> block. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e branch input. - Run the single Gutenberg checkout for both trunk and version branches, after the release details are retrieved (so the trunk ref resolves to the release tag), and target wp/X.Y for version branches. - Make the branch input a choice so only allowed values can be dispatched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…HPUnit methods. PHPUnit's TestCase::expectError()/expectErrorMessage() rely on PHPUnit converting PHP-native errors into catchable exceptions, a mechanism that has been soft-deprecated since PHPUnit 9.6 (triggering an internal "Expecting E_ERROR and E_USER_ERROR is deprecated" warning) and is no longer performed by default as of PHPUnit 10, where triggered errors are only reported, not thrown. On PHPUnit 10+ these two tests would silently stop verifying anything, since expectError() would never be satisfied. Both call sites are replaced with plain set_error_handler()/ restore_error_handler() blocks that capture the triggered error and assert against it directly, following the convention already established in Tests_User::test_wp_insert_user_with_null() (#61175). This works identically on PHPUnit 9, 10, 11, and 12. PHPUnit\Framework\Error\Error, the exception class expectError() checks for, is the base class of Warning, Notice, and Deprecated, so the previous expectError() calls loosely matched any of those levels rather than only E_ERROR/E_USER_ERROR. Neither of these two tests' functions under test actually triggers E_USER_ERROR: * wp_opcache_invalidate_directory() calls wp_trigger_error() without an explicit level, whose default is E_USER_NOTICE. * wp_strip_all_tags() explicitly calls wp_trigger_error() with E_USER_WARNING. The new set_error_handler() calls listen for these actual levels rather than E_USER_ERROR, so the migrated tests keep verifying the real behavior instead of merely happening to pass due to PHPUnit's loose class hierarchy. Props desrosj. Ticket #57375, #56434. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ith `set_error_handler()`. `TestCase::expectWarning()` and its `*Message()` companion rely on PHPUnit converting native PHP warnings into catchable exceptions, a mechanism that has been soft-deprecated since PHPUnit 9.6 and is no longer performed by default as of PHPUnit 10. Tests still using these methods would silently stop verifying anything on PHPUnit 10+, since no exception would ever arrive to satisfy the expectation. Replace each use with the version-agnostic pattern already established in `Tests_User::test_wp_insert_user_with_null()` (#61175): install a local `set_error_handler()` scoped to the relevant error level, capture the triggered errors, then assert against the captured array before restoring the previous handler. This works identically on PHPUnit 9, 10, 11, and 12. Some of the ported assertions needed a wider net than a single `expectWarning()` call implied: * `wp_user_settings()` triggers two "headers already sent" warnings, one per `setcookie()` call. * `rest_validate_value_from_schema()` and `rest_sanitize_value_from_schema()` read the same missing `'type'` schema array index repeatedly per call, and each test invokes the function twice, so those warnings now assert counts of 6 and 16 respectively rather than short-circuiting after the first one. In `Tests_Locale::test_get_weekday_undefined_index()`, `WP_Test_REST_Schema_Validation::test_format_validation_is_applied_if_missing_type()`, and `WP_Test_REST_Schema_Sanitization::test_format_validation_is_applied_if_missing_type()`, only the `PHP_VERSION_ID >= 80000` branch (native `E_WARNING`) was touched; the `expectNotice()` branch for PHP < 8 is left as-is. This is one part of preparing the test suite for compatibility with PHPUnit 10+, which no longer supports `expectWarning()`/`expectWarningMessage()`. Props desrosj. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…or_handler()`. PHPUnit's `expectNotice()` and `expectNoticeMessage()` rely on PHPUnit converting native PHP notices into catchable exceptions, a mechanism that has been soft-deprecated since PHPUnit 9.6 and is no longer performed by default as of PHPUnit 10. Continuing to rely on these methods means the affected tests would silently stop verifying anything once the suite is run under PHPUnit 10+, since no exception would ever be thrown to satisfy the expectation. Replaces each usage with a local `set_error_handler()`/`restore_error_handler()` block that captures the triggered notice(s) directly, following the same convention already established for `expectWarning()` in `Tests_User::test_wp_insert_user_with_null()` (see #61175). This works identically regardless of PHPUnit version. For `clean_dirsize_cache()`, `wp_unique_prefixed_id()`, and `wp_trigger_error()`, the notices are raised via `wp_trigger_error()` without an explicit error level, which defaults to `E_USER_NOTICE`. For `WP_Locale::get_weekday()` and the PHP-version-gated "undefined index" branches in the REST schema validation/sanitization tests, the notice is a native PHP "undefined index" notice (`E_NOTICE`) raised only on PHP < 8.0; on PHP 8.0+ the same access raises an `E_WARNING` instead, which is handled separately by the existing `expectWarning()` branch. Since these two test files are run exclusively under PHP >= 8.0 in this environment, the `E_NOTICE` branches could not be exercised directly, but were verified by inspection and by confirming an equivalent `E_WARNING` handler observes the expected number of triggers. Props desrosj. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PHPUnit's `TestCase::expectDeprecation()` (and its `expectDeprecationMessage()`/ `expectDeprecationMessageMatches()` companions) rely on PHPUnit converting PHP-native deprecation notices into catchable exceptions, a mechanism that has been soft-deprecated since PHPUnit 9.6 and is no longer performed by default as of PHPUnit 10 (triggered deprecations are only reported, never thrown). Calling these methods also emits its own PHPUnit-generated warning: "Expecting E_DEPRECATED and E_USER_DEPRECATED is deprecated and will no longer be possible in PHPUnit 10." On PHPUnit 10+, tests still using these methods wouldn't just print noise, they would silently stop verifying anything, since no exception would ever arrive to satisfy the expectation. Replace all 26 remaining `expectDeprecation()`/`expectDeprecationMessage()`/ `expectDeprecationMessageMatches()` call sites with the established `set_error_handler()`/`restore_error_handler()` pattern already used elsewhere in the test suite (see `Tests_User::test_wp_insert_user_with_null()` for ticket #61175), capturing triggered errors into an array and asserting against them directly. This works identically on PHPUnit 9, 10, 11, and 12. Notable details: * Several call sites (`Tests_Actions`, `Tests_Filters`, `Tests_Hooks_DoAction`, `Tests_Hooks_ApplyFilters`) trigger their deprecation during `add_action()`/`add_filter()` (float-to-int priority coercion), not during the later `do_action()`/`apply_filters()` call, so the error handler now wraps the full add/do sequence rather than just the final call. * `Tests_Formatting_wpTrimExcerpt::test_should_generate_excerpt_for_empty_values()` and `Tests_Admin_IncludesPost::test_post_exists_should_not_match_invalid_post_type_and_status_combined()` previously relied on PHPUnit's exception-based short-circuit to skip code after the triggering call; that code now genuinely executes, so the assertions were adjusted to verify it rather than assume it's unreached. * `Tests_HTTP_IncludeOldRequestsClass::test_should_include_old_requests_class()` used `new Requests()` purely to trigger the class's autoloader; the `WpOrg\Requests\Requests` parent class has a private constructor, so actual instantiation was never reachable before (the exception-based short-circuit masked this). Switched to `class_exists( 'Requests' )` to trigger the autoloader without attempting to construct the class. Deliberately left untouched: `setExpectedDeprecated()`, `setExpectedIncorrectUsage()`, `deprecated_function_run()`, `doing_it_wrong_run()` (WordPress's own deprecation tracking), and all `expectWarning()`/`expectNotice()`/`expectError()` call sites, which are a separate, unrelated PHPUnit-native-API cleanup. Props desrosj. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
desrosj
force-pushed
the
trunk
branch
2 times, most recently
from
August 28, 2026 12:51
8bc359a to
5d7de36
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trac ticket:
Use of AI Tools
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.