Make WordPress Core

Opened 13 months ago

Closed 24 hours ago

Last modified 24 hours ago

#57774 closed task (blessed) (fixed)

Add some initial PHPUnit tests for `WP_Filesystem_Direct`

Reported by: costdev's profile costdev Owned by: hellofromtonya's profile hellofromTonya
Milestone: 6.5 Priority: normal
Severity: normal Version: 2.5
Component: Build/Test Tools Keywords: has-patch has-unit-tests commit
Focuses: Cc:


This is a tests-only ticket.

WP_Filesystem_Direct is, by far, the most used filesystem abstraction class.

While some PHPUnit tests exist for a WP_Filesystem_MockFS class, this ticket aims to add PHPUnit tests for the WP_Filesystem_Direct class itself.

The other filesystem abstraction classes, WP_Filesystem_FTPext, WP_Filesystem_ftpsockets and WP_Filesystem_SSH2, do not have PHPUnit test runner configurations. While in future, we may reuse many of the same tests across filesystem abstraction classes, this ticket specifically targets WP_Filesystem_Direct.

Change History (23)

This ticket was mentioned in PR #4106 on WordPress/wordpress-develop by @costdev.

13 months ago

  • Keywords has-unit-tests added

This adds some initial tests for the WP_Filesystem_Direct filesystem abstraction class.

These tests do not aim to cover all paths, but to add line/branch coverage where source changes are not needed to facilitate testing or resolve bugs. The intention is to make it easier to know how to hit a line/branch when trying to cover paths in future.

This also introduces the WP_Filesystem_Direct_UnitTestCase class, which contains shared properties, set_up()/tear_down(), helpers and common data providers for the tests.

To allow constants to be set without affecting other tests in the test suite, some of these tests use the @runInSeparateProcess annotation [Ref]. To prevent an exception for Serialization of 'Closure' is not allowed, the @preserveGlobalState disabled annotation is also used [Ref].

Tests exist for the following methods:

  • __construct()
  • get_contents()
  • get_contents_array()
  • put_contents()
  • cwd()
  • chdir()
  • chgrp()
  • chmod()
  • chown()
  • getchmod()
  • copy()
  • move()
  • delete()
  • exists()
  • is_file()
  • is_dir()
  • is_readable()
  • is_writable()
  • atime()
  • mtime()
  • size()
  • touch()
  • mkdir()
  • rmdir()
  • dirlist()

Trac ticket:

#2 @costdev
13 months ago

PR 4106 is ready for review.

Line coverage: 74.56%
Branch coverage: 76.83%

To cover more lines/branches, source changes are needed to facilitate testing/fix bugs. This will be done in separate tickets.

Last edited 13 months ago by costdev (previous) (diff)

#3 @hellofromTonya
13 months ago

  • Owner changed from costdev to hellofromTonya
  • Status changed from assigned to reviewing

Self-assigning for review and commit consideration.

This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.

12 months ago

#5 @costdev
12 months ago

  • Milestone changed from 6.2 to 6.3

This ticket was discussed in the bug scrub.

While this is a tests-only ticket and can be committed at any time, we're approaching RC1 and the PR is awaiting review, so I'm going to move this ticket to the 6.3 milestone.

Should it be reviewed and ready for committed prior to 6.2 RC1, feel free to bring it back into the 6.2 milestone for commit.

Additional props: @mukesh27

#6 @oglekler
8 months ago

@hellofromTonya, please take a look at this one 🙏

This ticket was mentioned in Slack in #core by chaion07. View the logs.

8 months ago

#8 @swissspidy
8 months ago

  • Milestone changed from 6.3 to Future Release
  • Type changed from defect (bug) to enhancement

#9 @hellofromTonya
7 months ago

  • Milestone changed from Future Release to 6.4

Moving into the 6.4 milestone.

@costdev sorry for the big delay. I'll get your PR reviewed and then committed.

#10 @oglekler
6 months ago

  • Keywords needs-refresh added

The patch needs to be refreshed, most likely the part in phpcs.xml.dist is not needed anymore, see:

Great job, btw. Let's try to make it into the current milestone :)

#11 @costdev
6 months ago

  • Keywords needs-refresh removed

Thanks @oglekler!

PR 4106 has now been rebased on trunk and the conflict with phpcs.xml.dist resolved.

This ticket was mentioned in Slack in #core by oglekler. View the logs.

5 months ago

#13 @oglekler
5 months ago

This ticket was discussed during bug scrub.

@hellofromTonya can you please review this PR?

Add props to @mukesh27

This ticket was mentioned in Slack in #core by oglekler. View the logs.

5 months ago

#15 @hellofromTonya
5 months ago

As this ticket is for adding automated tests, it is not bound to the 6.4 release milestones and can be committed at any time before the 6.4 Dry Cycle starts.

#16 @hellofromTonya
5 months ago

  • Component changed from Filesystem API to Build/Test Tools

To avoid confusion for 6.4 triagers during the beta cycle, I changed the component to Build/Test as this ticket tracks only tests.

#17 @hellofromTonya
5 months ago

  • Milestone changed from 6.4 to 6.5

While this ticket is not bound to a major release's schedule, tomorrow RC1 ships and trunk will be branched to open it for 6.5-alpha. Thus, given the short time remaining in 6.4, moving this to 6.5.

What's left? Code review and then commit. If that can get done during 6.4 RC cycle, could consider backporting the tests to the 6.4-branch.

#18 @swissspidy
3 weeks ago

  • Type changed from enhancement to task (blessed)

#19 @hellofromTonya
3 weeks ago

Hey @costdev, we previously chatted about the patch's readiness. Fast wind to today, is it ready for code and commit review?

#20 @costdev
3 weeks ago

Hey @hellofromTonya! Yes I believe the PR should be ready for code and commit review.

#21 @swissspidy
6 days ago

  • Keywords commit added

#22 @swissspidy
24 hours ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 57753:

Build/Test Tools: Add initial tests for the WP_Filesystem_Direct class.

Since WP_Filesystem_Direct is by far the most used filesystem abstraction class, this facilitates future changes with sufficient test coverage.

Props swissspidy, costdev, mukesh27.
Fixes #57774.

Note: See TracTickets for help on using tickets.