[OlegApanovich]

Points the test suite at a dedicated wp-content/test-suite-uploads directory instead of the real wp-content/uploads, so test runs don't write to actual site content.

The directory wp-content/test-suite-uploads is deleted once before new tests run.

Adds an _upload_get_url() helper and updates tests that previously hardcoded wp-content/uploads.

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

Use of AI Tools

AI assistance: Yes
Tool(s): Claude
Model(s): GPT-5.1
Used for: Only for simple, repetitive actions; all architectural decisions are mine alone.

[github-actions]

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 mrdollar4444, 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]

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.

[lancewillett]

Please preserve the option-based upload tests: defining UPLOADS globally makes the relative-path, absolute-path and custom-URL cases exercise the override instead. Their original behavior needs to remain covered.

The new lock also fails when wp-content/uploads does not exist: touch() returns false, leaving deletion enabled without a marker. Please ensure successful lock creation before relying on it. A date hash also does not distinguish concurrent runs.

Finally, please run the ms-files group explicitly. Its main-site rewriting scenario ignores UPLOADS in production, so changing its expectation to test-suite-uploads does not match that path. The default multisite configuration excludes this group.

_{AI review · gpt-6}

[Copilot]

🟡 Changes recommended

Legacy multisite uploads remain outside the test directory and conflict with new assertions, while lock handling and filesystem initialization can delete fixtures or abort bootstrap.

4 open issues

• Isolate legacy multisite upload paths in the fixtures · New
• Use a per-run lock ID shared with subprocesses · New
• Fail safely when the uploads lock cannot be created · New
• Use the direct filesystem for local test uploads · New

What changed in this PR

This PR moves PHPUnit uploads to a dedicated test directory to keep test runs from writing to normal site uploads.

Changes:

• Defines UPLOADS and adds lock-based cleanup during test bootstrap.
• Adds _upload_get_url() and updates upload-related fixtures and assertions.
• Excludes the test uploads directory from version control.

File	Description
tests/​phpunit/​tests/​upload.php	Updates upload path and URL expectations.
tests/​phpunit/​tests/​speculative-loading/​wpGetSpeculationRules.php	Uses UPLOADS in exclusion patterns.
tests/​phpunit/​tests/​multisite/​site.php	Updates site-switching upload expectations.
tests/​phpunit/​tests/​multisite/​msFilesRewriting.php	Changes legacy multisite main-site expectations.
tests/​phpunit/​tests/​media/​previousImageLink.php	Uses the helper for previous-image URLs.
tests/​phpunit/​tests/​media/​nextImageLink.php	Uses the helper for next-image URLs.
tests/​phpunit/​tests/​media/​getPreviousImageLink.php	Updates expected previous-image markup.
tests/​phpunit/​tests/​media/​getPostGalleries.php	Updates gallery URLs and expected output.
tests/​phpunit/​tests/​media/​getNextImageLink.php	Updates expected next-image markup.
tests/​phpunit/​tests/​media/​getAdjacentImageLink.php	Updates expected adjacent-image markup.
tests/​phpunit/​tests/​media.php	Replaces hardcoded media upload URLs.
tests/​phpunit/​tests/​fonts/​font-library/​wpRestFontFacesController.php	Updates the expected font upload path.
tests/​phpunit/​tests/​fonts/​font-library/​wpFontsDir.php	Derives font expectations from the switched site's uploads.
tests/​phpunit/​includes/​functions.php	Adds the shared upload URL helper.
tests/​phpunit/​includes/​bootstrap.php	Configures test uploads and startup cleanup.
.gitignore	Ignores the dedicated test uploads directory.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

The inherited setup creates real upload directories before options are changed, so this path-only test class needs an upload-scan override to preserve filesystem isolation.

1 open issue

• Prevent upload scans from creating real directories in isolated tests · New

4 resolved since last review

• Isolate legacy multisite upload paths in the fixtures
• Use the direct filesystem for local test uploads
• Fail safely when the uploads lock cannot be created
• Use a per-run lock ID shared with subprocesses

_{Review effort: Balanced}

[OlegApanovich]

Please preserve the option-based upload tests: defining UPLOADS globally makes the relative-path, absolute-path and custom-URL cases exercise the override instead. Their original behavior needs to remain covered.

The new lock also fails when wp-content/uploads does not exist: touch() returns false, leaving deletion enabled without a marker. Please ensure successful lock creation before relying on it. A date hash also does not distinguish concurrent runs.

Finally, please run the ms-files group explicitly. Its main-site rewriting scenario ignores UPLOADS in production, so changing its expectation to test-suite-uploads does not match that path. The default multisite configuration excludes this group.

@lancewillett
Thanks for the review! All three points are addressed:

1. Option-based tests: uploadDirOptions.php runs these cases in separate processes without UPLOADS, so the original behavior is still covered. Ideally, _wp_upload_dir() would be split into smaller functions that could be tested on their own, but I don't take a risk do that in this PR cos it change core code, though, so I kept this PR to a test-level fix.
2. Lock: I replaced the file lock with a per-run environment variable that child processes inherit. It's simpler and shorter than the file-based version.
3. ms-files: I reverted the main-site expectation in msFilesRewriting.php. The group passes when run explicitly, and it's green in CI.

Could you please take another look?

[Copilot]

🔵 Needs a closer look

Unavailable test dependencies prevented runtime verification of the upload-option tests’ parent/child class-setup lifecycle and once-per-run cleanup across supported PHPUnit versions.

No open issues 🎉

1 resolved since last review

• Prevent upload scans from creating real directories in isolated tests

_{Review effort: Balanced}

[lancewillett]

The dedicated upload directory and the option/ms-files fixtures are useful. Please remove the recursive startup deletion and its WP_TESTS_UPLOADS_RUN_ID bookkeeping unless there is an existing failing case that requires them. The current runs establish compatibility, but do not show a consumer that needs the directory purged. Keep the upload-path override and separate-process option coverage; no additional synthetic test or locking mechanism is needed.

The isolated option tests pass in single-site and multisite, and the explicit ms-files tests pass. The broader affected selection has the same font-face test error on baseline and patch. This request concerns cleanup necessity, not a demonstrated regression.

_{AI review · gpt-6}

[lancewillett]

I investigated the startup cleanup further and can now demonstrate its benefit. This supersedes my request to remove it.

I stopped an existing media test immediately after its class fixture created the image files, then reran the unchanged tests from the same saved files. The dedicated UPLOADS path stayed enabled; only startup deletion was toggled.

In both single-site and multisite, Tests_Media::test_wp_get_attachment_image_should_use_wp_get_attachment_metadata:

• passes with a clean directory and deletion disabled;
• fails with interrupted-run leftovers and deletion disabled (test-image-large-1-150x150.jpg instead of test-image-large-150x150.jpg);
• passes with those same leftovers and deletion enabled.

This reproduces the filename-collision problem described in #63532 using real class fixtures. The full media selection also passes with cleanup in both modes: 368 tests / 1,193 assertions, with one existing skip. No test assertions were changed or new test methods added. The earlier isolated upload-option and explicit ms-files checks passed too; the broader font-face error reproduced on baseline and patch.

Startup cleanup therefore has demonstrated value for recovery after an interrupted run. This does not establish support for independently concurrent runs sharing the directory. Current-head CI is green.

_{AI review · gpt-6}

[github-actions]

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 65177
GitHub commit: 2700c1f

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.