Add unit tests for got_mod_rewrite() in src/wp-admin/includes/misc.php by pbearne · Pull Request #11656 · WordPress/wordpress-develop

pbearne · 2026-04-27T18:04:56Z

Reference: https://core.trac.wordpress.org/ticket/65134

This PR adds comprehensive unit tests for the got_mod_rewrite() function in src/wp-admin/includes/misc.php.
The tests cover:

Default behaviour based on Apache module detection.
Overriding behaviour using the got_rewrite filter.

Trac ticket: https://core.trac.wordpress.org/ticket/65134

Use of AI Tools

AI assistance: Yes
Tool(s): Junie (JetBrains)
Model(s): gemini-3-flash-preview
Used for: Identifying untested functions, generating the test skeleton, and implementing the dataProvider logic. All code was reviewed and verified against WordPress core testing patterns.

github-actions · 2026-04-27T18:05:07Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs.

Core Committers: Use this line as a base for the props when committing in SVN:

Props pbearne, westonruter, lancewillett.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

@propsbot add <username> to give props to a GitHub user. Requires write access.
@propsbot remove <username> to leave a user out of the props. Requires write access.
@propsbot refresh to refresh this list. Adding the props-bot label works too.

github-actions · 2026-04-27T18:09:22Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Reference: https://core.trac.wordpress.org/ticket/65134

westonruter · 2026-04-28T05:36:39Z

+		// without a framework, we rely on the filter for full control.
+
+		if ( null !== $filter_value ) {
+			add_filter( 'got_rewrite', array( $this, 'filter_got_rewrite' ) );


I think a closure would make more sense here. No need for a method and no need for $this->temp_filter_value

westonruter · 2026-04-28T05:37:34Z

+	 * @return array[] {
+	 *     @type bool      $expected      The expected result.
+	 *     @type bool      $apache_loaded Whether mod_rewrite is loaded (simulated via filter).
+	 *     @type bool|null $filter_value  The value to return from the filter.
+	 * }


Let's use PHPStan array shape syntax since there's no need to target WordPress Developer docs.

Not sure of the ask here
So I asked AI to help
Did it get it right?

Yeah. That looks right. We can remove the old one though. Added a suggestion.

…ne/wordpress-develop into 65134-misc-got_mod_rewrite

Co-authored-by: Weston Ruter <westonruter@gmail.com>

Copilot

🟡 Changes recommended

Every dataset overrides Apache detection with a non-null filter value, leaving got_mod_rewrite()’s unfiltered default behavior untested and allowing an incorrect constant result to pass.

1 open issue

Add an unfiltered test for default Apache module detection · New

What changed in this PR

Adds PHPUnit coverage for the WordPress admin helper got_mod_rewrite(), supporting Trac ticket #65134.

Changes:

Introduces a data-driven test for boolean overrides through the got_rewrite filter.
Adds admin/rewrite group annotations and function coverage metadata.

File	Description
`tests/phpunit/tests/admin/includes/misc/gotModRewrite.php`	Adds the test class and three filter-driven datasets for `got_mod_rewrite()`.

_{Review effort: Balanced}

lancewillett

The earlier scope and closure requests are addressed. The affected classes passed randomized single-site and multisite runs. Optional: remove the unused apache_loaded column and duplicate true row, and describe these as final-filter checks rather than Apache detection. Copilot’s detection suggestion need not become another delegation test. One CI job is cancelled, so this is code approval, not a claim that the full matrix completed.

_{AI review · gpt-6}

lancewillett · 2026-10-31T18:13:12Z

I tried rerunning the cancelled PHPUnit job, but GitHub rejected the request because the run is over a month old. Its annotation records a 20-minute timeout; the log is no longer available to identify the slow step. Please refresh the branch against current trunk to get a fresh CI result before landing. The existing code approval and optional cleanup feedback remain unchanged.

_{AI review · gpt-6}

Copilot

🟡 Changes recommended

Every dataset overrides the detected value without configuring server state, leaving the default Apache detection path untested and allowing detection regressions to pass.

2 open issues

_{Review effort: Balanced}

lancewillett · 2026-10-31T18:58:37Z

@westonruter The closure and array-shape changes you requested are now present, and the refreshed CI is green. Your changes-requested review and two outdated threads are still open. Could you confirm whether the current patch addresses your review? I’m holding the SVN landing for that confirmation.

_{AI review · gpt-6}

Co-Authored-By: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

Previously every dataset installed a `got_rewrite` filter that replaced the detected value, and the `apache_loaded` parameter never configured any server state, so a constant detection result would have passed all cases. Each dataset now sets the `$is_apache` global (restored in `tear_down()`) and asserts the unfiltered result before adding the filter, then asserts that the filter overrides it. The four datasets cover both detection outcomes, each overridden to true and to false. The separate test of the unfiltered result is dropped, since the datasets cover it now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Outside Apache's own process, apache_get_modules() is unavailable, so on Apache got_mod_rewrite() returns the `true` default rather than the result of real module detection. Say so in the comment, the assertion message, and the test summary, so they don't overstate what is covered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The callback previously ignored its argument, so the test would still pass if got_mod_rewrite() passed the wrong value to the filter. Capture the value the callback receives and assert that it matches the detection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This matches set_up(), which documents the `$is_apache` global it backs up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

westonruter

@lancewillett LGTM

Copilot

🟢 Approval recommended

No active review issues are recorded for the added got_mod_rewrite() unit tests.

No open issues 🎉

2 resolved since last review

_{Review effort: Balanced}

lancewillett

Weston’s approval covers the current head, and the previous review requests are addressed. The test now checks the unfiltered server result, the filter override, and the value passed into the filter.

The four new provider cases passed alongside two existing rewrite checks in randomized single-site and multisite runs: six tests, 14 assertions per mode. PHP lint and coding standards pass, and current CI is green. This checks the documented CLI fallback, not actual Apache module discovery.

_{AI review · gpt-6}

lancewillett · 2026-10-31T21:19:40Z

PR #11656 landed in https://core.trac.wordpress.org/changeset/65156

Verify `got_mod_rewrite()` uses the server flag for its CLI fallback and passes that detected value to `got_rewrite`. Check that the filter can override the result and restore the server flag after each test. Developed in: #11656 Props pbearne, westonruter. Fixes #65134. git-svn-id: https://develop.svn.wordpress.org/trunk@65156 602fd350-edb4-49c9-b593-d223f7449a82

Verify `got_mod_rewrite()` uses the server flag for its CLI fallback and passes that detected value to `got_rewrite`. Check that the filter can override the result and restore the server flag after each test. Developed in: WordPress/wordpress-develop#11656 Props pbearne, westonruter. Fixes #65134. Built from https://develop.svn.wordpress.org/trunk@65156 git-svn-id: http://core.svn.wordpress.org/trunk@64305 1a063a9b-81f0-0310-95a4-ce76da25c4cd

pbearne force-pushed the 65134-misc-got_mod_rewrite branch 2 times, most recently from 0f781a9 to c3cb218 Compare April 27, 2026 18:31

Add unit tests for got_mod_rewrite() in src/wp-admin/includes/misc.php

7f341bd

Reference: https://core.trac.wordpress.org/ticket/65134

pbearne force-pushed the 65134-misc-got_mod_rewrite branch from 78e67a3 to 7f341bd Compare April 27, 2026 18:45

Ensure empty marker lines are excluded in src/wp-admin/includes/misc.php

53ae4e0

westonruter requested changes Apr 28, 2026

View reviewed changes

pbearne force-pushed the 65134-misc-got_mod_rewrite branch 2 times, most recently from 52f84ae to 17c6cad Compare April 28, 2026 19:42

reverted blank line check src/wp-admin/includes/misc.php

1f73812

pbearne force-pushed the 65134-misc-got_mod_rewrite branch from 17c6cad to 1f73812 Compare April 28, 2026 19:46

Declare return type for data_got_mod_rewrite() in unit test.

84a37f5

pbearne force-pushed the 65134-misc-got_mod_rewrite branch from 9532b95 to 84a37f5 Compare April 28, 2026 19:57

Paul Bearne and others added 4 commits April 28, 2026 15:58

Remove unused tear_down() method in gotModRewrite unit test.

719bd4d

Merge branch 'trunk' into 65134-misc-got_mod_rewrite

0dae5c5

Simplify inline closure in got_rewrite filter within unit test.

c25af28

Merge branch '65134-misc-got_mod_rewrite' of https://github.com/pbear…

70d2ce4

…ne/wordpress-develop into 65134-misc-got_mod_rewrite

westonruter reviewed Apr 28, 2026

View reviewed changes

Comment thread tests/phpunit/tests/admin/includes/misc/gotModRewrite.php Resolved

westonruter reviewed Apr 28, 2026

View reviewed changes

Comment thread tests/phpunit/tests/admin/includes/misc/gotModRewrite.php Resolved

Apply suggestions from code review

9513cbc

Co-authored-by: Weston Ruter <westonruter@gmail.com>

pento force-pushed the trunk branch from 3af2d27 to 64ede28 Compare July 6, 2026 03:10

lancewillett requested a balanced review from Copilot October 31, 2026 00:22

Copilot started reviewing on behalf of lancewillett October 31, 2026 00:22

View session

Copilot AI reviewed Oct 31, 2026

View reviewed changes

Comment thread tests/phpunit/tests/admin/includes/misc/gotModRewrite.php Resolved

lancewillett approved these changes Oct 31, 2026

View reviewed changes

Merge branch 'trunk' into 65134-misc-got_mod_rewrite

ab35763

lancewillett requested a balanced review from Copilot October 31, 2026 18:49

Copilot started reviewing on behalf of lancewillett October 31, 2026 18:50

View session

Copilot AI reviewed Oct 31, 2026

View reviewed changes

Comment thread tests/phpunit/tests/admin/includes/misc/gotModRewrite.php Resolved

westonruter and others added 12 commits October 31, 2026 13:16

Merge branch 'trunk' into 65134-misc-got_mod_rewrite

c9ce208

Fix alignment of params

795ba21

Add native type hints

88952df

Link data provider to test

0319df2

Add test case

f536a09

Co-Authored-By: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

Use native property hint

f7975d5

Account for $is_apache maybe being null

01f8d89

Add docblock to tear_down()

c3f5467

This matches set_up(), which documents the `$is_apache` global it backs up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Add void return type to test method

e29b11a

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

westonruter approved these changes Oct 31, 2026

View reviewed changes

lancewillett requested a balanced review from Copilot October 31, 2026 21:15

Copilot started reviewing on behalf of lancewillett October 31, 2026 21:15

View session

Copilot AI reviewed Oct 31, 2026

View reviewed changes

lancewillett approved these changes Oct 31, 2026

View reviewed changes

lancewillett closed this Oct 31, 2026

Conversation

pbearne commented Apr 27, 2026

Use of AI Tools

Uh oh!

github-actions Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 27, 2026

Test using WordPress Playground

Some things to be aware of

Uh oh!

westonruter Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

pbearne Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

westonruter Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

pbearne Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

westonruter Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

lancewillett left a comment

Choose a reason for hiding this comment

Uh oh!

lancewillett commented Oct 31, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

lancewillett commented Oct 31, 2026

Uh oh!

westonruter left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

lancewillett left a comment

Choose a reason for hiding this comment

Uh oh!

lancewillett commented Oct 31, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Apr 27, 2026 •

edited

Loading