Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
Discovered in #2575725: Run PHPunit tests one at a time via UI to avoid exception when selecting too many. The simpletest schema looks like this:
CREATE TABLE `simpletest` (
`message_id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'Primary Key: Unique simpletest message ID.',
`test_id` int(11) NOT NULL DEFAULT '0' COMMENT 'Test ID, messages belonging to the same ID are reported together',
`test_class` varchar(255) CHARACTER SET ascii NOT NULL DEFAULT '' COMMENT 'The name of the class that created this message.',
`status` varchar(9) NOT NULL DEFAULT '' COMMENT 'Message status. Core understands pass, fail, exception.',
`message` text NOT NULL COMMENT 'The message itself.',
`message_group` varchar(255) CHARACTER SET ascii NOT NULL DEFAULT '' COMMENT 'The message group this message belongs to. For example: warning, browser, user.',
`function` varchar(255) CHARACTER SET ascii NOT NULL DEFAULT '' COMMENT 'Name of the assertion function or method that created this message.',
`line` int(11) NOT NULL DEFAULT '0' COMMENT 'Line number on which the function is called.',
`file` varchar(255) NOT NULL DEFAULT '' COMMENT 'Name of the file where the function is called.',
PRIMARY KEY (`message_id`),
KEY `reporter` (`test_class`,`message_id`)
) ENGINE=InnoDB AUTO_INCREMENT=739 DEFAULT CHARSET=utf8mb4 COMMENT='Stores simpletest messages'
The reporter key looks completely useless as there are no look ups on the test_class field in core - perhaps the old qa.drupal.org used it - but DrupalCI certainly does not. Also even if it was required adding the message_id to it is pointless as this is the primary key on the table.
Proposed resolution
Remove the reporter index and create an index on test_id since there are lots of lookups by that:
- This query in SimpletestResultsForm would benefit from an index on test_id... this index does not help...
- The query in simpletest_last_test_get() does not use the index either... again a test_id index would help...
- The query in simpletest_script_load_messages_by_test_id() also does not use it ... and yet again a test_id index would help.
- The query in simpletest_script_open_browser() also does not use it... and yet again a test_id index would help.
- The query in simpletest_clean_results_table() also does not use it... and yet again a test_id index would help.
- The query in TestBase::deleteAssert() does not use it.
Remaining tasks
User interface changes
None
API changes
None
Data model changes
Changed indexes only - no data changes.
Comment | File | Size | Author |
---|---|---|---|
#17 | 2748315-17.patch | 2.82 KB | anushrikumari |
#2 | 2748315-2.patch | 2.63 KB | alexpott |
|
Comments
Comment #2
alexpottComment #4
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedMaybe message_id was added to make the composite index (with test_class) unique. I am guessing that this table has been modified a bit for 8.x to (attempt to) cater for running PHPunit tests, which were not in the original design of the module in Drupal6 or 7.
Good analysis on the usage (or rather non-usage) of this index, however I would suggest waiting until some of the other simpletest/PHPunit issues are resolved before altering the table. See the related issues I have just added.
Comment #13
quietone CreditAttribution: quietone as a volunteer commentedTriaging issues in simpletest.module as part of the Bug Smash Initiative to determine if they should be in the Simpletest Project or core.
This looks like it belongs in the Simpletest project.
Comment #14
alexpottFunnily enough this schema is still part of core - see \Drupal\Core\Test\TestDatabase::testingSchema() so this is a core fix and needs a reroll.
Comment #15
alexpottPutting under phpunit because that's the generic testing bucket.
Comment #16
anushrikumari CreditAttribution: anushrikumari at OpenSense Labs commentedRerolled patch for 8.9.xComment #17
anushrikumari CreditAttribution: anushrikumari at OpenSense Labs commentedComment #18
DamienMcKennaIt seems like this should split into two - the core/lib/Drupal/Core/Test/TestDatabase.php changes int his issue and the rest moved to the Simpletest contrib module.