Address WordPress PHPUnit test fails: charset detection, length validation etc by chloe-pomegranate · Pull Request #331 · WordPress/sqlite-database-integration

chloe-pomegranate · 2026-03-20T14:21:31Z

Summary

Fixes (some of) #320 - WordPress PHPUnit test failures related to charset/collation handling, column length validation, and db capabilities

Changes Made

New Methods

get_col_charset($table, $column) - Returns charset for a table column by parsing the Collation field from SHOW FULL COLUMNS output
get_col_length($table, $column) - Returns column length information for validation, supporting char/varchar/binary/varbinary and TEXT/BLOB
has_cap($db_cap) - Reports supported capabilities: collation, group_concat, subqueries, set_charset, utf8mb4, utf8mb4_520, identifier_placeholders
determine_charset($charset, $collate) - Upgrades utf8 to utf8mb4 and handles collation mapping

Existing Methods Changed

query() - Added strip_invalid_text_from_query() validation to reject queries with invalid data for the column charset
close() - Properly sets ready=false and has_connected=false when closing the connection

Tests

Fixes from the issue:

Charset detection tests
Length validation tests
Capability tests
Charset tests
Data handling tests

Other

Haven't touched these ones as they're generally all MySQL specific or SQLite handles them differently:

test_replace - MySQL REPLACE returns affected rows differently than SQLite
test_close - Similar to above
test_mysqli_flush_sync - MySQL-specific, don't think we can do anything here
test_spatial_indices - SQLite doesn't support SPATIAL KEY
test_wp_update_comment
test_strip_invalid_text
test_set_charset_changes_the_connection_collation

Notes

For new methods I've used @since 2.3.0 - can change to whatever :)

JanJakes · 2026-03-30T12:08:19Z

Hi @chloe-pomegranate, thanks for your contribution! It demonstrates that we're only a few small fixes away from improving the WP unit test compatibility. I left some feedback, mostly with ideas of better aligning with WPDB and what makes sense for SQLite. Feel free to address them, or I can take it over later on. Thanks again for bringing these to our attention!

chloe-pomegranate · 2026-03-30T12:45:18Z

hey @JanJakes no prob, would be happy to! I can't see the review at the moment though, am I missing something?

JanJakes

@chloe-pomegranate Ups, sorry, missed the submit review button.

JanJakes · 2026-03-30T11:32:55Z

 	 *
 	 * @return bool Whether the database feature is supported, false otherwise.
 	 */
 	public function has_cap( $db_cap ) {
-		return 'subqueries' === strtolower( $db_cap );


Do we still need this method override at all? The capabilities are derived from the MySQL version, so they depend on the db_version() implementation (currently hardcoded to 8.0).

JanJakes · 2026-03-30T11:34:56Z

+			case 'utf8mb4':
+			case 'utf8mb4_520':
+			case 'identifier_placeholders':
+				return true;


This one seems to be dependent on WordPress version.

Added a version_compare for this one

JanJakes · 2026-03-30T11:37:36Z

-		 *        To do so, mb_convert_encoding can be used with an optional
-		 *        fallback to a htmlspecialchars method. E.g.:
-		 *          https://github.com/nette/utils/blob/be534713c227aeef57ce1883fc17bc9f9e29eca2/src/Utils/Strings.php#L42
-		 */


The TODO explains what should be done for SQLite. However, the implementation doesn't seem to follow it, and it removes all of that context. The TODO is there for a reason, and we should follow it.

JanJakes · 2026-03-30T11:42:42Z

+	 * Determines the best charset and collation for the database connection.
+	 *
+	 * This overrides wpdb::determine_charset() to handle SQLite's lack of mysqli.
+	 * WordPress expects utf8 to be upgraded to utf8mb4 when supported.


Let's just state that this closely mirrors the implementation of the parent method, like we do for other methods, keeping the parent method docs otherwise.

JanJakes · 2026-03-30T11:44:50Z

+	 * @since 2.3.0
+	 *
+	 * @return bool True if connection was closed successfully.
+	 */


Let's just state that this closely mirrors the implementation of the parent method, like we do for other methods, keeping the parent method docs otherwise.

JanJakes · 2026-03-30T11:48:56Z

+		$table_key  = strtolower( $table );
+		$column_key = strtolower( $column );
+
+		// Check cached column metadata first.


The parent method ensures $this->col_meta is populated, but here it's missing.

Alternatively, we could also consider just calling the parent:

try { $this->is_mysql = true; // We'd need to explain this hack. return parent::get_col_length( $table, $column ); } finally { $this->is_mysql = false; }

Amended to go with this method

JanJakes · 2026-03-30T11:52:04Z

+
+		// Use SHOW FULL COLUMNS to get column metadata with collation info.
+		// This works because the driver translates this to query the information schema.
+		$results = $this->get_results( "SHOW FULL COLUMNS FROM `{$table_key}`" );


Is there a reason to implement this differently than WPDB does?

Alternatively, we could also consider just calling the parent:

try { $this->is_mysql = true; // We'd need to explain this hack. return parent::get_col_charset( $table, $column ); } finally { $this->is_mysql = false; }

Amended to go with this method

chloe-pomegranate · 2026-03-31T11:59:59Z

@JanJakes Made a few updates from your review!

…ation, capabilities

…apabilities | Amends from review

Roll back active transactions and discard the driver handle on close. Release internally created PDO instances so reconnecting creates a fresh connection, while preserving externally supplied PDO instances for compatibility with the global injection mechanism.

Return the requested charset and collation unchanged when the wpdb instance has no active SQLite handle. Re-enable the WordPress charset tests that this behavior now satisfies.

Handle only the MySQL-client-specific utf8mb4 capability in the SQLite override and defer all other capabilities to WordPress. This restores identifier placeholders and database-version-based utf8mb4_520 detection.

Restore the previous is_mysql value after column charset and length lookups, treating an unset flag as null.

Remove existing WordPress tests that now pass through the charset and field-validation changes from the expected error and failure lists. Keep the remaining unsupported cases explicit.

Restore the previous query path and expected WordPress test outcomes so UTF-8 validation can be designed separately. Keep a focused TODO recommending PCRE validation while preserving wpdb exemptions.

chloe-pomegranate mentioned this pull request Mar 20, 2026

Address remaining WordPress PHPUnit test failures #320

Open

JanJakes reviewed Mar 30, 2026

View reviewed changes

chloe-pomegranate added 2 commits July 17, 2026 13:57

Address WordPress PHPUnit test fails: charset detection, length valid…

9013237

…ation, capabilities

WordPress PHPUnit test fails: charset detection, length validation, c…

3942a86

…apabilities | Amends from review

JanJakes force-pushed the fix-wp-tests-charset-etc branch 5 times, most recently from aac38ee to 7f0ab37 Compare July 18, 2026 07:57

JanJakes added 6 commits July 18, 2026 10:20

Preserve charset while disconnected

692a784

Return the requested charset and collation unchanged when the wpdb instance has no active SQLite handle. Re-enable the WordPress charset tests that this behavior now satisfies.

Delegate wpdb capability checks

7a69387

Handle only the MySQL-client-specific utf8mb4 capability in the SQLite override and defer all other capabilities to WordPress. This restores identifier placeholders and database-version-based utf8mb4_520 detection.

Preserve wpdb type during metadata lookup

5359f88

Restore the previous is_mysql value after column charset and length lookups, treating an unset flag as null.

Update WordPress test expectations

09b9fc5

Remove existing WordPress tests that now pass through the charset and field-validation changes from the expected error and failure lists. Keep the remaining unsupported cases explicit.

Defer SQLite query UTF-8 validation

00c2ff3

Restore the previous query path and expected WordPress test outcomes so UTF-8 validation can be designed separately. Keep a focused TODO recommending PCRE validation while preserving wpdb exemptions.

JanJakes force-pushed the fix-wp-tests-charset-etc branch from 7f0ab37 to 00c2ff3 Compare July 18, 2026 08:21

Uh oh!

Conversation

chloe-pomegranate commented Mar 20, 2026

Summary

Changes Made

New Methods

Existing Methods Changed

Tests

Other

Notes

Uh oh!

JanJakes commented Mar 30, 2026

Uh oh!

chloe-pomegranate commented Mar 30, 2026

Uh oh!

JanJakes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chloe-pomegranate commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants