Writing unit tests for extensions that use dbr calls

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Writing unit tests for extensions that use dbr calls

Jim Hu-2
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
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Writing unit tests for extensions that use dbr calls

Florian Schmidt
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#Databases 
[2] https://phpunit.de/manual/current/en/test-doubles.html

Best,
Florian

-----Ursprüngliche Nachricht-----
Von: Wikitech-l [mailto:[hidden email]] Im Auftrag von Jim Hu
Gesendet: Mittwoch, 27. September 2017 17:31
An: Wikitech mailing list <[hidden email]>
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
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Writing unit tests for extensions that use dbr calls

Brian Wolff
In reply to this post by Jim Hu-2
Try adding

$this->tablesUsed[] = 'cacao_annotation';

To the setUp() method of your tests

--
brian

On Wednesday, September 27, 2017, Jim Hu <[hidden email]> 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:
>
> 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
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Writing unit tests for extensions that use dbr calls

Jim Hu-2
In reply to this post by Florian Schmidt
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 <[hidden email]> 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#Databases 
> [2] https://phpunit.de/manual/current/en/test-doubles.html
>
> Best,
> Florian
>
> -----Ursprüngliche Nachricht-----
> Von: Wikitech-l [mailto:[hidden email]] Im Auftrag von Jim Hu
> Gesendet: Mittwoch, 27. September 2017 17:31
> An: Wikitech mailing list <[hidden email]>
> 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
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Writing unit tests for extensions that use dbr calls

Kevin Israel
In reply to this post by Jim Hu-2
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' ),

--
Kevin Israel -- Wikipedia editor, MediaWiki developer
https://en.wikipedia.org/wiki/User:PleaseStand

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l