r93980 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r93979‎ | r93980 | r93981 >
Date:15:17, 5 August 2011
Author:ialex
Status:ok (Comments)
Tags:todo 
Comment:
* Call Linker methods statically
* Use INSERT IGNORE for the site_stats row (as for the interwiki entries); was throwing query error about duplicate row on my installation
Modified paths:
  • /trunk/phase3/tests/parser/parserTest.inc (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/parser/NewParserTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/parser/NewParserTest.php
——@@ -164,7 +164,7 @@
165165 # Update certain things in site_stats
166166 $this->db->insert( 'site_stats',
167167 array( 'ss_row_id' => 1, 'ss_images' => 2, 'ss_good_articles' => 1 ),
168 - __METHOD__ );
 168+ __METHOD__, array( 'IGNORE' ) );
169169
170170 # Reinitialise the LocalisationCache to match the database state
171171 Language::getLocalisationCache()->unloadAll();
——@@ -500,8 +500,7 @@
501501 $replace = $opts['replace'][1];
502502 $out = $parser->replaceSection( $input, $section, $replace );
503503 } elseif ( isset( $opts['comment'] ) ) {
504 - $linker = $user->getSkin();
505 - $out = $linker->formatComment( $input, $title, $local );
 504+ $out = Linker::formatComment( $input, $title, $local );
506505 } elseif ( isset( $opts['preload'] ) ) {
507506 $out = $parser->getpreloadText( $input, $title, $options );
508507 } else {
Index: trunk/phase3/tests/parser/parserTest.inc
——@@ -452,8 +452,7 @@
453453 $replace = $opts['replace'][1];
454454 $out = $parser->replaceSection( $input, $section, $replace );
455455 } elseif ( isset( $opts['comment'] ) ) {
456 - $linker = $user->getSkin();
457 - $out = $linker->formatComment( $input, $title, $local );
 456+ $out = Linker::formatComment( $input, $title, $local );
458457 } elseif ( isset( $opts['preload'] ) ) {
459458 $out = $parser->getpreloadText( $input, $title, $options );
460459 } else {

Comments

#Comment by 😂 (talk | contribs)   15:23, 5 August 2011

I just removed that IGNORE because it wasn't throwing errors for me anymore. See r93933, r88772. Stack trace?

#Comment by IAlex (talk | contribs)   13:56, 6 August 2011

After some investigation, there are two things:

  • DatabaseSqliteTest calls SiteStatsInit::doAllAndCommit() which calls refresh() that does a DELETE/INSERT on the site_stats table, adding the row
  • That function is called once per Parser test, i.e. about 500 times when running the test suite.
#Comment by Hashar (talk | contribs)   08:26, 10 August 2011

Cant we disable site_stats when running parser tests?

#Comment by 😂 (talk | contribs)   16:55, 16 August 2011

If everything actually updated stats via SiteStats, then yeah we probably could. Unfortunately we've got a lot of manual site_stats queries all over the place.

Status & tagging log