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

Reply via email to