Hello Krinkle, Hashar,
I'd like you to do a code review. Please visit
https://gerrit.wikimedia.org/r/289363
to review the following change.
Change subject: Speed up password-handling in the unit tests
......................................................................
Speed up password-handling in the unit tests
* Speed up password generation and verification by setting MWOldPassword as the
default password type. Do this once, in MediaWikiTestCase::makeTestConfig(),
rather than in five different places.
* Rename '$pwhash' to '$passwordHash', for consistency. It's ugly to have both
'$passwordFactory' and '$pwhash' in the same scope.
* Make TestUser::setPasswordForUser() check first whether the desired password
is already set. This is actually the common case, since the password is reset
in the setup code for every test, but only a few tests actually change the
password.
Change-Id: I423f09ff7472b6cbde21cb709ea7c7ef9e298f18
---
M tests/phpunit/MediaWikiTestCase.php
M tests/phpunit/includes/TestUser.php
M tests/phpunit/includes/api/ApiLoginTest.php
M tests/phpunit/includes/session/BotPasswordSessionProviderTest.php
M tests/phpunit/includes/user/BotPasswordTest.php
5 files changed, 21 insertions(+), 23 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/63/289363/1
diff --git a/tests/phpunit/MediaWikiTestCase.php
b/tests/phpunit/MediaWikiTestCase.php
index 25e0e31..ce09eda 100644
--- a/tests/phpunit/MediaWikiTestCase.php
+++ b/tests/phpunit/MediaWikiTestCase.php
@@ -221,6 +221,9 @@
$defaultOverrides->set( 'ObjectCaches', $objectCaches );
$defaultOverrides->set( 'MainCacheType', CACHE_NONE );
+ // Use a fast hash algorithm to hash passwords.
+ $defaultOverrides->set( 'PasswordDefault', 'A' );
+
$testConfig = $customOverrides
? new MultiConfig( [ $customOverrides,
$defaultOverrides, $baseConfig ] )
: new MultiConfig( [ $defaultOverrides, $baseConfig ] );
diff --git a/tests/phpunit/includes/TestUser.php
b/tests/phpunit/includes/TestUser.php
index 142c77f..13bfa46 100644
--- a/tests/phpunit/includes/TestUser.php
+++ b/tests/phpunit/includes/TestUser.php
@@ -129,14 +129,16 @@
throw new MWException( "Passed User has not been added
to the database yet!" );
}
+ if ( $user->checkPassword( $password ) === true ) {
+ return; // Nothing to do.
+ }
+
$passwordFactory = new PasswordFactory();
$passwordFactory->init( RequestContext::getMain()->getConfig()
);
- // A is unsalted MD5 (thus fast) ... we don't care about
security here, this is test only
- $passwordFactory->setDefaultType( 'A' );
- $pwhash = $passwordFactory->newFromPlaintext( $password );
+ $passwordHash = $passwordFactory->newFromPlaintext( $password );
wfGetDB( DB_MASTER )->update(
'user',
- [ 'user_password' => $pwhash->toString() ],
+ [ 'user_password' => $passwordHash->toString() ],
[ 'user_id' => $user->getId() ],
__METHOD__
);
diff --git a/tests/phpunit/includes/api/ApiLoginTest.php
b/tests/phpunit/includes/api/ApiLoginTest.php
index bcd884e..510d8bf 100644
--- a/tests/phpunit/includes/api/ApiLoginTest.php
+++ b/tests/phpunit/includes/api/ApiLoginTest.php
@@ -226,8 +226,7 @@
$passwordFactory = new PasswordFactory();
$passwordFactory->init( RequestContext::getMain()->getConfig()
);
// A is unsalted MD5 (thus fast) ... we don't care about
security here, this is test only
- $passwordFactory->setDefaultType( 'A' );
- $pwhash = $passwordFactory->newFromPlaintext( 'foobaz' );
+ $passwordHash = $passwordFactory->newFromPlaintext( 'foobaz' );
$dbw = wfGetDB( DB_MASTER );
$dbw->insert(
@@ -235,7 +234,7 @@
[
'bp_user' => $centralId,
'bp_app_id' => 'foo',
- 'bp_password' => $pwhash->toString(),
+ 'bp_password' => $passwordHash->toString(),
'bp_token' => '',
'bp_restrictions' =>
MWRestrictions::newDefault()->toJson(),
'bp_grants' => '["test"]',
diff --git a/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php
b/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php
index edab0dc..d4b1587 100644
--- a/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php
+++ b/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php
@@ -65,9 +65,7 @@
public function addDBDataOnce() {
$passwordFactory = new \PasswordFactory();
$passwordFactory->init( \RequestContext::getMain()->getConfig()
);
- // A is unsalted MD5 (thus fast) ... we don't care about
security here, this is test only
- $passwordFactory->setDefaultType( 'A' );
- $pwhash = $passwordFactory->newFromPlaintext( 'foobaz' );
+ $passwordHash = $passwordFactory->newFromPlaintext( 'foobaz' );
$userId = \CentralIdLookup::factory( 'local'
)->centralIdFromName( 'UTSysop' );
@@ -82,7 +80,7 @@
[
'bp_user' => $userId,
'bp_app_id' => 'BotPasswordSessionProvider',
- 'bp_password' => $pwhash->toString(),
+ 'bp_password' => $passwordHash->toString(),
'bp_token' => 'token!',
'bp_restrictions' =>
'{"IPAddresses":["127.0.0.0/8"]}',
'bp_grants' => '["test"]',
diff --git a/tests/phpunit/includes/user/BotPasswordTest.php
b/tests/phpunit/includes/user/BotPasswordTest.php
index 27ce287..629c6e5 100644
--- a/tests/phpunit/includes/user/BotPasswordTest.php
+++ b/tests/phpunit/includes/user/BotPasswordTest.php
@@ -49,9 +49,7 @@
public function addDBData() {
$passwordFactory = new \PasswordFactory();
$passwordFactory->init( \RequestContext::getMain()->getConfig()
);
- // A is unsalted MD5 (thus fast) ... we don't care about
security here, this is test only
- $passwordFactory->setDefaultType( 'A' );
- $pwhash = $passwordFactory->newFromPlaintext( 'foobaz' );
+ $passwordHash = $passwordFactory->newFromPlaintext( 'foobaz' );
$dbw = wfGetDB( DB_MASTER );
$dbw->delete(
@@ -65,7 +63,7 @@
[
'bp_user' => 42,
'bp_app_id' => 'BotPassword',
- 'bp_password' => $pwhash->toString(),
+ 'bp_password' =>
$passwordHash->toString(),
'bp_token' => 'token!',
'bp_restrictions' =>
'{"IPAddresses":["127.0.0.0/8"]}',
'bp_grants' => '["test"]',
@@ -73,7 +71,7 @@
[
'bp_user' => 43,
'bp_app_id' => 'BotPassword',
- 'bp_password' => $pwhash->toString(),
+ 'bp_password' =>
$passwordHash->toString(),
'bp_token' => 'token!',
'bp_restrictions' =>
'{"IPAddresses":["127.0.0.0/8"]}',
'bp_grants' => '["test"]',
@@ -311,8 +309,6 @@
public function testSave( $password ) {
$passwordFactory = new \PasswordFactory();
$passwordFactory->init( \RequestContext::getMain()->getConfig()
);
- // A is unsalted MD5 (thus fast) ... we don't care about
security here, this is test only
- $passwordFactory->setDefaultType( 'A' );
$bp = BotPassword::newUnsaved( [
'centralId' => 42,
@@ -325,9 +321,9 @@
BotPassword::newFromCentralId( 42, 'TestSave',
BotPassword::READ_LATEST ), 'sanity check'
);
- $pwhash = $password ? $passwordFactory->newFromPlaintext(
$password ) : null;
- $this->assertFalse( $bp->save( 'update', $pwhash ) );
- $this->assertTrue( $bp->save( 'insert', $pwhash ) );
+ $passwordHash = $password ? $passwordFactory->newFromPlaintext(
$password ) : null;
+ $this->assertFalse( $bp->save( 'update', $passwordHash ) );
+ $this->assertTrue( $bp->save( 'insert', $passwordHash ) );
$bp2 = BotPassword::newFromCentralId( 42, 'TestSave',
BotPassword::READ_LATEST );
$this->assertInstanceOf( 'BotPassword', $bp2 );
$this->assertEquals( $bp->getUserCentralId(),
$bp2->getUserCentralId() );
@@ -356,9 +352,9 @@
$this->assertTrue( $pw->equals( $password ) );
}
- $pwhash = $passwordFactory->newFromPlaintext( 'XXX' );
+ $passwordHash = $passwordFactory->newFromPlaintext( 'XXX' );
$token = $bp->getToken();
- $this->assertTrue( $bp->save( 'update', $pwhash ) );
+ $this->assertTrue( $bp->save( 'update', $passwordHash ) );
$this->assertNotEquals( $token, $bp->getToken() );
$pw = TestingAccessWrapper::newFromObject( $bp )->getPassword();
$this->assertTrue( $pw->equals( 'XXX' ) );
--
To view, visit https://gerrit.wikimedia.org/r/289363
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I423f09ff7472b6cbde21cb709ea7c7ef9e298f18
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Ori.livneh <[email protected]>
Gerrit-Reviewer: Hashar <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
