WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#37046 closed defect (bug) (fixed)

Unit test initial setup fails if plugins added tables with foreign key constraints

Reported by: javorszky Owned by: pento
Milestone: 4.6 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
Focuses: Cc:

Description

When setting up a new test run, the script would drop all tables and recreate them from scratch. See here: https://core.trac.wordpress.org/browser/trunk/tests/phpunit/includes/install.php?marks=43-45#L43

Relevant code copied:

foreach ( $wpdb->tables() as $table => $prefixed_table ) {
	$wpdb->query( "DROP TABLE IF EXISTS $prefixed_table" );
}

If there's a foreign key constraint on a table that has not been dropped yet that references one that's being dropped, MySQL (and MariaDB) will return an error about the foreign key constraint failing, and won't actually drop the table.

This can lead to dirty state for the next test.

Attachments (1)

37046.patch (732 bytes) - added by javorszky 4 years ago.
Patch disabling foreign key checks for dropping tables and re-enabling them after.

Download all attachments as: .zip

Change History (5)

#1 @javorszky
4 years ago

Use case: testing a plugin that makes use of the WP testing library.

When the plugin's install script runs (either on init or hooked into activation), it creates a new table, for example:

$wpdb->query( "CREATE TABLE {$wpdb->prefix}tablename (
    id BIGINT(20) UNSIGNED AUTO_INCREMENT,
    post_id BIGINT(20) UNSIGNED,
    some_custom_value VARCHAR(191),
    PRIMARY KEY (`id`),
    KEY (`post_id`),
    KEY (`some_custom_value`),
    FOREIGN KEY (post_id)
        REFERENCES {$wpdb->posts}(ID)
        ON DELETE CASCADE
    ) ENGINE=INNODB {$collate}");

where $collate is

$collate = '';
if ( $wpdb->has_cap( 'collation' ) ) {
	if ( ! empty( $wpdb->charset ) ) {
		$collate .= "DEFAULT CHARACTER SET $wpdb->charset";
	}
	if ( ! empty( $wpdb->collate ) ) {
		$collate .= " COLLATE $wpdb->collate";
	}
}

Next time I want to run the tests, the posts table will not have been deleted. This only works if tablename comes later in alphabetical order than posts in this context.

Patch incoming.

Last edited 4 years ago by javorszky (previous) (diff)

@javorszky
4 years ago

Patch disabling foreign key checks for dropping tables and re-enabling them after.

#2 @netweb
4 years ago

  • Owner set to pento
  • Status changed from new to reviewing

This looks good to me, @pento thoughts?

#3 @pento
4 years ago

  • Milestone changed from Awaiting Review to 4.6
  • Status changed from reviewing to accepted
  • Version trunk deleted

How do we unit test the unit test code? :-D

Looks good, let's do this.

#4 @pento
4 years ago

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

In 37654:

Tests: Disable foreign_key_checks while dropping existing tables.

To ensure a clean run, the test suite drops all tables before installing, by simply looping over the table list and dropping them if they exist. This works well for Core, but may fail when a plugin has created a table with foreign key constraints in a previous test run.

Many plugins choose to base their test suite on the Core setup, so making life easier for them is a plus, even if Core doesn't directly need this change.

Props javorszky.

Fixes #37046.

Note: See TracTickets for help on using tickets.