Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filesystem API: Add initial tests for WP_Filesystem_Direct. #4106

Closed

Conversation

costdev
Copy link
Contributor

@costdev costdev commented Feb 21, 2023

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: https://core.trac.wordpress.org/ticket/57774

@costdev costdev marked this pull request as ready for review February 21, 2023 00:52
@costdev costdev force-pushed the wp_filesystem_direct_tests_initial branch 6 times, most recently from 3c544dc to dda0e95 Compare February 22, 2023 02:36
Comment on lines 131 to 132
// Note: setMethods() is deprecated in PHPUnit 9, but still supported.
->setMethods( array( 'dirlist' ) )
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note:

  • setMethods() is deprecated in PHPUnit 9 and removed in PHPUnit 10.
  • The suitable replacement for this case and others in this PR is onlyMethods(), introduced in PHPUnit 8.3.0.
  • However, as the WP Core test suite runs in PHP 5.6, this uses an earlier version of PHPUnit, so setMethods() must be used.
  • It may be preferable to add a PHP or PHPUnit version check here and run the appropriate method, so that compatibility with PHPUnit 10+ can be achieved without being coupled to the minimum WP Core version run in the test suite, or this could be done later to address all uses of setMethods() in the test suite in one go.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*
* @covers WP_Filesystem_Direct::chgrp
*/
class Tests_Filesystem_WpFilesystemDirect_Chgrp extends WP_Filesystem_Direct_UnitTestCase {
Copy link
Contributor Author

@costdev costdev Mar 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further testing requires more investigation for stable testing. This includes a source change to support detecting root as the user to prevent test failures in local test runners for methods such as ::owner() and ::group() when root is the user. The results of further investigation, and the above source change will be proposed in separate tickets/PRs.

🔢 Applies elsewhere in the tests / WP_Filesystem_Direct class.

@costdev costdev force-pushed the wp_filesystem_direct_tests_initial branch from dda0e95 to 1237eea Compare March 20, 2023 20:15
@costdev costdev force-pushed the wp_filesystem_direct_tests_initial branch from 1237eea to c855b82 Compare July 12, 2023 09:20
@costdev costdev force-pushed the wp_filesystem_direct_tests_initial branch from c855b82 to 76af012 Compare August 16, 2023 16:29
@costdev costdev force-pushed the wp_filesystem_direct_tests_initial branch from 76af012 to 60af67e Compare September 7, 2023 01:14
@costdev costdev force-pushed the wp_filesystem_direct_tests_initial branch from 60af67e to 29a9c65 Compare September 17, 2023 15:27
Copy link

github-actions bot commented Mar 2, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @[email protected].

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

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

Props swissspidy, costdev, mukesh27.

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

@swissspidy
Copy link
Member

@swissspidy swissspidy closed this Mar 2, 2024
Copy link

github-actions bot commented Mar 2, 2024

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

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants