jenkins-bot has submitted this change and it was merged.
Change subject: Clean-up and code coverage improvements
......................................................................
Clean-up and code coverage improvements
Change-Id: I7aec8807e1f01a4f27525817ae0caf1bdd0d9663
---
M includes/jobs/JobBase.php
M includes/utilities/ObserverInterfaceProvider.php
M tests/phpunit/includes/jobs/JobBaseTest.php
A tests/phpunit/includes/utilities/ObserverInterfaceTest.php
4 files changed, 202 insertions(+), 14 deletions(-)
Approvals:
Mwjames: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/jobs/JobBase.php b/includes/jobs/JobBase.php
index 2d22b10..af4d5d4 100644
--- a/includes/jobs/JobBase.php
+++ b/includes/jobs/JobBase.php
@@ -49,7 +49,7 @@
/**
* Returns invoked Title object
*
- * Apparently Jobs::getTitle() in MW 1.19 does not exists
+ * Apparently Job::getTitle() in MW 1.19 does not exists
*
* @since 1.9
*
@@ -68,6 +68,7 @@
*/
public function setStore( Store $store ) {
$this->store = $store;
+ return $this;
}
/**
@@ -95,6 +96,7 @@
*/
public function setSettings( Settings $settings ) {
$this->settings = $settings;
+ return $this;
}
/**
diff --git a/includes/utilities/ObserverInterfaceProvider.php
b/includes/utilities/ObserverInterfaceProvider.php
index 58cdb81..d3cb91b 100644
--- a/includes/utilities/ObserverInterfaceProvider.php
+++ b/includes/utilities/ObserverInterfaceProvider.php
@@ -139,7 +139,7 @@
* @param Subscriber $observer
*/
public function attach( Subscriber $observer ) {
- if ( $this->observers === array() || !isset(
$this->observers[$observer] ) ) {
+ if ( $this->contains( $observer ) === null ) {
$this->observers[] = $observer;
}
}
@@ -150,8 +150,9 @@
* @param Subscriber $observer
*/
public function detach( Subscriber $observer ) {
- if ( $this->observers !== array() && isset(
$this->observers[$observer] ) ) {
- unset( $this->observers[$observer] );
+ $index = $this->contains( $observer );
+ if ( $index !== null ) {
+ unset( $this->observers[$index] );
}
}
@@ -186,11 +187,8 @@
* @since 1.9
*/
public function notify() {
-
- if ( $this->observers !== array() ) {
- foreach ( $this->observers as $observer ) {
- $observer->update( $this );
- }
+ foreach ( $this->observers as $observer ) {
+ $observer->update( $this );
}
}
@@ -199,10 +197,28 @@
*
* @since 1.9
*
- * @return arrau
+ * @return array
*/
public function getObservers() {
return $this->observers;
}
-}
\ No newline at end of file
+ /**
+ * Returns an index (or null) of a registered Observer
+ *
+ * @since 1.9
+ *
+ * @param Subscriber $observer
+ *
+ * @return integer|null
+ */
+ protected function contains( Subscriber $observer ) {
+ foreach ( $this->observers as $key => $obs ) {
+ if ( $obs === $observer ) {
+ return $key;
+ }
+ }
+ return null;
+ }
+
+}
diff --git a/tests/phpunit/includes/jobs/JobBaseTest.php
b/tests/phpunit/includes/jobs/JobBaseTest.php
index 1b90079..bba7a2f 100644
--- a/tests/phpunit/includes/jobs/JobBaseTest.php
+++ b/tests/phpunit/includes/jobs/JobBaseTest.php
@@ -79,11 +79,13 @@
*/
public function testGetSetStore() {
- $instance = $this->getInstance();
+ $instance = $this->getInstance();
+ $mockStore = $this->newMockObject()->getMockStore();
$this->assertInstanceOf( '\SMW\Store', $instance->getStore() );
- $instance->setStore( $this->newMockObject()->getMockStore() );
+ $this->assertInstanceOf( $this->getClass(),
$instance->setStore( $mockStore ) );
$this->assertInstanceOf( '\SMW\Store', $instance->getStore() );
+ $this->assertEquals( $mockStore, $instance->getStore() );
}
/**
@@ -95,10 +97,13 @@
public function testGetSetSettings() {
$instance = $this->getInstance();
+ $settings = $this->getSettings( array( 'test' => 'lula' ) );
$this->assertInstanceOf( '\SMW\Settings',
$instance->getSettings() );
- $instance->setSettings( $this->getSettings() );
+ $this->assertInstanceOf( $this->getClass(),
$instance->setSettings( $settings ) );
$this->assertInstanceOf( '\SMW\Settings',
$instance->getSettings() );
+ $this->assertEquals( $settings, $instance->getSettings() );
+
}
/**
diff --git a/tests/phpunit/includes/utilities/ObserverInterfaceTest.php
b/tests/phpunit/includes/utilities/ObserverInterfaceTest.php
new file mode 100644
index 0000000..b2fb8f4
--- /dev/null
+++ b/tests/phpunit/includes/utilities/ObserverInterfaceTest.php
@@ -0,0 +1,165 @@
+<?php
+
+namespace SMW\Test;
+
+/**
+ * Tests for the Observer/Subject pattern
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ *
+ * @license GNU GPL v2+
+ * @since 1.9
+ *
+ * @author mwjames
+ */
+
+/**
+ * @covers \SMW\Observer
+ * @covers \SMW\Subject
+ *
+ * @ingroup Test
+ *
+ * @group SMW
+ * @group SMWExtension
+ */
+class ObserverInterfaceTest extends SemanticMediaWikiTestCase {
+
+ /**
+ * Returns the name of the class to be tested
+ *
+ * @return string|false
+ */
+ public function getClass() {
+ return false;
+ }
+
+ /**
+ * Helper method that returns a Observer object
+ *
+ * @since 1.9
+ *
+ * @param $data
+ *
+ * @return Observer
+ */
+ private function newObserver() {
+
+ $observer = $this->getMockBuilder( '\SMW\Observer' )
+ ->setMethods( array( 'lila' ) )
+ ->getMock();
+
+ $observer->expects( $this->any() )
+ ->method( 'lila' )
+ ->will( $this->returnCallback( array( $this,
'lilaObserverCallback' ) ) );
+
+ return $observer;
+
+ }
+
+ /**
+ * Helper method that returns a Subject object
+ *
+ * @since 1.9
+ *
+ * @param $data
+ *
+ * @return Subject
+ */
+ private function newObserverSubject() {
+
+ $subject = $this->getMockBuilder( '\SMW\Subject' )
+ ->setMethods( array( 'lulu' ) )
+ ->getMock();
+
+ $subject->expects( $this->any() )
+ ->method( 'lulu' )
+ ->will( $this->returnCallback( array( $this,
'luluSubjectCallback' ) ) );
+
+ return $subject;
+
+ }
+
+ /**
+ * @since 1.9
+ */
+ public function testConstructor() {
+ $this->assertInstanceOf( '\SMW\Observer', $this->newObserver()
);
+ $this->assertInstanceOf( '\SMW\Subject',
$this->newObserverSubject() );
+ }
+
+ /**
+ * @since 1.9
+ */
+ public function testInvokeAndDetach() {
+
+ $subject = $this->getMockForAbstractClass( '\SMW\Subject' );
+
+ // Same Observer instance attached twice results in only one
registered object
+ $observer = $this->getMockForAbstractClass( '\SMW\Observer',
array( $subject ) );
+ $subject->attach( $observer );
+
+ $this->assertCount( 1, $subject->getObservers() );
+ $subject->detach( $observer );
+ $this->assertCount( 0, $subject->getObservers() );
+
+ // Two different instances of an Observer
+ $this->getMockForAbstractClass( '\SMW\Observer', array(
$subject ) );
+ $observer = $this->getMockForAbstractClass( '\SMW\Observer',
array( $subject ) );
+
+ $this->assertCount( 2, $subject->getObservers() );
+ $subject->detach( $observer );
+ $this->assertCount( 1, $subject->getObservers() );
+
+ }
+
+ /**
+ * @since 1.9
+ */
+ public function testNotifyAndUpdate() {
+
+ $subject = $this->newObserverSubject();
+ $subject->attach( $this->newObserver() );
+
+ $this->assertNull( $subject->getState() );
+ $subject->lulu( $subject );
+
+ $this->assertCount( 1, $subject->getObservers() );
+ $this->assertEquals( 'lila', $subject->getState() );
+ $this->assertEquals( 'lila was informed by lulu',
$subject->info );
+
+ }
+
+ /**
+ * Notify the observer to execute "lila" (which is part of the Observer)
+ *
+ * @since 1.9
+ */
+ public function luluSubjectCallback( $subject ) {
+ $subject->setState( 'lila' );
+ }
+
+ /**
+ * Verify that the Observer was acting on the invoked Subject
+ *
+ * @since 1.9
+ */
+ public function lilaObserverCallback( $subject ) {
+ return $subject->info = 'lila was informed by lulu';
+ }
+
+}
--
To view, visit https://gerrit.wikimedia.org/r/76278
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7aec8807e1f01a4f27525817ae0caf1bdd0d9663
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/SemanticMediaWiki
Gerrit-Branch: master
Gerrit-Owner: Mwjames <[email protected]>
Gerrit-Reviewer: Mwjames <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits