Problem/Motivation

#2318191: [meta] Database tests fail on SQLite fixed all Database tests when running SQLite as database backend. Now all remaining issues related to SQLite need to be fixed in order to make all tests pass the (temporary) SQLite TestBot.

Remaining tasks

Active:

Fixed:
#2475187: Add a user-space implementation for LIKE BINARY in SQLite
#2486831: The $dbh and $connection members are mixed up in Drupal\Core\Database\StatementPrefetch
#2484539: PostgreSQL: Fix views\Tests\Handler\FieldFieldTest
#2475247: SQLite: Fix views\Tests\Handler\FilterStringTest
#2454751: SQLite: Fix user\Tests\Views\RelationshipRepresentativeNodeTest
#2454747: SQLite: Fix sub-system tests in system test group
#2475177: SQLite: Fix system\Tests\Database\FetchTest
#2463321: Serializing the database connection is dangerous and error-prone, make it unserializable again
#2465633: Bring back the custom Statement class for the SQLite driver
#2454669: SQLite: Fix tests in migrate_drupal test group
#2463263: SQLite: Fix system\Tests\Entity\EntityDefinitionUpdateTest
#2454625: SQLite: Fix SQLITE_SCHEMA errors in web tests
#2454753: [Meta] SQLite: Fix sub-system tests in the views module
#2454721: SQLite: Fix comment\Tests\CommentFieldsTest
#2454723: SQLite: Fix config\Tests\ConfigImportAllTest
#2454727: SQLite: Fix contact\Tests\ContactStorageTest
#2454729: SQLite: Fix search\Tests\SearchNumbersTest
#2454731: SQLite: Fix search\Tests\SearchRankingTest
#2454733: Add a user-space case-insensitive collation to the SQLite driver
#2454735: SQLite: Fix toolbar\Tests\ToolbarAdminMenuTest
#2258177: Convert migrate_drupal tests to KernelTestBase

User interface changes

Nope.

API changes

Possible changes in the SQLite database driver.

CommentFileSizeAuthor
#33 Screen Shot 2015-05-22 at 1.56.12 PM.png38.18 KBwebchick
#30 sqlite-test-run.txt362.83 KBamateescu

Comments

dawehner’s picture

dawehner
Primary language German
commented

+1 nice effort!

amateescu’s picture

amateescu commented

This is the list of tests that still fail with #2428695: SQLite date handling is wrongly implemented and arguments handling needs override applied:

Drupal\comment\Tests\CommentFieldsTest

Drupal\config\Tests\ConfigImportAllTest

Drupal\contact\Tests\ContactStorageTest

Drupal\content_translation\Tests\ContentTranslationSettingsTest

Drupal\field_ui\Tests\ManageDisplayTest

Drupal\file\Tests\FileFieldWidgetTest
Drupal\file\Tests\PrivateFileOnTranslatedEntityTest

Drupal\image\Tests\ImageFieldDisplayTest

Drupal\migrate_drupal\Tests\d6\MigrateAggregatorFeedTest
Drupal\migrate_drupal\Tests\d6\MigrateAggregatorItemTest
Drupal\migrate_drupal\Tests\d6\MigrateBlockContentTest
Drupal\migrate_drupal\Tests\d6\MigrateBlockTest
Drupal\migrate_drupal\Tests\d6\MigrateBookTest
Drupal\migrate_drupal\Tests\d6\MigrateCckFieldRevisionTest
Drupal\migrate_drupal\Tests\d6\MigrateCckFieldValuesTest
Drupal\migrate_drupal\Tests\d6\MigrateCommentTest
Drupal\migrate_drupal\Tests\d6\MigrateContactCategoryTest
Drupal\migrate_drupal\Tests\d6\MigrateDateFormatTest
Drupal\migrate_drupal\Tests\d6\MigrateFieldFormatterSettings
Drupal\migrate_drupal\Tests\d6\MigrateFieldInstanceTest
Drupal\migrate_drupal\Tests\d6\MigrateFieldTest
Drupal\migrate_drupal\Tests\d6\MigrateFieldWidgetSettingsTest
Drupal\migrate_drupal\Tests\d6\MigrateDrupal6Test
Drupal\migrate_drupal\Tests\d6\MigrateFileTest
Drupal\migrate_drupal\Tests\d6\MigrateFilterFormatTest
Drupal\migrate_drupal\Tests\d6\MigrateMenuTest
Drupal\migrate_drupal\Tests\d6\MigrateMenuLinkTest
Drupal\migrate_drupal\Tests\d6\MigrateNodeBundleSettingsTest
Drupal\migrate_drupal\Tests\d6\MigrateNodeRevisionTest
Drupal\migrate_drupal\Tests\d6\MigrateNodeTest
Drupal\migrate_drupal\Tests\d6\MigrateNodeTypeTest
Drupal\migrate_drupal\Tests\d6\MigrateTaxonomyTermTest
Drupal\migrate_drupal\Tests\d6\MigrateTaxonomyVocabularyTest
Drupal\migrate_drupal\Tests\d6\MigrateTermNodeRevisionTest
Drupal\migrate_drupal\Tests\d6\MigrateTermNodeTest
Drupal\migrate_drupal\Tests\d6\MigrateUrlAliasTest
Drupal\migrate_drupal\Tests\d6\MigrateUploadTest
Drupal\migrate_drupal\Tests\d6\MigrateUserContactSettingsTest
Drupal\migrate_drupal\Tests\d6\MigrateUserPictureFileTest
Drupal\migrate_drupal\Tests\d6\MigrateUserProfileEntityDisplayTest
Drupal\migrate_drupal\Tests\d6\MigrateUserProfileEntityFormDisplayTest
Drupal\migrate_drupal\Tests\d6\MigrateUserProfileFieldInstanceTest
Drupal\migrate_drupal\Tests\d6\MigrateUserProfileFieldTest
Drupal\migrate_drupal\Tests\d6\MigrateUserRoleTest
Drupal\migrate_drupal\Tests\d6\MigrateUserTest
Drupal\migrate_drupal\Tests\d6\MigrateUserProfileValuesTest
Drupal\migrate_drupal\Tests\d6\MigrateViewModesTest
Drupal\migrate_drupal\Tests\d6\MigrateVocabularyEntityDisplayTest
Drupal\migrate_drupal\Tests\d6\MigrateVocabularyEntityFormDisplayTest
Drupal\migrate_drupal\Tests\d6\MigrateVocabularyFieldInstanceTest
Drupal\migrate_drupal\Tests\d6\MigrateVocabularyFieldTest

Drupal\system\Tests\Database\ConnectionUnitTest
Drupal\system\Tests\Entity\EntityDefinitionUpdateTest
Drupal\system\Tests\Entity\EntityQueryTest
Drupal\system\Tests\Entity\FieldSqlStorageTest
Drupal\system\Tests\Module\DependencyTest
Drupal\system\Tests\Module\InstallUninstallTest
Drupal\system\Tests\Session\SessionHttpsTest

Drupal\options\Tests\OptionsFieldUITest

Drupal\search\Tests\SearchNumbersTest
Drupal\search\Tests\SearchRankingTest

Drupal\taxonomy\Tests\TermTest

Drupal\toolbar\Tests\ToolbarAdminMenuTest

Drupal\user\Tests\UserAdminTest
Drupal\user\Tests\Views\HandlerFieldRoleTest
Drupal\user\Tests\Views\RelationshipRepresentativeNodeTest

Drupal\views\Tests\GlossaryTest
Drupal\views\Tests\Handler\ArgumentDateTest
Drupal\views\Tests\SearchMultilingualTest

I'll start opening sub-issues and post patches where I have them.

dawehner’s picture

dawehner
Primary language German
commented

@amateescu
You should maybe also have a look https://www.drupal.org/files/issues/ktbng.1.patch ... it contains maybe a bunch of fixes for views and what not.

amateescu’s picture

amateescu commented

@dawehner, yup, I'll also start testing with that :)

amateescu’s picture

amateescu commented
Issue summary: View changes

Opened #2454625: SQLite: Fix SQLITE_SCHEMA errors in web tests and #2454669: SQLite: Fix tests in migrate_drupal test group for the tests that can be fixed together. I'm afraid all the other ones will need individual care.

I looked at the patch mentioned in #3 but it seems all the SQLite fixes have been extracted into #2318191: [meta] Database tests fail on SQLite, so there's no further help to seek from it.

amateescu’s picture

amateescu commented
Issue summary: View changes

Added issues for all the other ones.

chx’s picture

chx commented

I filed #2459745: Allow the database driver to skip test classes as a last resort.

amateescu’s picture

amateescu commented

Wow, that really is a last resort type of patch :) Thanks for heads up!

In the meantime, I made some more progress here, I figured out that we can move the fix from #2454625: SQLite: Fix SQLITE_SCHEMA errors in web tests up a level to fix even more tests, so I closed a few sub-issues as duplicates, see comments #2 and #3 in that issue.

chx’s picture

chx commented

Please realize you will need #7. I am reasonably sure https://www.drupal.org/node/2157455#comment-9713189 can't pass as we do not have collation support in SQLite.

amateescu’s picture

amateescu commented

@chx, we do have it now: #2454733: Add a user-space case-insensitive collation to the SQLite driver

amateescu’s picture

amateescu commented

I just opened #2463321: Serializing the database connection is dangerous and error-prone, make it unserializable again as a major bug in the database system and I'm hoping that it'll fix more tests than just the one mentioned in comment #1 there, so I'm advertising it in this meta in hope of quicker reviews :)

amateescu’s picture

amateescu commented
Issue summary: View changes

We're *really* close to having all tests passing on SQLite :) There are only 4 issues left that need review:

#2463321: Serializing the database connection is dangerous and error-prone, make it unserializable again
#2465633: Bring back the custom Statement class for the SQLite driver
#2454669: SQLite: Fix tests in migrate_drupal test group
#2463263: SQLite: Fix system\Tests\Entity\EntityDefinitionUpdateTest

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

Can we close this and move #2454751: SQLite: Fix user\Tests\Views\RelationshipRepresentativeNodeTest to critical? And awesome work @amateescu.

catch’s picture

catch
he/him
Primary language English
commented

Let's keep it open until we actually see a 100% test run with HEAD, but yes great to see this close.

catch’s picture

catch
he/him
Primary language English
commented
Title: [Meta] Make Drupal 8 work with SQLite » [meta-1] Make Drupal 8 work with SQLite
catch’s picture

catch
he/him
Primary language English
commented
Title: [meta-1] Make Drupal 8 work with SQLite » [meta-2] Make Drupal 8 work with SQLite
Issue summary: View changes
amateescu’s picture

amateescu commented
Title: [meta-2] Make Drupal 8 work with SQLite » [meta-5] Make Drupal 8 work with SQLite
Issue summary: View changes

I had to open a few more today :/

#2475187: Add a user-space implementation for LIKE BINARY in SQLite
#2475177: SQLite: Fix system\Tests\Database\FetchTest
#2475247: SQLite: Fix views\Tests\Handler\FilterStringTest

And there are also some tests that fail on the DrupalCI bot but pass locally:

Drupal\system\Tests\Form\TriggeringElementTest

Drupal\locale\Tests\LocaleImportFunctionalTest

Drupal\Tests\simpletest\Functional\BrowserTestBaseTest
Drupal\simpletest\Tests\SimpleTestBrowserTest

Drupal\views\Tests\Handler\ArgumentDateTest

At some point I think I'll need to set up a DrupalCI environment locally to check them out.

xjm’s picture

xjm
she/her
Primary language English
commented

Also see: #567148: Use ONLY_FULL_GROUP_BY for MySQL -- a change that allegedly broke SQLite somehow more than 5 years ago.

dcrocks’s picture

dcrocks commented

Another old one. Shouldn't #2027727: Fix Sqlite Schema definition and add unit tests be tracked here as well?

amateescu’s picture

amateescu commented
Title: [meta-5] Make Drupal 8 work with SQLite » [meta-2] Make Drupal 8 work with SQLite
Issue summary: View changes

3 child issues have been fixed in the meantime.

dawehner’s picture

dawehner
Primary language German
commented
Title: [meta-2] Make Drupal 8 work with SQLite » [meta-1] Make Drupal 8 work with SQLite

.

amateescu’s picture

amateescu commented
Issue summary: View changes

Yay, the new production branch of DrupalCI just got (back) support for SQLite testing: #2484103: Add support for sqlite database testing

Since the current temporary testbot is based on the older PoC DrupalCI code/containers, tomorrow I'll run all the failing tests to see what's really left to do here.

amateescu’s picture

amateescu commented
Title: [meta-1] Make Drupal 8 work with SQLite » [meta-4] Make Drupal 8 work with SQLite
Issue summary: View changes

We are so close to having tests fully passing on SQLite!

I just ran all the failing test from the temporary testbot on a local DrupalCI setup with these patches applied:

#2475187-8: Add a user-space implementation for LIKE BINARY in SQLite
#2258177-18: Convert migrate_drupal tests to KernelTestBase
#2486831-1: The $dbh and $connection members are mixed up in Drupal\Core\Database\StatementPrefetch
#2484539-2: PostgreSQL: Fix views\Tests\Handler\FieldFieldTest

and I got some great results:

Drupal test run
---------------

Tests to be run:
  - Drupal\system\Tests\Database\LoggingTest
  - Drupal\system\Tests\Entity\EntityQueryTest
  - Drupal\image\Tests\ImageAdminStylesTest
  - Drupal\migrate_drupal\Tests\d6\MigrateDrupal6Test
  - Drupal\rest\Tests\Views\StyleSerializerTest
  - Drupal\shortcut\Tests\ShortcutLinksTest
  - Drupal\Tests\simpletest\Functional\BrowserTestBaseTest
  - Drupal\simpletest\Tests\SimpleTestBrowserTest
  - Drupal\user\Tests\UserPasswordResetTest
  - Drupal\views\Tests\Handler\FieldFieldTest

Test run started:
Drupal\system\Tests\Database\LoggingTest                      24 passes
Drupal\system\Tests\Entity\EntityQueryTest                   141 passes
Drupal\migrate_drupal\Tests\d6\MigrateDrupal6Test            767 passes   1 fails
Drupal\Tests\simpletest\Functional\BrowserTestBaseTest         2 passes
Drupal\simpletest\Tests\SimpleTestBrowserTest                102 passes
Drupal\image\Tests\ImageAdminStylesTest                      272 passes
Drupal\views\Tests\Handler\FieldFieldTest                    196 passes
Drupal\rest\Tests\Views\StyleSerializerTest                  149 passes
Drupal\shortcut\Tests\ShortcutLinksTest                      458 passes
Drupal\user\Tests\UserPasswordResetTest                      104 passes

Test run duration: 4 min 3 sec

Sadly, I can not reproduce the MigrateDrupal6Test fail locally, I need to investigate how to debug in a DrupalCI container. Any hints are welcomed :)

amateescu’s picture

amateescu commented

Posted a new patch in #2258177-22: Convert migrate_drupal tests to KernelTestBase which fixes the last failure, so we're 4 patch reviews away from closing this issue :)

catch’s picture

catch
he/him
Primary language English
commented
Title: [meta-4] Make Drupal 8 work with SQLite » [meta-3] Make Drupal 8 work with SQLite
dawehner’s picture

dawehner
Primary language German
commented

#2486831: The $dbh and $connection members are mixed up in Drupal\Core\Database\StatementPrefetch is in

amateescu’s picture

amateescu commented
Title: [meta-3] Make Drupal 8 work with SQLite » [meta-2] Make Drupal 8 work with SQLite
Issue summary: View changes

1 more down, 2 to go :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Go go @amateescu \o/

fabianx’s picture

fabianx commented
Title: [meta-2] Make Drupal 8 work with SQLite » [meta-1] Make Drupal 8 work with SQLite
Issue summary: View changes

#2258177: Convert migrate_drupal tests to KernelTestBase is in already, just one more to go.

amateescu’s picture

amateescu commented
StatusFileSize
new sqlite-test-run.txt362.83 KB

I just did a full test run locally with the latest DrupalCI code / containers and I can confirm that all tests are passing with the patch from #2475187-16: Add a user-space implementation for LIKE BINARY in SQLite applied. Attaching the glorious test run :)

amateescu’s picture

amateescu commented
Title: [meta-1] Make Drupal 8 work with SQLite » [meta] Make Drupal 8 work with SQLite
Issue summary: View changes

Yay, the last issue has been committed! I've emailed @bzrudi71 to see if he can update the current temporary testbot to the latest DrupalCI code, which should confirm my successful test run from #30.

dawehner’s picture

dawehner
Primary language German
commented

https://www.youtube.com/watch?v=ch02NDiG7M8

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Active » Fixed
StatusFileSize
new Screen Shot 2015-05-22 at 1.56.12 PM.png38.18 KB

HOLY FRIJOLES!! According to http://d8sqlitebot.erwanderbar.de/ we are now at ZERO FAILS!!!

91641 passes, 0 fails, 1 exception

The one exception btw is coming from:

Drupal\language\Tests\LanguageConfigSchemaTest                 0 passes             1 exceptions

(Edit: Oops, I should point out that according to bzrudi71 in this issue summary update, random exceptions are still happening on his bots and he's working on a fix.)

So I think... I think... I think it's... fixed?!???

F*CK YEAH AMATEESCU!! :D

xjm’s picture

xjm
she/her
Primary language English
commented

AW HECK YES! :) @amateescu wins at the internet.

So I think, +1 for marking it fixed. And then for any SQLite regressions between now and release:

  • If a new failure is introduced by a non-critical issue, we revert the issue that introduced it as soon as possible, and ensure SQLite passes before committing an updated patch.
  • If a critical introduces an SQLite failure, we can decide whether to make fixing the SQLite failure a critical followup.
  • In general, any SQLite failure is individually critical.
Crell’s picture

Crell commented

*throws confetti* Epic win!

bzrudi71’s picture

bzrudi71 commented

Yep, let's consider this one as fixed as all exceptions are bot issues (confirmed by dozens of runs) :) Hopefully we will see some *confetti* even for the not so loved 'most advanced open source database' namely PostgreSQL soon ;) Great work @amateescu!

moshe weitzman’s picture

moshe weitzman commented

Nice work! If anyone spots problems with drush+sqlite please let us know.

amateescu’s picture

amateescu commented

WOOHOO, thanks everyone for helping with this!

Status: Fixed » Closed (fixed)

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