Apply fixes found during coverage improvements

brianjmiller · brianjmiller · commit c845d8fc5d56 · 2016-08-25T17:47:29.000-05:00
* Significant unit test coverage improvement (credit @WillSkates) * Patches to issues found while applying the above (also credit @WillSkates) * Exception namespace issue in `Map` and `State` * Accept-Language header issue in `RemoteLRS.retrieveActivity` * Incorrect class in failure reason in `SignatureComparisonTrait` * Style fix for 'else if'
diff --git a/src/AsVersionTrait.php b/src/AsVersionTrait.php
@@ -46,7 +46,7 @@ public function asVersion($version) {
             if ($value instanceof VersionableInterface) {
                 $value = $value->asVersion($version);
             }
-            else if (is_array($value) && !empty($value)) {
+            elseif (is_array($value) && !empty($value)) {
                 $tmp_value = array();
                 foreach ($value as $element) {
                     if ($element instanceof VersionableInterface) {
diff --git a/src/Map.php b/src/Map.php
@@ -55,7 +55,7 @@ public function __call($func, $args) {
                 return $this->_unset($args[0]);
             break;
             default:
-                throw new BadMethodCallException(__CLASS__ . "::$func() does not exist");
+                throw new \BadMethodCallException(get_class($this) . "::$func() does not exist");
         }
     }
 }
diff --git a/src/RemoteLRS.php b/src/RemoteLRS.php
@@ -229,7 +229,7 @@ private function _parseMetadata($metadata) {
                 if ($pair[0] === 'charset') {
                     $result['headers']['contentTypeCharset'] = $pair[1];
                 }
-                else if ($pair[0] === 'boundary') {
+                elseif ($pair[0] === 'boundary') {
                     $result['headers']['contentTypeBoundary'] = $pair[1];
                 }
             }
@@ -246,7 +246,7 @@ private function _parseMultipart($boundary, $content) {
             if ($part === '') {
                 continue;
             }
-            else if ($part === '--') {
+            elseif ($part === '--') {
                 break;
             }
             list($header, $body) = explode("\r\n\r\n", $part, 2);
@@ -956,7 +956,7 @@ public function deleteActivityProfile($activity, $id) {
     public function retrieveActivity($activityid) {
         $headers = array('Accept-language: *');
         if (isset($_SERVER['HTTP_ACCEPT_LANGUAGE'])) {
-            $headers = 'Accept-language: ' . $_SERVER['HTTP_ACCEPT_LANGUAGE'] . ', *';
+            $headers = array('Accept-language: ' . $_SERVER['HTTP_ACCEPT_LANGUAGE'] . ', *');
         }
 
         $response = $this->sendRequest(
diff --git a/src/SignatureComparisonTrait.php b/src/SignatureComparisonTrait.php
@@ -68,7 +68,7 @@ private static function doMatch($a, $b, $description) {
         }
 
         if (is_object($a) && ! ($b instanceof $a)) {
-            $result['reason'] = "Comparison of $description failed: not a " . get_class($this) . " value";
+            $result['reason'] = "Comparison of $description failed: not a " . get_class($a) . " value";
             return $result;
         }
 
diff --git a/src/State.php b/src/State.php
@@ -52,7 +52,7 @@ public function getAgent() { return $this->agent; }
 
     public function setRegistration($value) {
         if (isset($value) && ! preg_match(Util::UUID_REGEX, $value)) {
-            throw new InvalidArgumentException('arg1 must be a UUID');
+            throw new \InvalidArgumentException('arg1 must be a UUID');
         }
 
         $this->registration = $value;
diff --git a/src/Statement.php b/src/Statement.php
@@ -249,7 +249,7 @@ public function verify($options = array()) {
         if (isset($options['publicKey'])) {
             $publicKeyFile = $options['publicKey'];
         }
-        else if (isset($header['x5c'])) {
+        elseif (isset($header['x5c'])) {
             $cert = "-----BEGIN CERTIFICATE-----\r\n" . chunk_split($header['x5c'][0], 64, "\r\n") . "-----END CERTIFICATE-----\r\n";
             $cert = openssl_x509_read($cert);
             if (! $cert) {
diff --git a/tests/ActivityProfileTest.php b/tests/ActivityProfileTest.php
@@ -0,0 +1,30 @@
+<?php
+/*
+    Copyright 2016 Rustici Software
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+*/
+
+namespace TinCanTest;
+
+use TinCan\ActivityProfile;
+use TinCan\Activity;
+
+class ActivityProfileTest extends \PHPUnit_Framework_TestCase {
+    public function testSetActivity() {
+        $profile = new ActivityProfile();
+        $profile->setActivity(['id' => COMMON_ACTIVITY_ID]);
+
+        $this->assertInstanceOf('TinCan\Activity', $profile->getActivity());
+    }
+}
diff --git a/tests/AgentProfileTest.php b/tests/AgentProfileTest.php
@@ -0,0 +1,35 @@
+<?php
+/*
+    Copyright 2016 Rustici Software
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+*/
+
+namespace TinCanTest;
+
+use TinCan\AgentProfile;
+use TinCan\Agent;
+use TinCan\Group;
+
+class AgentProfileTest extends \PHPUnit_Framework_TestCase {
+    public function testSetAgent() {
+        $profile = new AgentProfile();
+        $profile->setAgent(['mbox' => COMMON_MBOX]);
+
+        $this->assertInstanceOf('TinCan\Agent', $profile->getAgent());
+
+        $profile->setAgent(['objectType' => 'Group']);
+
+        $this->assertInstanceOf('TinCan\Group', $profile->getAgent());
+    }
+}
diff --git a/tests/ContextActivitiesTest.php b/tests/ContextActivitiesTest.php
@@ -276,4 +276,26 @@ public function testCompareWithSignature() {
         }
         $this->runSignatureCases("TinCan\ContextActivities", $cases);
     }
+
+    /**
+     * @dataProvider invalidListSetterDataProvider
+     */
+    public function testListSetterThrowsInvalidArgumentException($publicMethodName, $invalidValue) {
+        $this->setExpectedException(
+            'InvalidArgumentException',
+            'type of arg1 must be Activity, array of Activity properties, or array of Activity/array of Activity properties'
+        );
+        $obj = new ContextActivities();
+        $obj->$publicMethodName($invalidValue);
+    }
+
+    public function invalidListSetterDataProvider() {
+        $invalidValue = 1;
+        return [
+            ["setCategory", $invalidValue],
+            ["setParent", $invalidValue],
+            ["setGrouping", $invalidValue],
+            ["setOther", $invalidValue]
+        ];
+    }
 }
diff --git a/tests/ContextTest.php b/tests/ContextTest.php
@@ -359,4 +359,23 @@ public function testCompareWithSignature() {
         ];
         $this->runSignatureCases("TinCan\Context", $cases);
     }
+
+    public function testSetInstructorConvertToGroup() {
+        $obj = new Context();
+        $obj->setInstructor(
+            [
+                'objectType' => 'Group'
+            ]
+        );
+        $this->assertInstanceOf('TinCan\Group', $obj->getInstructor());
+    }
+
+    public function testSetRegistrationInvalidArgumentException() {
+        $this->setExpectedException(
+            'InvalidArgumentException',
+            'arg1 must be a UUID'
+        );
+        $obj = new Context();
+        $obj->setRegistration('232....3.3..3./2/2/1m3m3m3');
+    }
 }
diff --git a/tests/DocumentTest.php b/tests/DocumentTest.php
@@ -0,0 +1,34 @@
+<?php
+/*
+    Copyright 2016 Rustici Software
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+*/
+
+namespace TinCanTest;
+
+use TinCan\Document;
+
+class StubDocument extends Document {}
+
+class DocumentTest extends \PHPUnit_Framework_TestCase {
+    public function testExceptionOnInvalidDateTime() {
+        $this->setExpectedException(
+            "InvalidArgumentException",
+            'type of arg1 must be string or DateTime'
+        );
+
+        $obj = new StubDocument();
+        $obj->setTimestamp(1);
+    }
+}
diff --git a/tests/GroupTest.php b/tests/GroupTest.php
@@ -149,6 +149,9 @@ public function testAddMember() {
 
         $obj->addMember($common_agent);
         $this->assertEquals([$common_agent], $obj->getMember(), 'member list existing Agent');
+
+        $versioned = $obj->asVersion('1.0.0');
+        $this->assertSame($versioned['member'][0], $common_agent->asVersion('1.0.0'));
     }
 
     public function testCompareWithSignature() {
diff --git a/tests/LRSResponseTest.php b/tests/LRSResponseTest.php
@@ -0,0 +1,29 @@
+<?php
+/*
+    Copyright 2016 Rustici Software
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+*/
+
+namespace TinCanTest;
+
+use TinCan\LRSResponse;
+
+class LRSResponseTest extends \PHPUnit_Framework_TestCase {
+    public function testInstantiation() {
+        $obj = new LRSResponse(true, '', false);
+        $this->assertTrue($obj->success);
+        $this->assertEquals('', $obj->content);
+        $this->assertFalse($obj->httpResponse);
+    }
+}
diff --git a/tests/LanguageMapTest.php b/tests/LanguageMapTest.php
@@ -66,5 +66,23 @@ public function testGetNegotiatedLanguageString() {
 
         $this->assertEquals($usValue, $langs['en-US'], 'US name equal');
         $this->assertEquals($ukValue, $langs['en-GB'], 'UK name equal');
+
+        $nullValue = $obj->getNegotiatedLanguageString();
+        $this->assertEquals($nullValue, $langs['en-GB'], 'from null: UK name equal');
+
+        if (isset($_SERVER['HTTP_ACCEPT_LANGUAGE'])) {
+            $restore = $_SERVER['HTTP_ACCEPT_LANGUAGE'];
+        }
+        $_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'en-US';
+
+        $nullAcceptValue = $obj->getNegotiatedLanguageString();
+        $this->assertEquals($nullAcceptValue, $langs['en-US'], 'from server: US name equal');
+
+        if (isset($restore)) {
+            $_SERVER['HTTP_ACCEPT_LANGUAGE'] = $restore;
+        }
+        else {
+            unset($_SERVER['HTTP_ACCEPT_LANGUAGE']);
+        }
     }
 }
diff --git a/tests/MapTest.php b/tests/MapTest.php
@@ -0,0 +1,60 @@
+<?php
+/*
+    Copyright 2016 Rustici Software
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+*/
+
+namespace TinCanTest;
+
+use TinCan\Map;
+
+class StubMap extends Map {}
+
+class MapTest extends \PHPUnit_Framework_TestCase {
+    public function testInstantiation() {
+        $obj = new StubMap();
+    }
+
+    public function testInstantiationWithArg() {
+        $obj = new StubMap([]);
+        $this->assertTrue($obj->isEmpty());
+    }
+
+    public function testSetUnset() {
+        $obj = new StubMap();
+
+        $code = 'code';
+        $value = 'value';
+
+        $obj->set($code, $value);
+
+        $this->assertEquals($value, $obj->asVersion()[$code]);
+
+        $obj->unset($code);
+
+        $this->assertFalse(isset($obj->asVersion()[$code]));
+    }
+
+    public function testExceptionOnBadMethodCall() {
+        $badName ="dsadasdasdasdasdasdas";
+
+        $this->setExpectedException(
+            '\BadMethodCallException',
+            get_class(new StubMap) . "::$badName() does not exist"
+        );
+
+        $obj = new StubMap();
+        $obj->$badName();
+    }
+}
diff --git a/tests/RemoteLRSTest.php b/tests/RemoteLRSTest.php
@@ -527,6 +527,11 @@ public function testRetrieveActivity() {
         $this->assertEquals($testActivity, $response->content, 'retrieved activity');
     }
 
+    public function testRetrieveActivityWithHttpAcceptLanguageHeader() {
+        $_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'en-US';
+        $this->testRetrieveActivity();
+    }
+
     public function testRetrieveAgentProfileIds() {
         $lrs = new RemoteLRS(self::$endpoint, self::$version, self::$username, self::$password);
         $response = $lrs->retrieveAgentProfileIds(
diff --git a/tests/SignatureComparisonTraitTest.php b/tests/SignatureComparisonTraitTest.php
diff --git a/tests/StateTest.php b/tests/StateTest.php
diff --git a/tests/StatementBaseTest.php b/tests/StatementBaseTest.php
diff --git a/tests/StatementTest.php b/tests/StatementTest.php
diff --git a/tests/VersionTest.php b/tests/VersionTest.php

Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ public function asVersion($version) {`
`46`	`46`	`if ($value instanceof VersionableInterface) {`
`47`	`47`	`$value = $value->asVersion($version);`
`48`	`48`	`}`
`49`		`- else if (is_array($value) && !empty($value)) {`
	`49`	`+ elseif (is_array($value) && !empty($value)) {`
`50`	`50`	`$tmp_value = array();`
`51`	`51`	`foreach ($value as $element) {`
`52`	`52`	`if ($element instanceof VersionableInterface) {`
Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ public function __call($func, $args) {`
`55`	`55`	`return $this->_unset($args[0]);`
`56`	`56`	`break;`
`57`	`57`	`default:`
`58`		`- throw new BadMethodCallException(__CLASS__ . "::$func() does not exist");`
	`58`	`+ throw new \BadMethodCallException(get_class($this) . "::$func() does not exist");`
`59`	`59`	`}`
`60`	`60`	`}`
`61`	`61`	`}`
Original file line number	Diff line number	Diff line change
`@@ -229,7 +229,7 @@ private function _parseMetadata($metadata) {`
`229`	`229`	`if ($pair[0] === 'charset') {`
`230`	`230`	`$result['headers']['contentTypeCharset'] = $pair[1];`
`231`	`231`	`}`
`232`		`- else if ($pair[0] === 'boundary') {`
	`232`	`+ elseif ($pair[0] === 'boundary') {`
`233`	`233`	`$result['headers']['contentTypeBoundary'] = $pair[1];`
`234`	`234`	`}`
`235`	`235`	`}`
`@@ -246,7 +246,7 @@ private function _parseMultipart($boundary, $content) {`
`246`	`246`	`if ($part === '') {`
`247`	`247`	`continue;`
`248`	`248`	`}`
`249`		`- else if ($part === '--') {`
	`249`	`+ elseif ($part === '--') {`
`250`	`250`	`break;`
`251`	`251`	`}`
`252`	`252`	`list($header, $body) = explode("\r\n\r\n", $part, 2);`
`@@ -956,7 +956,7 @@ public function deleteActivityProfile($activity, $id) {`
`956`	`956`	`public function retrieveActivity($activityid) {`
`957`	`957`	`$headers = array('Accept-language: *');`
`958`	`958`	`if (isset($_SERVER['HTTP_ACCEPT_LANGUAGE'])) {`
`959`		`- $headers = 'Accept-language: ' . $_SERVER['HTTP_ACCEPT_LANGUAGE'] . ', *';`
	`959`	`+ $headers = array('Accept-language: ' . $_SERVER['HTTP_ACCEPT_LANGUAGE'] . ', *');`
`960`	`960`	`}`
`961`	`961`
`962`	`962`	`$response = $this->sendRequest(`
Original file line number	Diff line number	Diff line change
`@@ -68,7 +68,7 @@ private static function doMatch($a, $b, $description) {`
`68`	`68`	`}`
`69`	`69`
`70`	`70`	`if (is_object($a) && ! ($b instanceof $a)) {`
`71`		`- $result['reason'] = "Comparison of $description failed: not a " . get_class($this) . " value";`
	`71`	`+ $result['reason'] = "Comparison of $description failed: not a " . get_class($a) . " value";`
`72`	`72`	`return $result;`
`73`	`73`	`}`
`74`	`74`
Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ public function getAgent() { return $this->agent; }`
`52`	`52`
`53`	`53`	`public function setRegistration($value) {`
`54`	`54`	`if (isset($value) && ! preg_match(Util::UUID_REGEX, $value)) {`
`55`		`- throw new InvalidArgumentException('arg1 must be a UUID');`
	`55`	`+ throw new \InvalidArgumentException('arg1 must be a UUID');`
`56`	`56`	`}`
`57`	`57`
`58`	`58`	`$this->registration = $value;`
Original file line number	Diff line number	Diff line change
`@@ -249,7 +249,7 @@ public function verify($options = array()) {`
`249`	`249`	`if (isset($options['publicKey'])) {`
`250`	`250`	`$publicKeyFile = $options['publicKey'];`
`251`	`251`	`}`
`252`		`- else if (isset($header['x5c'])) {`
	`252`	`+ elseif (isset($header['x5c'])) {`
`253`	`253`	`$cert = "-----BEGIN CERTIFICATE-----\r\n" . chunk_split($header['x5c'][0], 64, "\r\n") . "-----END CERTIFICATE-----\r\n";`
`254`	`254`	`$cert = openssl_x509_read($cert);`
`255`	`255`	`if (! $cert) {`