r90483 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r90482‎ | r90483 | r90484 >
Date:20:24, 20 June 2011
Author:demon
Status:reverted (Comments)
Tags:
Comment:
Mark test incomplete for a legitimate reason like trying to serialize a PDO object--rather than imaginary ones such as blaming memory_limit being too low when the test was being stupid and lowering it for us.
Modified paths:
  • /trunk/phase3/tests/phpunit/includes/parser/NewParserTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/parser/NewParserTest.php
——@@ -551,7 +551,7 @@
552552 */
553553 function testFuzzTests() {
554554
555 - $this->markTestIncomplete( 'Breaks tesla due to memory restrictions' );
 555+ $this->markTestIncomplete( "Somebody is serializing PDO objects, that's a no-no" );
556556
557557 global $wgParserTestFiles;
558558
——@@ -564,8 +564,6 @@
565565 $dict = $this->getFuzzInput( $files );
566566 $dictSize = strlen( $dict );
567567 $logMaxLength = log( $this->maxFuzzTestLength );
568 -
569 - ini_set( 'memory_limit', $this->memoryLimit * 1048576 );
570568
571569 $user = new User;
572570 $opts = ParserOptions::newFromUser( $user );

Follow-up revisions

RevisionCommit summaryAuthorDate
r92803self rv r90483. Still needs investigating thoughdemon22:04, 21 July 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   17:57, 23 June 2011

Per notes on bug 29493 it looks like the PDO objects getting serialized happens during the approximate memory usage breakdown that gets called when we find we've hit 90% of the memory limit. This allows us to get some idea of what was going on when the test went wild and blew up memory usage.

Recommend a revert on this plus a fix of the memory breakdown; may also need to adjust that 50M upwards (PHP 5.2/5.3 have increased reported memory usage from older versions, and we're running a lot of stuff on 64bit I imagine which means higher memory usage for every zval).

#Comment by 😂 (talk | contribs)   19:26, 23 June 2011

I had just figured this same situation out last night. Great minds and all... ;-)

FWIW: I was wrong in my original suspicions...this does seem to be pretty heavily memory bound, so bumping that 50M up would be good. For unit tests I wouldn't mind putting it to something like ~250M to be safe.

#Comment by Brion VIBBER (talk | contribs)   19:53, 23 June 2011

I think we also wanted to be reasonably sure that you could throw all sorts of stuff at a fairly realistic memory limit without it crashing -- hitting memory_limit is fatal so will kill a batch process like a job runner or import/export script dead.

250's a lot higher than we use even for production (looks like 100?)

#Comment by Platonides (talk | contribs)   22:30, 22 July 2011

> This allows us to get some idea of what was going on when the test went wild and blew up memory usage

I don't think so. It's an infinite loop that only ends due to memory limit.

And yes, it is an ugly way to do it through serialize.

Status & tagging log