I’m trying to improve my coding practices for our MW extensions, and I’m very confused by whether my problem with PHPUnit is a problem with the test I’m writing or with the extension object class I’m trying to test.
My extension uses some custom database tables to store the properties of objects that are created during execution. In older versions of MW, I could just create an object and test that the methods of the object class returned appropriate values for known data that was already in the database. Now, instantiating an object is creating temporary tables that don’t exist and causing fatal errors. Example:
7) CacaoModelAnnotationTest::testNewCacaoModelAnnotationSuccess Wikimedia\Rdbms\DBQueryError: A database query error has occurred. Did you forget to run your application's database schema updater after upgrading? Query: SELECT annotation_id,row_id,original_row_data,annotation_timestamp,user_id,team_id,session_id,annotation_inning FROM `unittest_cacao_annotation` INNER JOIN `unittest_cacao_user` ON ((annotation_user = cacao_user.id)) WHERE annotation_id = "6057" Function: CacaoModelAnnotation::load Error: 1054 Unknown column 'cacao_user.id' in 'on clause' (localhost)
This is the load method from class CacaoModelAnnotation:
public function load() { try{ wfProfileIn( __METHOD__ ); if ( $this->loaded ) { return false; }
MWDebug::log( __METHOD__ . ' called for annotation_id: ' . $this->id );
$dbr = wfGetDB( DB_SLAVE ); // query to get annotation attributes $result = $dbr->select( array( 'cacao_annotation', 'cacao_user' ), array( 'annotation_id', 'row_id', 'original_row_data', 'annotation_timestamp', 'user_id', 'team_id', 'session_id', 'annotation_inning' ), 'annotation_id = "' . $this->id . '"', __METHOD__, array(), array( 'cacao_user' => array('INNER JOIN', 'annotation_user = cacao_user.id' ), ) ); if ( $dbr->numRows($result) == 0 ) { throw new CacaoException( wfMessage('cacao-annotation-load-returned-zero-rows', $this->id)->text() ); } $resultRow = $dbr->fetchObject( $result ); $this->loadFromResultRow($resultRow); wfProfileOut( __METHOD__ ); }catch(CacaoException $e){ if(!isset($thrown)){ throw $e; } $thrown = true; } }
Is there a good example of an extension that uses custom database tables that I can use as a role model?
Hi!
First of all: You should try to avoid accessing the database in tests, if possible. E.g. separating the logic of your extension from the database code. If you really need the DB, the Database section of the Writing unit test page on mediawiki.org[1] should explain how you can make the database table available during test and how to use it.
Another common way of writing tests without relying on the database is to mock objects and simulate how objects, on which the code under test relies on, behaves under defined circumstances. For more information see the PHPUnit Test doubles page[2].
One hint unrelated to your question: The code you pasted looks a bit confusing: You throw an exception (there's only one place where it is thrown from the code you're showing in this method) directly inside a try/catch block, which only catches this exception. That doesn't make much sense, I would recommend to directly do what you need to do in the error case. Also: Why do you check for a local variable $thrown, which can only be set inside the catch block as the last statement of the method?
[1] https://www.mediawiki.org/wiki/Manual:PHP_unit_testing/Writing_unit_tests#Da... [2] https://phpunit.de/manual/current/en/test-doubles.html
Best, Florian
-----Ursprüngliche Nachricht----- Von: Wikitech-l [mailto:wikitech-l-bounces@lists.wikimedia.org] Im Auftrag von Jim Hu Gesendet: Mittwoch, 27. September 2017 17:31 An: Wikitech mailing list wikitech-l@lists.wikimedia.org Betreff: [Wikitech-l] Writing unit tests for extensions that use dbr calls
I’m trying to improve my coding practices for our MW extensions, and I’m very confused by whether my problem with PHPUnit is a problem with the test I’m writing or with the extension object class I’m trying to test.
My extension uses some custom database tables to store the properties of objects that are created during execution. In older versions of MW, I could just create an object and test that the methods of the object class returned appropriate values for known data that was already in the database. Now, instantiating an object is creating temporary tables that don’t exist and causing fatal errors. Example:
7) CacaoModelAnnotationTest::testNewCacaoModelAnnotationSuccess Wikimedia\Rdbms\DBQueryError: A database query error has occurred. Did you forget to run your application's database schema updater after upgrading? Query: SELECT annotation_id,row_id,original_row_data,annotation_timestamp,user_id,team_id,session_id,annotation_inning FROM `unittest_cacao_annotation` INNER JOIN `unittest_cacao_user` ON ((annotation_user = cacao_user.id)) WHERE annotation_id = "6057" Function: CacaoModelAnnotation::load Error: 1054 Unknown column 'cacao_user.id' in 'on clause' (localhost)
This is the load method from class CacaoModelAnnotation:
public function load() { try{ wfProfileIn( __METHOD__ ); if ( $this->loaded ) { return false; }
MWDebug::log( __METHOD__ . ' called for annotation_id: ' . $this->id );
$dbr = wfGetDB( DB_SLAVE ); // query to get annotation attributes $result = $dbr->select( array( 'cacao_annotation', 'cacao_user' ), array( 'annotation_id', 'row_id', 'original_row_data', 'annotation_timestamp', 'user_id', 'team_id', 'session_id', 'annotation_inning' ), 'annotation_id = "' . $this->id . '"', __METHOD__, array(), array( 'cacao_user' => array('INNER JOIN', 'annotation_user = cacao_user.id' ), ) ); if ( $dbr->numRows($result) == 0 ) { throw new CacaoException( wfMessage('cacao-annotation-load-returned-zero-rows', $this->id)->text() ); } $resultRow = $dbr->fetchObject( $result ); $this->loadFromResultRow($resultRow); wfProfileOut( __METHOD__ ); }catch(CacaoException $e){ if(!isset($thrown)){ throw $e; } $thrown = true; } }
Is there a good example of an extension that uses custom database tables that I can use as a role model? _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Note that I’m not accessing the code directly in the test, but I am in the extension object I’m testing.
Yeah, my code is not great, although this is actually legacy from one of my former people. A lot of what you point out should be cleaned up, but I didn’t want to do that here in case it was related to why things were acting up. I was wondering about that $thrown myself.
Jim
On Sep 27, 2017, at 11:04 AM, Florian Schmidt florian.schmidt.welzow@t-online.de wrote:
Hi!
First of all: You should try to avoid accessing the database in tests, if possible. E.g. separating the logic of your extension from the database code. If you really need the DB, the Database section of the Writing unit test page on mediawiki.org[1] should explain how you can make the database table available during test and how to use it.
Another common way of writing tests without relying on the database is to mock objects and simulate how objects, on which the code under test relies on, behaves under defined circumstances. For more information see the PHPUnit Test doubles page[2].
One hint unrelated to your question: The code you pasted looks a bit confusing: You throw an exception (there's only one place where it is thrown from the code you're showing in this method) directly inside a try/catch block, which only catches this exception. That doesn't make much sense, I would recommend to directly do what you need to do in the error case. Also: Why do you check for a local variable $thrown, which can only be set inside the catch block as the last statement of the method?
[1] https://www.mediawiki.org/wiki/Manual:PHP_unit_testing/Writing_unit_tests#Da... [2] https://phpunit.de/manual/current/en/test-doubles.html
Best, Florian
-----Ursprüngliche Nachricht----- Von: Wikitech-l [mailto:wikitech-l-bounces@lists.wikimedia.org] Im Auftrag von Jim Hu Gesendet: Mittwoch, 27. September 2017 17:31 An: Wikitech mailing list wikitech-l@lists.wikimedia.org Betreff: [Wikitech-l] Writing unit tests for extensions that use dbr calls
I’m trying to improve my coding practices for our MW extensions, and I’m very confused by whether my problem with PHPUnit is a problem with the test I’m writing or with the extension object class I’m trying to test.
My extension uses some custom database tables to store the properties of objects that are created during execution. In older versions of MW, I could just create an object and test that the methods of the object class returned appropriate values for known data that was already in the database. Now, instantiating an object is creating temporary tables that don’t exist and causing fatal errors. Example:
- CacaoModelAnnotationTest::testNewCacaoModelAnnotationSuccess
Wikimedia\Rdbms\DBQueryError: A database query error has occurred. Did you forget to run your application's database schema updater after upgrading? Query: SELECT annotation_id,row_id,original_row_data,annotation_timestamp,user_id,team_id,session_id,annotation_inning FROM `unittest_cacao_annotation` INNER JOIN `unittest_cacao_user` ON ((annotation_user = cacao_user.id)) WHERE annotation_id = "6057" Function: CacaoModelAnnotation::load Error: 1054 Unknown column 'cacao_user.id' in 'on clause' (localhost)
This is the load method from class CacaoModelAnnotation:
public function load() { try{ wfProfileIn( __METHOD__ );
if ( $this->loaded ) { return false; } MWDebug::log( __METHOD__ . ' called for annotation_id: ' . $this->id ); $dbr = wfGetDB( DB_SLAVE );
// query to get annotation attributes $result = $dbr->select( array( 'cacao_annotation', 'cacao_user' ), array( 'annotation_id', 'row_id', 'original_row_data', 'annotation_timestamp', 'user_id', 'team_id', 'session_id', 'annotation_inning' ), 'annotation_id = "' . $this->id . '"', __METHOD__, array(), array( 'cacao_user' => array('INNER JOIN', 'annotation_user = cacao_user.id' ), ) ); if ( $dbr->numRows($result) == 0 ) { throw new CacaoException( wfMessage('cacao-annotation-load-returned-zero-rows', $this->id)->text() ); } $resultRow = $dbr->fetchObject( $result ); $this->loadFromResultRow($resultRow); wfProfileOut( __METHOD__ ); }catch(CacaoException $e){ if(!isset($thrown)){ throw $e; } $thrown = true; } }
Is there a good example of an extension that uses custom database tables that I can use as a role model? _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Try adding
$this->tablesUsed[] = 'cacao_annotation';
To the setUp() method of your tests
-- brian
On Wednesday, September 27, 2017, Jim Hu jim.hu.biobio@gmail.com wrote:
I’m trying to improve my coding practices for our MW extensions, and I’m
very confused by whether my problem with PHPUnit is a problem with the test I’m writing or with the extension object class I’m trying to test.
My extension uses some custom database tables to store the properties of
objects that are created during execution. In older versions of MW, I could just create an object and test that the methods of the object class returned appropriate values for known data that was already in the database. Now, instantiating an object is creating temporary tables that don’t exist and causing fatal errors. Example:
- CacaoModelAnnotationTest::testNewCacaoModelAnnotationSuccess
Wikimedia\Rdbms\DBQueryError: A database query error has occurred. Did
you forget to run your application's database schema updater after upgrading?
Query: SELECT
annotation_id,row_id,original_row_data,annotation_timestamp,user_id,team_id,session_id,annotation_inning FROM `unittest_cacao_annotation` INNER JOIN `unittest_cacao_user` ON ((annotation_user = cacao_user.id)) WHERE annotation_id = "6057"
Function: CacaoModelAnnotation::load Error: 1054 Unknown column 'cacao_user.id' in 'on clause' (localhost)
This is the load method from class CacaoModelAnnotation:
public function load() { try{ wfProfileIn( __METHOD__ ); if ( $this->loaded ) { return false; } MWDebug::log( __METHOD__ . ' called for
annotation_id: ' . $this->id );
$dbr = wfGetDB( DB_SLAVE ); // query to get annotation attributes $result = $dbr->select( array( 'cacao_annotation', 'cacao_user' ), array( 'annotation_id', 'row_id',
'original_row_data', 'annotation_timestamp',
'user_id', 'team_id',
'session_id', 'annotation_inning'
), 'annotation_id = "' . $this->id . '"', __METHOD__, array(), array( 'cacao_user' => array('INNER
JOIN', 'annotation_user = cacao_user.id' ),
) ); if ( $dbr->numRows($result) == 0 ) { throw new CacaoException(
wfMessage('cacao-annotation-load-returned-zero-rows', $this->id)->text() );
} $resultRow = $dbr->fetchObject( $result ); $this->loadFromResultRow($resultRow); wfProfileOut( __METHOD__ ); }catch(CacaoException $e){ if(!isset($thrown)){ throw $e; } $thrown = true; } }
Is there a good example of an extension that uses custom database tables
that I can use as a role model?
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 09/27/2017 11:31 AM, Jim Hu wrote:
Query: SELECT annotation_id,row_id,original_row_data,annotation_timestamp,user_id,team_id,session_id,annotation_inning FROM `unittest_cacao_annotation` INNER JOIN `unittest_cacao_user` ON ((annotation_user = cacao_user.id)) WHERE annotation_id = "6057" Function: CacaoModelAnnotation::load Error: 1054 Unknown column 'cacao_user.id' in 'on clause' (localhost)
MediaWikiTestCase sets $wgDBprefix to cause database queries to refer to temporary tables instead of the original tables. If your code does not handle table prefixes correctly, you can get such an error message.
'cacao_user' => array('INNER JOIN', 'annotation_user = cacao_user.id' ),
To make a query like this work with a table prefix, you should alias the referenced table ("cacao_user" in this case), then in the condition, refer to the table using the alias:
$result = $dbr->select( array( 'cacao_annotation', 'u' => 'cacao_user' ),
[...]
'cacao_user' => array( 'INNER JOIN', 'annotation_user = u.id' ),
wikitech-l@lists.wikimedia.org