Problem/Motivation

Drupal\KernelTests\Core\Database\ConnectionUnitTest is using simpletest logic to skip testing for non-MySql drivers.

Proposed resolution

Use proper PHPUnit markTestSkipped, and cleanup.

Remaining tasks

Review

User interface changes

none

API changes

none

Data model changes

none

Release notes snippet

none

CommentFileSizeAuthor
#2 3065166-2.patch11 KBmondrake

Comments

mondrake created an issue. See original summary.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Needs review
StatusFileSize
new 3065166-2.patch11 KB

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs review » Reviewed & tested by the community

This is some great cleanup!

With the patch (on sqlite):

PHPUnit 6.5.14 by Sebastian Bergmann and contributors.

Testing Drupal\KernelTests\Core\Database\ConnectionUnitTest
SSSS.                                                               5 / 5 (100%)

Time: 28.6 seconds, Memory: 8.00MB

OK, but incomplete, skipped, or risky tests!
Tests: 5, Assertions: 2, Skipped: 4.

without the patch:

PHPUnit 6.5.14 by Sebastian Bergmann and contributors.

Testing Drupal\KernelTests\Core\Database\ConnectionUnitTest
.....                                                               5 / 5 (100%)

Time: 26.54 seconds, Memory: 8.00MB

OK (5 tests, 7 assertions)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 3065166-2.patch, failed testing. View results

daffie’s picture

daffie commented
Status: Needs work » Reviewed & tested by the community

Testbot is green again.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 3065166-2.patch, failed testing. View results

daffie’s picture

daffie commented
Status: Needs work » Reviewed & tested by the community

Testbot failure unrelated to this patch.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 8.9.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 98daa59973 to 9.0.x and a3e523074f to 8.9.x and 5358f24e31 to 8.8.x. Thanks!

Backported to 8.8.x as this is a test-only fix.

  • alexpott committed 98daa59 on 9.0.x 
    Issue #3065166 by mondrake: Modernize Drupal\KernelTests\Core\Database\...

  • alexpott committed a3e5230 on 8.9.x 
    Issue #3065166 by mondrake: Modernize Drupal\KernelTests\Core\Database\...

  • alexpott committed 5358f24 on 8.8.x 
    Issue #3065166 by mondrake: Modernize Drupal\KernelTests\Core\Database\...

Status: Fixed » Closed (fixed)

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