Problem/Motivation

#2791163: Random automatic testing failures on SQLite with PHP 5.5 broke simpletest_clean_database() - opps.

Proposed resolution

Clean tables like test* instead of simpletest*

Remaining tasks

User interface changes

API changes

None

Data model changes

None

CommentFileSizeAuthor
#11 2807171-11.patch1.07 KBalexpott
#11 8-11-interdiff.txt599 bytesalexpott
#8 3-8-interdiff.txt983 bytesalexpott
#8 2807171-8.patch1.08 KBalexpott
#3 2807171-2.patch1 KBalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review

I think we should commit this before 8.2.0 since this is not run-time code and only test code plus it might allow us to fix #2806697: Random fail for AlreadyInstalledException

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 2807171-2.patch1 KB
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
cilefen’s picture

cilefen commented 
+++ b/core/modules/simpletest/simpletest.module
@@ -673,13 +673,12 @@ function simpletest_clean_environment() {
     // Strip the prefix and skip tables without digits following "simpletest",
     // e.g. {simpletest_test_id}.

Wrong now, yes?

cilefen’s picture

cilefen commented

Actually, it is a useless comment.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/simpletest/simpletest.module
@@ -673,13 +673,12 @@ function simpletest_clean_environment() {
     // Strip the prefix and skip tables without digits following "simpletest",
     // e.g. {simpletest_test_id}.
-    if (preg_match('/simpletest\d+.*/', $table, $matches)) {
+    if (preg_match('/test\d+.*/', $table, $matches)) {
       db_drop_table($matches[0]);

IMHO the comment is still valid, just written a bit weird. Well, we don't want to delete the simpletest table, but yeah this is not a problem with this regex at all.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 2807171-8.patch1.08 KB
new 3-8-interdiff.txt983 bytes

Actually the current function is broken on prefixed sites. Here's a patch that is more reliable. I've tested this on both a prefixed db and a non prefixed db. Unfortunately this method is hard to test using DrupalCI.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Title: Fix simpletest_clean_database() » Use the correct testing table prefix in simpletest_clean_database()
Status: Needs review » Reviewed & tested by the community

Tested the patch manually, I can confirm that now any table prefixed with "test" is removed as it should.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented 
+++ b/core/modules/simpletest/simpletest.module
@@ -673,13 +673,12 @@ function simpletest_clean_environment() {
+    // Only drop tables which begin wih 'test' followed by digits, for example,
+    // example, {test12345678node__body}.

"example" twice in the comment, can be fixed on commit.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 8-11-interdiff.txt599 bytes
new 2807171-11.patch1.07 KB

  • catch committed e014859 on 8.3.x 
    Issue #2807171 by alexpott: Use the correct testing table prefix in...

  • catch committed bab1243 on 8.2.x 
    Issue #2807171 by alexpott: Use the correct testing table prefix in...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.3.x and cherry-picked to 8.2.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.