From 293e14a79a1b0959005a5b627053ca4e88617df6 Mon Sep 17 00:00:00 2001
From: addshore <addshorewiki@gmail.com>
Date: Wed, 25 Oct 2017 18:22:32 +0100
Subject: [PATCH] Refactor BlockTest

Something is holding state and interfering with
RevisionUnitTest from within BlockTest
as seen in I4f24e7fbb683cb51f3fd8b250732bae9c7541ba2
PS30

Change-Id: I5b779941385eb88e560dd2c63a8aea356804c806
---
 tests/phpunit/includes/BlockTest.php | 90 +++++++++++++++-------------
 1 file changed, 50 insertions(+), 40 deletions(-)

diff --git a/tests/phpunit/includes/BlockTest.php b/tests/phpunit/includes/BlockTest.php
index 63d05a0653..c422b515cc 100644
--- a/tests/phpunit/includes/BlockTest.php
+++ b/tests/phpunit/includes/BlockTest.php
@@ -6,69 +6,62 @@
  */
 class BlockTest extends MediaWikiLangTestCase {
 
-	/** @var Block */
-	private $block;
-	private $madeAt;
-
-	/* variable used to save up the blockID we insert in this test suite */
-	private $blockId;
-
-	function addDBData() {
-		$user = User::newFromName( 'UTBlockee' );
-		if ( $user->getId() == 0 ) {
-			$user->addToDatabase();
-			TestUser::setPasswordForUser( $user, 'UTBlockeePassword' );
-
-			$user->saveSettings();
-		}
+	/**
+	 * @return User
+	 */
+	private function getUserForBlocking() {
+		$testUser = $this->getMutableTestUser();
+		$user = $testUser->getUser();
+		$user->addToDatabase();
+		TestUser::setPasswordForUser( $user, 'UTBlockeePassword' );
+		$user->saveSettings();
+		return $user;
+	}
 
+	/**
+	 * @param User $user
+	 *
+	 * @return Block
+	 * @throws MWException
+	 */
+	private function addBlockForUser( User $user ) {
 		// Delete the last round's block if it's still there
-		$oldBlock = Block::newFromTarget( 'UTBlockee' );
+		$oldBlock = Block::newFromTarget( $user->getName() );
 		if ( $oldBlock ) {
 			// An old block will prevent our new one from saving.
 			$oldBlock->delete();
 		}
 
 		$blockOptions = [
-			'address' => 'UTBlockee',
+			'address' => $user->getName(),
 			'user' => $user->getId(),
 			'reason' => 'Parce que',
 			'expiry' => time() + 100500,
 		];
-		$this->block = new Block( $blockOptions );
-		$this->madeAt = wfTimestamp( TS_MW );
+		$block = new Block( $blockOptions );
 
-		$this->block->insert();
+		$block->insert();
 		// save up ID for use in assertion. Since ID is an autoincrement,
 		// its value might change depending on the order the tests are run.
 		// ApiBlockTest insert its own blocks!
-		$newBlockId = $this->block->getId();
-		if ( $newBlockId ) {
-			$this->blockId = $newBlockId;
-		} else {
+		if ( !$block->getId() ) {
 			throw new MWException( "Failed to insert block for BlockTest; old leftover block remaining?" );
 		}
 
 		$this->addXffBlocks();
-	}
 
-	/**
-	 * debug function : dump the ipblocks table
-	 */
-	function dumpBlocks() {
-		$v = $this->db->select( 'ipblocks', '*' );
-		print "Got " . $v->numRows() . " rows. Full dump follow:\n";
-		foreach ( $v as $row ) {
-			print_r( $row );
-		}
+		return $block;
 	}
 
 	/**
 	 * @covers Block::newFromTarget
 	 */
 	public function testINewFromTargetReturnsCorrectBlock() {
+		$user = $this->getUserForBlocking();
+		$block = $this->addBlockForUser( $user );
+
 		$this->assertTrue(
-			$this->block->equals( Block::newFromTarget( 'UTBlockee' ) ),
+			$block->equals( Block::newFromTarget( $user->getName() ) ),
 			"newFromTarget() returns the same block as the one that was made"
 		);
 	}
@@ -77,8 +70,11 @@ class BlockTest extends MediaWikiLangTestCase {
 	 * @covers Block::newFromID
 	 */
 	public function testINewFromIDReturnsCorrectBlock() {
+		$user = $this->getUserForBlocking();
+		$block = $this->addBlockForUser( $user );
+
 		$this->assertTrue(
-			$this->block->equals( Block::newFromID( $this->blockId ) ),
+			$block->equals( Block::newFromID( $block->getId() ) ),
 			"newFromID() returns the same block as the one that was made"
 		);
 	}
@@ -87,8 +83,12 @@ class BlockTest extends MediaWikiLangTestCase {
 	 * per T28425
 	 */
 	public function testBug26425BlockTimestampDefaultsToTime() {
+		$user = $this->getUserForBlocking();
+		$block = $this->addBlockForUser( $user );
+		$madeAt = wfTimestamp( TS_MW );
+
 		// delta to stop one-off errors when things happen to go over a second mark.
-		$delta = abs( $this->madeAt - $this->block->mTimestamp );
+		$delta = abs( $madeAt - $block->mTimestamp );
 		$this->assertLessThan(
 			2,
 			$delta,
@@ -105,9 +105,12 @@ class BlockTest extends MediaWikiLangTestCase {
 	 * @covers Block::newFromTarget
 	 */
 	public function testBug29116NewFromTargetWithEmptyIp( $vagueTarget ) {
-		$block = Block::newFromTarget( 'UTBlockee', $vagueTarget );
+		$user = $this->getUserForBlocking();
+		$initialBlock = $this->addBlockForUser( $user );
+		$block = Block::newFromTarget( $user->getName(), $vagueTarget );
+
 		$this->assertTrue(
-			$this->block->equals( $block ),
+			$initialBlock->equals( $block ),
 			"newFromTarget() returns the same block as the one that was made when "
 				. "given empty vagueTarget param " . var_export( $vagueTarget, true )
 		);
@@ -351,6 +354,9 @@ class BlockTest extends MediaWikiLangTestCase {
 	 * @covers Block::chooseBlock
 	 */
 	public function testBlocksOnXff( $xff, $exCount, $exResult ) {
+		$user = $this->getUserForBlocking();
+		$this->addBlockForUser( $user );
+
 		$list = array_map( 'trim', explode( ',', $xff ) );
 		$xffblocks = Block::getBlocksForIPList( $list, true );
 		$this->assertEquals( $exCount, count( $xffblocks ), 'Number of blocks for ' . $xff );
@@ -411,8 +417,11 @@ class BlockTest extends MediaWikiLangTestCase {
 	}
 
 	public function testSystemBlocks() {
+		$user = $this->getUserForBlocking();
+		$this->addBlockForUser( $user );
+
 		$blockOptions = [
-			'address' => 'UTBlockee',
+			'address' => $user->getName(),
 			'reason' => 'test system block',
 			'timestamp' => wfTimestampNow(),
 			'expiry' => $this->db->getInfinity(),
@@ -438,4 +447,5 @@ class BlockTest extends MediaWikiLangTestCase {
 			$this->assertSame( 'Cannot autoblock from a system block', $ex->getMessage() );
 		}
 	}
+
 }
-- 
2.20.1