SQlite has wrong DB structure after upgrading to 1.35
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Peter.ovchyn
	Feb 25 2020, 9:32 AM

Description

Motivation

At the moment after incremental upgrade DB to 1.35, there's a difference between MySQL and SQLite DB structure:

fields related to 'actor' have default 0:

ar_actor
img_actor
fa_actor
etc...

Expected Result:

DB structure should be equal. Defaults should be dropped by the SQLite's incremental upgrade scripts.
Tests should pass.

Additional Info:

1) DatabaseSqliteTest::testUpgrades
Default values does not match for column filearchive.fa_actor upgrading from 1.15 to 1.35.0-alpha
Failed asserting that '0' matches expected null.

/Users/peter/work/Wiki/gerrit/mediawiki/tests/phpunit/integration/includes/db/DatabaseSqliteTest.php:399
/Users/peter/work/Wiki/gerrit/mediawiki/tests/phpunit/MediaWikiIntegrationTestCase.php:417

FAILURES!
Tests: 42, Assertions: 407, Failures: 1, Risky: 1.
Script phpunit --colors=always --testsuite=core:integration,extensions:integration,skins:integration handling the phpunit:integration event returned with error code 1

Details

	Subject	Repo	Branch	Lines +/-
	Drop DEFAULT from *_actor columns for SQLite tables that still have it	mediawiki/core	master	+158 -1

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved		Peter.ovchyn	T245995 Sqlite Integration tests fail on macOS php 7.4
		Resolved		aaron	T246077 SQlite has wrong DB structure after upgrading to 1.35

Event Timeline

Peter.ovchyn created this task.Feb 25 2020, 9:32 AM

Bumping to high, since this seems to break sqlite installs

daniel added a project: MW-1.35-release.Feb 26 2020, 10:30 AM

Hmm. It looks like rMW214750d8d224: Define unit and integration test suites may have broken the running of this test in CI, when it moved the test from tests/phpunit/includes to tests/phpunit/integration but never added the latter path to tests/phpunit/suite.xml.

When I try to run it locally, it doesn't even get to the failure quoted, instead it dies on "Undefined index: topologyRole" (added in fb621c26a37, and the necessary change to this test wasn't done because the test wasn't being run). Fixing that turns up several more failures.

I guess you must be using composer phpunit:integration to try to run tests? When I try to run that locally, it dumps a bunch of warnings then blows up with a ContainerDisabledException. When I try it with only core:integration (to skip it trying to run tests for extensions I have in my extensions directory but don't have set up and enabled), it dumps even more warnings and fails with this error plus 5 others that seem to be from 673d496f. Given all that I have to wonder if this testing entry point is actually used anywhere.

As for this task itself, it looks like rMWc29909e59fd8: Mostly drop old pre-actor user schemas missed including the change of default for the xx_actor fields in the SQLite versions of the patches.

AFAIK composer phpunit:integration is not used in CI, because we wanted to avoid duplicate test runs. Part of T87781 implies eventually migrating to the plain PHPUnit entrypoint and removing tests/phpunit/phpunit.php so we only have one way to run these tests instead of two.

when it moved the test from tests/phpunit/includes to tests/phpunit/integration but never added the latter path to tests/phpunit/suite.xml.

Yes, sorry. Do you want to submit a patch or should I?

kostajh added a subscriber: TK-999.Feb 26 2020, 3:11 PM

In T246077#5920066, @kostajh wrote:

AFAIK composer phpunit:integration is not used in CI, because we wanted to avoid duplicate test runs. Part of T87781 implies eventually migrating to the plain PHPUnit entrypoint and removing tests/phpunit/phpunit.php so we only have one way to run these tests instead of two.

We should probably take this discussion to a different task. Do you think T227900 would be appropriate?

when it moved the test from tests/phpunit/includes to tests/phpunit/integration but never added the latter path to tests/phpunit/suite.xml.

Yes, sorry. Do you want to submit a patch or should I?

At the moment there seems to be three parts: the failure described in this task (currently claimed by @Peter.ovchyn), the failures due to changes in 673d496f, and then the actual re-enabling. I'll be happy to review if the two of you would like to divide up the work (or one of you just wants to do all of it).

We might want to retitle and expand this task to cover all three parts, or else make a parent task to this one covering all the parts.

Anomie mentioned this in T246078: Add SQLite infra to Jenkins CI.Feb 26 2020, 4:55 PM

It looks like T245995 was already filed for the second of the three parts, and has a patch.

We should probably take this discussion to a different task. Do you think T227900 would be appropriate?

Yeah let's discuss there

Anomie mentioned this in T245995: Sqlite Integration tests fail on macOS php 7.4.Feb 26 2020, 9:31 PM

This issue is 100% exactly what described.

There're SQL scripts with that drop defaults for fields like filearchive.fa_actor, etc for Postgres and MySQL, but no the same for SQLite.

See patch-drop-user-fields.sql. No equivalent for SQLite. When I have time, I'll fix this.

Peter.ovchyn moved this task from Backlog to Estimated/Estimation on the Platform Team Workboards (S&F Workboard) board.Mar 12 2020, 9:43 PM

Peter.ovchyn moved this task from Estimated/Estimation to In Progress/Doing on the Platform Team Workboards (S&F Workboard) board.

Change 581884 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/core@master] Drop DEFAULT from *_actor columns for SQLite tables that still have it

https://gerrit.wikimedia.org/r/581884

gerritbot added a project: Patch-For-Review.Mar 20 2020, 4:18 AM

Peter.ovchyn reassigned this task from Peter.ovchyn to aaron.Mar 20 2020, 7:55 AM

Peter.ovchyn removed a project: Platform Team Workboards (S&F Workboard).Mar 20 2020, 7:58 AM

Anomie added a project: Platform Team Workboards (Clinic Duty Team).Mar 20 2020, 1:30 PM

Anomie closed this task as Resolved.Mar 20 2020, 1:43 PM

Anomie moved this task from Inbox to External Code Review Completed on the Platform Team Workboards (Clinic Duty Team) board.

Change 581884 merged by jenkins-bot:
[mediawiki/core@master] Drop DEFAULT from *_actor columns for SQLite tables that still have it

https://gerrit.wikimedia.org/r/581884

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.25; 2020-03-24).Mar 20 2020, 2:00 PM

Maintenance_bot removed a project: Patch-For-Review.Mar 20 2020, 2:10 PM

Krinkle awarded a token.Mar 20 2020, 5:26 PM