Limit writable directories required by WordPress unit test suite

[danielbachhuber]

Some environments only permit writing to specific directories. But, the WordPress test suite currently assumes it can write to a variety of directories.

Based on searches for file_put_contents(), copy(), and symlink(), here are the directories I found:

• tests/phpunit/data
  • .trac-ticket-cache.* is created to store a cache of Trac ticket numbers.
• tests/phpunit/build/logs/
  • junit.xml is created with the results of the test run.
• tests/qunit/fixtures
  • wp-api-generated.js is generated by PHPUnit for use in QUnit tests.
• /tmp/
  • Variety of files are copied out of phpunit/data into /tmp/ for test-specific transformation.
• wp-content/themes
  • theme-file-parent and theme-file-child are symlinked from the PHPUnit data directory into the theme directory
• wp-content/uploads
  • Attachments are copied

Generally, in write-restricted environments, the test suite fails when it tries to write to directories it doesn't have access to. More directories could be found by running the test suite against a write-restricted environment.

To better ensure WordPress compatibility with a variety of platforms, we should ensure these file write requirements are compatible with limited write directories. Here are some strategies we can use:

• Properly use wp_tempnam() instead of hardcoding /tmp/ paths.
• Permit some write directories to be configurable with a constant or environment variable.
• Failing gracefully, in certain cases, when a file can't be written.

cc @mikeschroder @octalmage

[jnylen0]

For your first three examples, at least, shouldn't we always be able to assume that anything under the source checkout of WordPress is writable?

Otherwise, this implies that two different users did the source checkout vs. the test run, which seems like a very odd situation (a non-writable source checkout is probably not all that useful for development-related tasks like running the test suite).

[danielbachhuber]

Replying to jnylen0:

For your first three examples, at least, shouldn't we always be able to assume that anything under the source checkout of WordPress is writable?

Not necessarily, if we want to run the test suite against production-esque environments for a variety of hosting companies, and report the results back to WordPress.org.

For instance, in Git mode, Pantheon delivers the source files to the environment via Git, and only the uploads directory is writable.

Similarly, other hosts restrict write access to the wp-content directory.

[jnylen0]

.trac-ticket-cache.* is created to store a cache of Trac ticket numbers.

I really, really wish we would just get rid of these - our test suite should not be calling out to our issue tracker.

wp-api-generated.js is generated by PHPUnit for use in QUnit tests.

It should be fine to ignore a failure to update this file, as wp-api-generated.js is checked into source control and is not usually updated.

[netweb]

Replying to danielbachhuber:

• Properly use wp_tempnam() instead of hardcoding /tmp/ paths.

There's a patch for this in #39975, it's using get_temp_dir() instead of wp_tempnam()

[dd32]

Replying to jnylen0:

.trac-ticket-cache.* is created to store a cache of Trac ticket numbers.

I really, really wish we would just get rid of these - our test suite should not be calling out to our issue tracker.

Originally I believe this was for skipping known-failing test cases for open tickets, although I think it may have been repurposed over the years (I'm not quite sure). Since we now only commit tests that are expected to be operational that functionality can probably be removed from the test runner..

[netweb]

Replying to dd32:

Replying to jnylen0:

.trac-ticket-cache.* is created to store a cache of Trac ticket numbers.

I really, really wish we would just get rid of these - our test suite should not be calling out to our issue tracker.

Originally I believe this was for skipping known-failing test cases for open tickets, although I think it may have been repurposed over the years (I'm not quite sure).

Most recent work in this area was done in #30284

[andraganescu]

Reading through #30284 it appears that the original intention was to remove all the .trac-ticket-cache.* but apparently it got omitted.

Indeed we should use wp_tempnam() instead of hardcoding /tmp/ paths and failing gracefully, seems a sensible idea.

Also, what do you mean by "permitting some write directories to be configurable with a constant or environment variable", I'm not sure I understand what this means. Do you have a specific case in mind for this?