From 09ff912ddd3c8b153cdd49082bfc8eabb3b522e5 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Thu, 7 Apr 2016 15:20:38 +1000 Subject: [PATCH] parserTests.php: fix three bitrot bugs with --record * Add a subtest index to the recorded test name, to avoid an SQL duplicate key error. * Introduce TestFileDataProvider. Previously, the order of named parameters in TestFileIterator was relevant but undocumented, so adding a subtest parameter broke phpunit tests. * Don't implicitly commit (commitMasterChanges) an explicit transaction, since that now causes a fatal error. * Reset namespace cache as in NewParserTest.php, so that the MemoryAlpha article insertion doesn't fail. This was only visible with --record because the namespace cache is initialised by SpecialVersion::getVersion() during recorder setup. Change-Id: Ied4636b4acbf1d268e45901fed4d4e077b5ed666 --- tests/TestsAutoLoader.php | 1 + tests/parser/parserTest.inc | 6 +- .../phpunit/includes/parser/NewParserTest.php | 2 +- tests/testHelpers.inc | 58 +++++++++++++++---- 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/tests/TestsAutoLoader.php b/tests/TestsAutoLoader.php index c8b466145d..26085b8239 100644 --- a/tests/TestsAutoLoader.php +++ b/tests/TestsAutoLoader.php @@ -32,6 +32,7 @@ $wgAutoloadClasses += [ 'DelayedParserTest' => "$testDir/testHelpers.inc", 'ParserTestResult' => "$testDir/parser/ParserTestResult.php", 'TestFileIterator' => "$testDir/testHelpers.inc", + 'TestFileDataProvider' => "$testDir/testHelpers.inc", 'TestRecorder' => "$testDir/testHelpers.inc", 'ITestRecorder' => "$testDir/testHelpers.inc", 'DjVuSupport' => "$testDir/testHelpers.inc", diff --git a/tests/parser/parserTest.inc b/tests/parser/parserTest.inc index 56108c96e8..78e5f6f9c4 100644 --- a/tests/parser/parserTest.inc +++ b/tests/parser/parserTest.inc @@ -244,6 +244,10 @@ class ParserTest { // "extra language links" // see https://gerrit.wikimedia.org/r/111390 array_push( $wgExtraInterlanguageLinkPrefixes, 'mul' ); + + // Reset namespace cache + MWNamespace::getCanonicalNamespaces( true ); + Language::factory( 'en' )->resetNamespaces(); } /** @@ -536,7 +540,7 @@ class ParserTest { $result = $this->runTest( $t['test'], $t['input'], $t['result'], $t['options'], $t['config'] ); $ok = $ok && $result; - $this->recorder->record( $t['test'], $result ); + $this->recorder->record( $t['test'], $t['subtest'], $result ); } if ( $this->showProgress ) { diff --git a/tests/phpunit/includes/parser/NewParserTest.php b/tests/phpunit/includes/parser/NewParserTest.php index b1119a1925..22bb23784f 100644 --- a/tests/phpunit/includes/parser/NewParserTest.php +++ b/tests/phpunit/includes/parser/NewParserTest.php @@ -680,7 +680,7 @@ class NewParserTest extends MediaWikiTestCase { $this->file = $wgParserTestFiles[0]; } - return new TestFileIterator( $this->file, $this ); + return new TestFileDataProvider( $this->file, $this ); } /** diff --git a/tests/testHelpers.inc b/tests/testHelpers.inc index 76544a50bc..d04e0fcb54 100644 --- a/tests/testHelpers.inc +++ b/tests/testHelpers.inc @@ -44,9 +44,10 @@ interface ITestRecorder { /** * Called after each test * @param string $test + * @param integer $subtest * @param bool $result */ - public function record( $test, $result ); + public function record( $test, $subtest, $result ); /** * Called before finishing the test run @@ -74,7 +75,7 @@ class TestRecorder implements ITestRecorder { $this->success = 0; } - function record( $test, $result ) { + function record( $test, $subtest, $result ) { $this->total++; $this->success += ( $result ? 1 : 0 ); } @@ -147,9 +148,17 @@ class DbTestPreviewer extends TestRecorder { $this->results = []; } - function record( $test, $result ) { - parent::record( $test, $result ); - $this->results[$test] = $result; + function getName( $test, $subtest ) { + if ( $subtest ) { + return "$test subtest #$subtest"; + } else { + return $test; + } + } + + function record( $test, $subtest, $result ) { + parent::record( $test, $subtest, $result ); + $this->results[ $this->getName( $test, $subtest ) ] = $result; } function report() { @@ -299,10 +308,9 @@ class DbTestPreviewer extends TestRecorder { } /** - * Commit transaction and clean up for result recording + * Close the DB connection */ function end() { - $this->lb->commitMasterChanges(); $this->lb->closeAll(); parent::end(); } @@ -350,17 +358,25 @@ class DbTestRecorder extends DbTestPreviewer { * @param string $test * @param bool $result */ - function record( $test, $result ) { - parent::record( $test, $result ); + function record( $test, $subtest, $result ) { + parent::record( $test, $subtest, $result ); $this->db->insert( 'testitem', [ 'ti_run' => $this->curRun, - 'ti_name' => $test, + 'ti_name' => $this->getName( $test, $subtest ), 'ti_success' => $result ? 1 : 0, ], __METHOD__ ); } + + /** + * Commit transaction and clean up for result recording + */ + function end() { + $this->db->commit( __METHOD__ ); + parent::end(); + } } class TestFileIterator implements Iterator { @@ -479,6 +495,7 @@ class TestFileIterator implements Iterator { $this->test = [ 'test' => ParserTest::chomp( $this->sectionData['test'] ), + 'subtest' => $this->nextSubTest, 'input' => ParserTest::chomp( $this->sectionData[$input] ), 'result' => ParserTest::chomp( $this->sectionData[$result] ), 'options' => ParserTest::chomp( $this->sectionData['options'] ), @@ -665,6 +682,27 @@ class TestFileIterator implements Iterator { } } +/** + * An iterator for use as a phpunit data provider. Provides the test arguments + * in the order expected by NewParserTest::testParserTest(). + */ +class TestFileDataProvider extends TestFileIterator { + function current() { + $test = parent::current(); + if ( $test ) { + return [ + $test['test'], + $test['input'], + $test['result'], + $test['options'], + $test['config'], + ]; + } else { + return $test; + } + } +} + /** * A class to delay execution of a parser test hooks. */ -- 2.20.1