Last modified: 2014-11-12 00:51:21 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 37601 - Tests introduced in tests/phpunit/includes/db/TestORMRowTest.php fail on PostgreSQL
Tests introduced in tests/phpunit/includes/db/TestORMRowTest.php fail on Post...
Status: PATCH_TO_REVIEW
Product: MediaWiki
Classification: Unclassified
Unit tests (Other open bugs)
1.20.x
All All
: Normal normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
: 38930 (view as bug list)
Depends on:
Blocks: postgres
  Show dependency treegraph
 
Reported: 2012-06-14 17:48 UTC by Marcin Cieślak
Modified: 2014-11-12 00:51 UTC (History)
9 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Patch for table creation in test case. (3.13 KB, patch)
2012-09-19 21:54 UTC, Tim Landscheidt
Details

Description Marcin Cieślak 2012-06-14 17:48:29 UTC
maintenance/update.php script was run, but the tests fail nethertheless on PostgreSQL:

$ sh -x /usr/home/saper/bin/runphpunit.sh /usr/home/saper/public_html/pg/w/tests/phpunit/includes/db/TestORMRowTest.php
+ php -c /usr/home/saper/php.ini tests/phpunit/phpunit.php --configuration tests/phpunit/suite.xml --exclude-group Broken,Stub,Dump,ParserFuzz --log-junit /usr/home/saper/tests/log/postgres-log.xml /usr/home/saper/public_html/pg/w/tests/phpunit/includes/db/TestORMRowTest.php
PHPUnit 3.6.10 by Sebastian Bergmann.

Configuration read from /usr/home/saper/public_html/pg/w/tests/phpunit/suite.xml

EFF

Time: 1 second, Memory: 31.25Mb

There was 1 error:

1) TestORMRowTest::testConstructor with data set #0 (array('Foobar', 42, 9000.1, true, array(13, 11, 7, 5, 3, 2), stdClass), true)
DBQueryError: A database error has occurred.  Did you forget to run maintenance/update.php after upgrading?  See: https://www.mediawiki.org/wiki/Manual:Upgrading#Run_the_update_script
Query: CREATE TABLE IF NOT EXISTS "unittest_orm_test"(
				test_id                    INT unsigned        NOT NULL auto_increment PRIMARY KEY,
				test_name                  VARCHAR(255)        NOT NULL,
				test_age                   TINYINT unsigned    NOT NULL,
				test_height                FLOAT               NOT NULL,
				test_awesome               TINYINT unsigned    NOT NULL,
				test_stuff                 BLOB                NOT NULL,
				test_moarstuff             BLOB                NOT NULL,
				test_time                  varbinary(14)       NOT NULL
			);
Function: DatabaseBase::safeQuery
Error: 42601 ERROR:  syntax error at or near "unsigned"
LINE 2:     test_id                    INT unsigned        NOT NULL ...
                                           ^



/usr/home/saper/public_html/pg/w/includes/db/Database.php:953
/usr/home/saper/public_html/pg/w/includes/db/DatabasePostgres.php:394
/usr/home/saper/public_html/pg/w/includes/db/Database.php:920
/usr/home/saper/public_html/pg/w/includes/db/Database.php:1006
/usr/home/saper/public_html/pg/w/includes/db/Database.php:1031
/usr/home/saper/public_html/pg/w/tests/phpunit/includes/db/TestORMRowTest.php:82
/usr/home/saper/public_html/pg/w/tests/phpunit/MediaWikiTestCase.php:75
/usr/home/saper/public_html/pg/w/tests/phpunit/MediaWikiPHPUnitCommand.php:45
/usr/home/saper/public_html/pg/w/tests/phpunit/phpunit.php:103

--


There were 2 failures:

1) TestORMRowTest::testSave with data set #0 (array('Foobar', 42, 9000.1, true, array(13, 11, 7, 5, 3, 2), stdClass), true)
Failed asserting that false is true.

/usr/home/saper/public_html/pg/w/tests/phpunit/includes/db/ORMRowTest.php:102
/usr/home/saper/public_html/pg/w/tests/phpunit/MediaWikiTestCase.php:75
/usr/home/saper/public_html/pg/w/tests/phpunit/MediaWikiPHPUnitCommand.php:45
/usr/home/saper/public_html/pg/w/tests/phpunit/phpunit.php:103

2) TestORMRowTest::testRemove with data set #0 (array('Foobar', 42, 9000.1, true, array(13, 11, 7, 5, 3, 2), stdClass), true)
Failed asserting that false is true.

/usr/home/saper/public_html/pg/w/tests/phpunit/includes/db/ORMRowTest.php:117
/usr/home/saper/public_html/pg/w/tests/phpunit/MediaWikiTestCase.php:75
/usr/home/saper/public_html/pg/w/tests/phpunit/MediaWikiPHPUnitCommand.php:45
/usr/home/saper/public_html/pg/w/tests/phpunit/phpunit.php:103

FAILURES!
Tests: 3, Assertions: 6, Failures: 2, Errors: 1.
+ exit
Comment 1 Aaron Schulz 2012-06-14 18:04:16 UTC
Nothing should be using safeQuery() like that anyway.
Comment 2 Tim Landscheidt 2012-09-19 21:54:41 UTC
Created attachment 11128 [details]
Patch for table creation in test case.

The attached patch solves the table creation bug (although the BLOB data type needs more thought), but the actual test then fails with:

| There was 1 error:

| 1) TestORMRowTest::testSave with data set #0 (array('Foobar', 42, 9000.1, true, array(13, 11, 7, 5, 3, 2), stdClass), true)
| DBQueryError: A database error has occurred.  Did you forget to run maintenance/update.php after upgrading?  See: https://www.mediawiki.org/wiki/Manual:Upgrading#Run_the_update_script
| Query: INSERT INTO "unittest_orm_test" (test_id,test_name,test_age,test_height,test_awesome,test_stuff,test_moarstuff) VALUES (NULL,'Foobar','42','9000.1',1,'a:6:{i:0;i:13;i:1;i:11;i:2;i:7;i:3;i:5;i:4;i:3;i:5;i:2;}','O:8:"stdClass":3:{s:3:"foo";s:3:"bar";s:3:"bar";a:2:{i:0;i:4;i:1;i:2;}s:3:"baz";b:1;}')
| Function: ORMRow::insert
| Error: 23502 FEHLER:  NULL-Wert in Spalte »test_id« verletzt Not-Null-Constraint

This is due to explicitly passing NULL as the value for the ID column which in MySQL means "use next value", but in PostgreSQL means "use NULL".  It can be worked around by passing the keyword "DEFAULT" (without quotes) as the value, e. g. "INSERT INTO t (c) VALUES (DEFAULT);".

Jeroen, what do you think is the best approach to support PostgreSQL here?  From a glance at the code, I would probably exclude ID columns in ORMRow::getWriteValues() when their value is null.
Comment 3 Jeroen De Dauw 2012-09-24 11:18:57 UTC
Marcin: thanks for pointing this out

Aaron: the ORMTable/Row code is not using safeQuery. I can in fact not find the method, so am guessing it got killed?

Tim Landscheidt: thanks for the patch. Will also try to look at the blob issue soonish.
Comment 4 Tim Landscheidt 2012-09-25 13:31:19 UTC
(In reply to comment #3)
> [...]
> Tim Landscheidt: thanks for the patch. Will also try to look at the blob issue
> soonish.

Just to clarify: There isn't necessarily an "issue" with PostgreSQL's blob data type, it just has to be reviewed that BYTEA is an appropriate data type in this case as well as whether for example test_age's TINYINT unsigned can be translated to SMALLINT or should be translated to "SMALLINT CHECK(test_age >= 0)".  As this is "just" a test case with the emphasis on the ORM part, it's probably not that important, but I wanted to have pointed it out.
Comment 5 Tim Landscheidt 2012-12-28 01:39:49 UTC
*** Bug 38930 has been marked as a duplicate of this bug. ***
Comment 6 Gerrit Notification Bot 2013-12-07 21:01:21 UTC
Change 100180 had a related patch set uploaded by saper:
Database independent test for bug 37601

https://gerrit.wikimedia.org/r/100180
Comment 7 Marcin Cieślak 2013-12-07 21:09:38 UTC
Reverting state to NEW:

Gerrit change Iebe9d6ff73fac930b456b24b6317cafb56d0163f is a test case to expose the failure in the database-independent way.

It seems to me that ORMRow::getWriteValues() had a test preventing "id"
fields and values from being ever written to the database (rightly so).

ece97c358424015777db3881d87aa33966d634bd has moved this responsibility from the row-level to the table level, but the test has been removed, therefore this bug.

I am not sure what is the proper place to fix this - to reintroduce the following lines from ORMRow into ORMTable

   227  // Skip null id fields so that the DBMS can set the default.
   228  if ( $name === 'id' && is_null( $value ) ) {
   229        continue;
   230  }

The test introduced in Iebe9d6ff73fac930b456b24b6317cafb56d0163f would be more elegant if there were some way to test getWriteValues() directly; maybe it even belongs to ORMTableTest instead but I could not figure out how to inject required IORMRow dependency into that test and lost too much energy trying.
Comment 8 Gerrit Notification Bot 2013-12-07 21:59:00 UTC
Change 100183 had a related patch set uploaded by saper:
Teach ORMTable to use sequences

https://gerrit.wikimedia.org/r/100183
Comment 9 Marcin Cieślak 2013-12-07 22:09:37 UTC
This is a very very early attempt to do something about primary keys.

With Gerrit change #100183 PostgreSQL and MySQL sucessfully passes tests/phpunit/includes/db/TestORMRowTest.phptests/phpunit/includes/db/TestORMRowTest.php in isolation, but fails when a whole test suite is run:


10) TestORMRowTest::testSaveAndRemove with data set #0 (array('Foobar', '2012-01-01 02:02:02 GMT', 42, 9000.1, true, array(13, 11, 7, 5, 3, 2), stdClass), true)
DBQueryError: A database error has occurred. Did you forget to run maintenance/update.php after upgrading?  See: https://www.mediawiki.org/wiki/Manual:Upgrading#Run_the_update_script
Query: SELECT nextval('orm_test_test_id_seq')
Function: DatabaseBase::query
Error: 42P01 ERROR:  relation "orm_test_test_id_seq" does not exist
LINE 1: ...ELECT /* DatabaseBase::query 127.0.0.1 */ nextval('orm_test_...
                                                             ^



/usr/home/saper/test/mytest/includes/db/Database.php:1111
/usr/home/saper/test/mytest/includes/db/DatabasePostgres.php:511
/usr/home/saper/test/mytest/includes/db/Database.php:1077
/usr/home/saper/test/mytest/includes/db/DatabasePostgres.php:991
/usr/home/saper/test/mytest/includes/db/ORMTable.php:1025
/usr/home/saper/test/mytest/includes/db/ORMRow.php:352
/usr/home/saper/test/mytest/tests/phpunit/includes/db/ORMRowTest.php:165
/usr/home/saper/test/mytest/tests/phpunit/MediaWikiTestCase.php:123
/usr/home/saper/test/mytest/tests/phpunit/MediaWikiPHPUnitCommand.php:80
/usr/home/saper/test/mytest/tests/phpunit/MediaWikiPHPUnitCommand.php:64
/usr/home/saper/test/mytest/tests/phpunit/phpunit.php:115

I am kind of at loss to figure out whether we are using mocked table and row instance but still somehow touch the database anyway; should we also mock sequences???  This is getting crazy.

Btw, shouldn't those tests be moved into @Database groups since the CREATE TABLEs and stuff?
Comment 10 Marcin Cieślak 2013-12-07 22:29:30 UTC
I tend to believe that this patch fixes bug 43475 somehow, but tests are still broken.
Comment 11 Gerrit Notification Bot 2014-11-12 00:51:18 UTC
Change 172662 had a related patch set uploaded by saper:
Mark TestORMRowTest as broken

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

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links