Opened 5 years ago
Last modified 5 years ago
#50209 new enhancement
Remove all uploads between test methods
Reported by: | Frank Klein | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 4.0 |
Component: | Build/Test Tools | Keywords: | needs-patch |
Focuses: | Cc: |
Description
Commit r29120 added a curious feature to the WordPress testing framework: selective uploads deletion between test runs.
What it means is that the first time that WP_UnitTestCase::setUp()
is called for a test case, it will create a list of all files contained in the uploads directory. This list, stored in the static $ignore_files
property, allows to selectively delete files uploaded during tests.
This code was added so that tests can be run against a development installation of WordPress. In opposition to running tests against a throwaway installation, as should be the case.
There are three issues with this behavior:
- Scanning the uploads directory is slow. If you have too many files in that directory, the test runner will "hang" before running the first test. Ultimately it will result in a memory exhaustion error.
- Attachments are not properly deleted.
_delete_all_data()
runs a SQL query that deletes the database entries of attachments, but not the associated files. Since the function is called beforewpTearDownAfterClass
runs, there's no way to do manual cleanups in tests. The next test case that gets booted up will add the remaining files to the list of files to ignore, making it impossible to do selective uploads deletion. See #41978. - There is no automatic clean up of uploads between test methods or test cases, leaving files lingering around. Even after all tests have been run.
In short this approach is slow, half-baked, and unneeded. We should therefore remove it, and have uploads be cleared the same as any other fixture.
Since this behaviour has been around for years now, we can't unfortunately just remove it, so I propose that we use a feature flag for disabling it.
Change History (4)
#2
in reply to:
↑ 1
@
5 years ago
Using a different uploads directory would solve this problem. We would need to document this change well though. Some developers might rely on the default directory name in their tests.
In terms of cleanup, we need to clean uploads:
- After every test: to ensure that all tests start with a clean slate.
- Before every test suite run: to ensure that if cleanup didn't happen due to test runner issues, the first test still has a clean slate. Because if a previous test run got stopped due to a PHP error, no cleanup would have happened.
Could we fix this by specifying a different uploads directory during test runs that gets nuked upon completion?