15.04 Unit tests fail using MySql

Bug #1515473 reported by Ghada El-Zoghbi on 2015-11-12
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Mahara
Low
Unassigned
15.04
Low
Unassigned
15.10
Low
Unassigned

Bug Description

To replicate:

1. Install Mahara on MySQL
2. In the terminal, cd to your docroot (e.g. /var/www/mahara)
3. run "make phpunit"

Expected outcome: Something like "........SSS Time: 9.42 seconds, Memory: 29.75Mb"
Actual outcome: Big ugly error stack, and "........EEE"

Mahara:
$config->version = 2015030419;
$config->series = '15.04';
$config->release = '15.04.4';

DB: mysql 5.5
(mysql Ver 14.14 Distrib 5.5.46, for debian-linux-gnu (x86_64) using readline 6.3)

When running unit tests on a vanilla Mahara 15.04.4 with a mysql database, 3 tests fail:

Configuration read from /var/www/palnet/mahara/phpunit.xml

........EEE

Time: 39.5 seconds, Memory: 36.00Mb

There were 3 errors:

1) WebServiceGroupTest::testRun
SQLException: Failed to get a recordset: mysqli error: [1064: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '"tst_config" WHERE "field" = 'webservice_enabled' LIMIT 2' at line 1] in EXECUTE("SELECT * FROM "tst_config" WHERE "field" = 'webservice_enabled' LIMIT 2")
Command was: SELECT * FROM "tst_config" WHERE "field" = ? and values was (webservice_enabled)

/var/www/palnet/mahara/htdocs/lib/dml.php:480
/var/www/palnet/mahara/htdocs/lib/dml.php:314
/var/www/palnet/mahara/htdocs/lib/dml.php:291
/var/www/palnet/mahara/htdocs/lib/mahara.php:885
/var/www/palnet/mahara/htdocs/webservice/tests/phpunit/WebServiceTestBase.class.php:85
/var/www/palnet/mahara/htdocs/webservice/tests/phpunit/WebServiceGroupTest.php:28

2) WebServiceInstitutionTest::testRun
SQLException: Failed to get a recordset: mysqli error: [1064: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '"tst_config" WHERE "field" = 'webservice_enabled' LIMIT 2' at line 1] in EXECUTE("SELECT * FROM "tst_config" WHERE "field" = 'webservice_enabled' LIMIT 2")
Command was: SELECT * FROM "tst_config" WHERE "field" = ? and values was (webservice_enabled)

/var/www/palnet/mahara/htdocs/lib/dml.php:480
/var/www/palnet/mahara/htdocs/lib/dml.php:314
/var/www/palnet/mahara/htdocs/lib/dml.php:291
/var/www/palnet/mahara/htdocs/lib/mahara.php:885
/var/www/palnet/mahara/htdocs/webservice/tests/phpunit/WebServiceTestBase.class.php:85
/var/www/palnet/mahara/htdocs/webservice/tests/phpunit/WebServiceInstitutionTest.php:28

3) WebServiceUserTest::testRun
SQLException: Failed to get a recordset: mysqli error: [1064: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '"tst_config" WHERE "field" = 'webservice_enabled' LIMIT 2' at line 1] in EXECUTE("SELECT * FROM "tst_config" WHERE "field" = 'webservice_enabled' LIMIT 2")
Command was: SELECT * FROM "tst_config" WHERE "field" = ? and values was (webservice_enabled)

/var/www/palnet/mahara/htdocs/lib/dml.php:480
/var/www/palnet/mahara/htdocs/lib/dml.php:314
/var/www/palnet/mahara/htdocs/lib/dml.php:291
/var/www/palnet/mahara/htdocs/lib/mahara.php:885
/var/www/palnet/mahara/htdocs/webservice/tests/phpunit/WebServiceTestBase.class.php:85
/var/www/palnet/mahara/htdocs/webservice/tests/phpunit/WebServiceUserTest.php:28

FAILURES!
Tests: 11, Assertions: 21, Errors: 3.
make: *** [phpunit] Error 2

-----------------------------------------------------------------------------------------------
On the same vanilla Mahara 15.04.4, the unit tests pass using postgres:

PHPUnit 4.3.4 by Sebastian Bergmann.

Configuration read from /var/www/palnet/mahara/phpunit.xml

........SSS

Time: 38.13 seconds, Memory: 35.25Mb

OK, but incomplete, skipped, or risky tests!
Tests: 11, Assertions: 21, Skipped: 3.

Do we still use unit tests?

Aaron Wells (u-aaronw) wrote :

Well, we have a (very few) written, including these three from the webservices. And we have the infrastructure for writing and running more of them. But it has not been our policy to routinely write unit tests, because Mahara wasn't originally written with unit testing in mind, and hence it would require a lot of refactoring to make most of it unit testable.

The three tests that are failing in MySQL are the ones that Piers wrote for the webservices module, which we disabled because they need some non-trivial shims to make it possible to run those tests successfully. (Specifically, the tests send out curl requests to Mahara's webservices; but the instance of Mahara being accessed by that curl URL is not the same "test instance" that phpunit is using.)

That's why they show up as "SSS" on the end of the Postgres results. The "S" indicates we put a flag on the test saying to skip it.

It's strange that they're not being skipped in MySQL. :-\

Ghada El-Zoghbi (ghada-z) wrote :

Hi Aaron,

Yes, it's strange. Our Continuous Integration is running them and failing for a project that uses MySql.

That's why I tested it with a Vanilla Mahara and got the same results (to make sure I wasn't going crazy).

Ghada

Aaron Wells (u-aaronw) wrote :

Okay, I found a few things going on here:

1. I put the "markTestSkipped" command in the "testRun()" method. So, the "setup()" method still runs, unnecessarily, for the webservices tests.

2. The WebServiceTestBase class fails to call parent::setup() during its setup() method. That's what is causing this specific error.

3. Our phpunit infrastructure is a bit broken. It doesn't tear everything down properly, so if you do "make phpunit" twice on the same database, you get errors the second time when it tries to re-create things that still exist. I think this is because we have some database items that are not defined in our install.xml files. These are mostly indexes and constraints that are too complex to specify in xmldb format, so they're created directly with SQL queries during installation. But this isn't a MySQL-specific problem. It happens in Postgres too.

I'll put in patches right now to fix 1 & 2 since they're one-liners. Number 3 will need to go in a different bug I think.

Cheers,
Aaron

Mahara Bot (dev-mahara) wrote :

Patch for "master" branch: https://reviews.mahara.org/5708

Aaron Wells (u-aaronw) wrote :

Created Bug 1515794 to track the separate issue of incomplete phpunit teardown.

description: updated
Aaron Wells (u-aaronw) wrote :

These two patches can be backported to 15.10 and 15.04, because they only affect dev-related code, not end-user code. (All the phpunit code gets removed by the release packaging script.)

Reviewed: https://reviews.mahara.org/5707
Committed: https://git.mahara.org/mahara/mahara/commit/b89579095531e938a8175cbe7548bf0acd966e8e
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit b89579095531e938a8175cbe7548bf0acd966e8e
Author: Aaron Wells <email address hidden>
Date: Fri Nov 13 12:22:36 2015 +1300

Call parent::setUp() in WebServiceTestBase (Bug 1515473)

behatnotneeded: This is phpunit, not behat. ;)

Change-Id: I906089c82c6e5ca521007359006b49381c537f2b

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/5708
Committed: https://git.mahara.org/mahara/mahara/commit/2f0b7d113ae5806ebe2de0943469d5265b9d9b46
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 2f0b7d113ae5806ebe2de0943469d5265b9d9b46
Author: Aaron Wells <email address hidden>
Date: Fri Nov 13 12:23:58 2015 +1300

Move WebService "markTestSkipped" to setup (Bug 1515473)

... so that we don't bother doing setup just to get
to testRun() and realize we're skipping this test

behatnotneeded: phpunit, not behat

Change-Id: I0c96e364129435f9eca3f0cb3bb44beaf9db4701

Mahara Bot (dev-mahara) wrote :

Patch for "15.10_STABLE" branch: https://reviews.mahara.org/6229

Mahara Bot (dev-mahara) wrote :

Patch for "15.04_STABLE" branch: https://reviews.mahara.org/6230

Mahara Bot (dev-mahara) wrote :

Patch for "15.04_STABLE" branch: https://reviews.mahara.org/6231

Reviewed: https://reviews.mahara.org/6230
Committed: https://git.mahara.org/mahara/mahara/commit/e4652530e67f7fa8e107d927ce9d362183dea5a4
Submitter: Robert Lyon (<email address hidden>)
Branch: 15.04_STABLE

commit e4652530e67f7fa8e107d927ce9d362183dea5a4
Author: Aaron Wells <email address hidden>
Date: Fri Nov 13 12:22:36 2015 +1300

Call parent::setUp() in WebServiceTestBase (Bug 1515473)

behatnotneeded: This is phpunit, not behat. ;)

Change-Id: I906089c82c6e5ca521007359006b49381c537f2b
(cherry picked from commit b89579095531e938a8175cbe7548bf0acd966e8e)

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/6231
Committed: https://git.mahara.org/mahara/mahara/commit/ba3f804f06dff1a698d3e79868580b7815fdaf2f
Submitter: Robert Lyon (<email address hidden>)
Branch: 15.04_STABLE

commit ba3f804f06dff1a698d3e79868580b7815fdaf2f
Author: Aaron Wells <email address hidden>
Date: Fri Nov 13 12:23:58 2015 +1300

Move WebService "markTestSkipped" to setup (Bug 1515473)

... so that we don't bother doing setup just to get
to testRun() and realize we're skipping this test

behatnotneeded: phpunit, not behat

Change-Id: I0c96e364129435f9eca3f0cb3bb44beaf9db4701

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/6228
Committed: https://git.mahara.org/mahara/mahara/commit/48ea9ab8ae39d04bcebba8d22497ac5b57e85585
Submitter: Robert Lyon (<email address hidden>)
Branch: 15.10_STABLE

commit 48ea9ab8ae39d04bcebba8d22497ac5b57e85585
Author: Aaron Wells <email address hidden>
Date: Fri Nov 13 12:22:36 2015 +1300

Call parent::setUp() in WebServiceTestBase (Bug 1515473)

behatnotneeded: This is phpunit, not behat. ;)

Change-Id: I906089c82c6e5ca521007359006b49381c537f2b
(cherry picked from commit b89579095531e938a8175cbe7548bf0acd966e8e)

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/6229
Committed: https://git.mahara.org/mahara/mahara/commit/defaf5d46cb6c73e0ee56c26b215261b546b1239
Submitter: Robert Lyon (<email address hidden>)
Branch: 15.10_STABLE

commit defaf5d46cb6c73e0ee56c26b215261b546b1239
Author: Aaron Wells <email address hidden>
Date: Fri Nov 13 12:23:58 2015 +1300

Move WebService "markTestSkipped" to setup (Bug 1515473)

... so that we don't bother doing setup just to get
to testRun() and realize we're skipping this test

behatnotneeded: phpunit, not behat

Change-Id: I0c96e364129435f9eca3f0cb3bb44beaf9db4701

no longer affects: mahara/16.04
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers