After running a KernelTestBaseTNG™ the test tables are left in the database.

CommentFileSizeAuthor
#14 interdiff.txt626 bytesamateescu
#14 2553533-14.patch3.96 KBamateescu
#11 2553533-11.patch4.31 KBhussainweb
#11 interdiff-9-11.txt3.19 KBhussainweb
#9 2553533-9.patch6.75 KBdawehner
#7 2553533-7.patch7.96 KBdawehner
#7 interdiff.txt6.77 KBdawehner
#4 2553533.4.patch2.18 KBalexpott
#4 2-4-interdiff.txt1.11 KBalexpott
#2 2553533.2.patch1.07 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Active » Needs review
Issue tags: +Needs tests
FileSize
2553533.2.patch1.07 KB

This should be testable somehow.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented

Can't we test using the same method I used in #1713332: The SQLite database driver fails to drop simpletest tables, by overriding the run() method (or the phpunit equivalent)?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
FileSize
2-4-interdiff.txt1.11 KB
2553533.4.patch2.18 KB

@amateescu everything is destroyed in tearDown so there is nowhere to override like in #1713332: The SQLite database driver fails to drop simpletest tables

Also we have some tests that run set up multiple times generating a new prefix and in the sqlite case files that are not cleaned up in /tmp and an under declared dependency on sqlite.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented

How about overriding tearDown() itself?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

Tried that... after tear down everything is gone.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Tag1 Consulting commented
FileSize
interdiff.txt6.77 KB
2553533-7.patch7.96 KB

Tried out something like this, but for some reason, which is not obvious for me, the \Drupal\Core\Database\Driver\sqlite\Schema::findTables in the tearDown method itself does not find the config table. The next method in \Drupal\KernelTests\KernelTestBaseTest::tearDown then though finds it.

Status: Needs review » Needs work

The last submitted patch, 7: 2553533-7.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Tag1 Consulting commented
Status: Needs work » Needs review
FileSize
2553533-9.patch6.75 KB

Reupload.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Issue tags: -Needs tests +Needs reroll

Looking good!

+++ b/core/tests/Drupal/KernelTests/KernelTestBaseTest.php
@@ -19,7 +20,7 @@ class KernelTestBaseTest extends KernelTestBase {
-  public function testSetUpBeforeClass() {
+  public function ptestSetUpBeforeClass() {

@@ -27,7 +28,7 @@ public function testSetUpBeforeClass() {
-  public function testBootEnvironment() {
+  public function ptestBootEnvironment() {

@@ -51,31 +52,15 @@ public function testBootEnvironment() {
+  public function ptestGetDatabaseConnectionInfoWithOutManualSetDbUrl() {

… and more — all debug leftovers.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented
Issue tags: -Needs reroll
FileSize
interdiff-9-11.txt3.19 KB
2553533-11.patch4.31 KB

I think I got them all.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Tag1 Consulting commented

It would be great if @amateescu could have a look at it.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented

I tested the patch on SQLite and I can confirm that it works and the test tables are properly removed at the end of the test run.

+++ b/core/tests/Drupal/KernelTests/KernelTestBaseTest.php
@@ -19,7 +20,7 @@ class KernelTestBaseTest extends KernelTestBase {
-  public function testSetUpBeforeClass() {
+  public function ptestSetUpBeforeClass() {

Missed one :)

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Needs review » Reviewed & tested by the community
FileSize
2553533-14.patch3.96 KB
interdiff.txt626 bytes

No reason to hold it up over that.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Fixed

I'll be honest. I have absolutely no idea what this does, but random fails are bad, mmmkay?

Committed and pushed to 8.0.x. ;) Thanks!

  • webchick committed d6d3f6a on 8.0.x 
    Issue #2553533 by alexpott, dawehner, amateescu, hussainweb:...

Status: Fixed » Closed (fixed)

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