Drop the test tables from the database during cleanup - #337
Open
ekamran wants to merge 1 commit into
Open
Conversation
Data from a previous run can survive into the next one even though the test suite drops all tables before installing. WordPress boots before that drop and loads data such as user roles into memory, then writes it back into the fresh tables, so stale rows reappear. Wiping the tables between runs is the only reliable reset. See WordPress#110 for the original report, where stale role capabilities made the capability tests fail until the database was cleaned by hand. cleanup.php now drops every base table whose name starts with the test table prefix before removing the directories. The work is done by the new standalone cleanup-db.php script, which runs where the tests ran: piped over SSH to the remote PHP binary on stdin when a remote test environment is configured, or executed directly for local runs. The script reads the database credentials from wp-tests-config.php in the test directory, the same file the test suite used, so no password appears on a command line or in a process list. The file is parsed rather than included, because prepare.php prepends a logger with side effects to it. Matching by prefix also removes multisite sub-site tables and tables left behind by other branches or interrupted runs. LIKE wildcards in the prefix are escaped, and an empty prefix is refused so the cleanup can never touch unrelated tables. A cleanup failure is logged as a warning instead of stopping the run, so the directory cleanup still happens. Set WPT_SKIP_DB_CLEANUP to skip the database step entirely.
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #110
Stale rows survive between runs even though the test suite drops all tables before installing. WordPress boots in
tests/phpunit/includes/install.phpbefore that drop, loads data such as user roles into memory, and writes it back into the fresh tables. Reproduced on current trunk: one extra capability on the administrator role in the database produced the exact failure from the issue, and dropping the test tables fixed it.What this does
cleanup.phpnow drops every base table matching the test table prefix before removing the directories, so multisite sub-site tables likewptests_2_postsare covered and unrelated tables are never touched. The new standalonecleanup-db.phpruns where the tests ran: over stdin through SSH whenWPT_SSH_CONNECTis set, directly otherwise. It reads credentials fromwp-tests-config.phpin the test directory, so no password appears on any command line. A database failure is a warning, never a block on the directory cleanup.WPT_SKIP_DB_CLEANUPskips the step.The script is standalone because neither WordPress nor
functions.phpexists on the remote host, which is why it is the bulk of the diff. It parses the config rather than including it, sinceprepare.phpprepends a logger with side effects. DB_HOST handling is a port ofwpdb::parse_db_host(), verified against all 26 cases of the core data provider.The PHPUnit workflow gets a step that fails CI if any test table remains after cleanup.
#138 by @mrxkon showed the right approach in 2021, including the mysqli reasoning. This builds on it, adds the remote path and non-fatal error handling, and replaces the fixed table list with prefix matching. Independent of #336, which touches different lines of
cleanup.php.Testing
php -land PHPCS clean with the same invocation CI uses