Update unit tests to be rid of Phockito

This odd dependency is causing inclusion issues to do with namespacing and
the ability to work well with the framework (according to the dependency's
readme). The usage of the tool in this case adds no value, as it's
performing purely stub type work, rather than mock work. So instead we can
use an actual stub (simply return test values depending on input) and have
everything work well.

After removing the Phockito dependency it was found that the tests that
would skip when it isn't present were actually in a failing state. So they
had to be repaired too, mostly through the use of fixing the poor code
that was causing them to fail (as opposed to being bad tests). This is
both configuration and proceedural work.

_config/injector.yml

@@ -2,7 +2,7 @@
 Name: externallinksdependencies
 ---
 SilverStripe\Core\Injector\Injector:
-  LinkChecker: SilverStripe\ExternalLinks\Tasks\CurlLinkChecker
+  SilverStripe\ExternalLinks\Tasks\LinkChecker: SilverStripe\ExternalLinks\Tasks\CurlLinkChecker
   Psr\SimpleCache\CacheInterface.CurlLinkChecker:
     factory: SilverStripe\Core\Cache\CacheFactory
     constructor:

_config/routes.yml

@@ -1,7 +1,7 @@
 ---
-Name: externallink
+Name: externallinkroutes
-After: framework/routes
+Before: '#adminroutes'
 ---
-Director:
+SilverStripe\Control\Director:
   rules:
-    'admin/externallinks//$Action': 'CMSExternalLinksController'
+    'admin/externallinks//$Action': SilverStripe\ExternalLinks\Controllers\CMSExternalLinksController

src/Controllers/CMSExternalLinksController.php

@@ -12,13 +12,16 @@ use Symbiote\QueuedJobs\Services\QueuedJobService;
 class CMSExternalLinksController extends Controller
 {
 
-    private static $allowed_actions = array('getJobStatus', 'start');
+    private static $allowed_actions = [
+        'getJobStatus',
+        'start'
+    ];
 
     /**
-	 * Respond to Ajax requests for info on a running job
+     * Respond to Ajax requests for info on a running job
-	 *
+     *
-	 * @return string JSON string detailing status of the job
+     * @return string JSON string detailing status of the job
-	 */
+     */
     public function getJobStatus()
     {
         // Set headers
@@ -32,19 +35,19 @@ class CMSExternalLinksController extends Controller
         // Format status
         $track = BrokenExternalPageTrackStatus::get_latest();
         if ($track) {
-            return json_encode(array(
+            return json_encode([
-            'TrackID' => $track->ID,
+                'TrackID' => $track->ID,
-            'Status' => $track->Status,
+                'Status' => $track->Status,
-            'Completed' => $track->getCompletedPages(),
+                'Completed' => $track->getCompletedPages(),
-            'Total' => $track->getTotalPages()
+                'Total' => $track->getTotalPages()
-            ));
+            ]);
         }
     }
 
 
     /**
-	 * Starts a broken external link check
+     * Starts a broken external link check
-	 */
+     */
     public function start()
     {
         // return if the a job is already running

src/Model/BrokenExternalLink.php

@@ -69,16 +69,17 @@ class BrokenExternalLink extends DataObject
     public function getHTTPCodeDescription()
     {
         $code = $this->HTTPCode;
-        if (empty($code)) {
+
+        try {
+            $response = HTTPResponse::create('', $code);
             // Assume that $code = 0 means there was no response
-            $description = _t(__CLASS__ . '.NOTAVAILABLE', 'Server Not Available');
+            $description = $code ?
-        } elseif (($descriptions = Config::inst()->get(HTTPResponse::class, 'status_codes'))
+                $response->getStatusDescription() :
-            && isset($descriptions[$code])
+                _t(__CLASS__ . '.NOTAVAILABLE', 'Server Not Available');
-        ) {
+        } catch (InvalidArgumentException $e) {
-            $description = $descriptions[$code];
-        } else {
             $description = _t(__CLASS__ . '.UNKNOWNRESPONSE', 'Unknown Response Code');
         }
+
         return sprintf("%d (%s)", $code, $description);
     }
 }

src/Model/BrokenExternalPageTrackStatus.php

@@ -111,10 +111,10 @@ class BrokenExternalPageTrackStatus extends DataObject
     }
 
     /**
-	 * Create and prepare a new status
+     * Create and prepare a new status
-	 *
+     *
-	 * @return BrokenExternalPageTrackStatus
+     * @return BrokenExternalPageTrackStatus
-	 */
+     */
     public static function create_status()
     {
         // If the script is to be started create a new status

src/Reports/BrokenExternalLinksReport.php

@@ -2,12 +2,13 @@
 
 namespace SilverStripe\ExternalLinks\Reports;
 
-use SilverStripe\Core\Convert;
-use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus;
 use SilverStripe\ORM\ArrayList;
-use SilverStripe\View\Requirements;
+use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus;
+use SilverStripe\Core\Convert;
+use SilverStripe\View\HTML;
 use SilverStripe\Forms\LiteralField;
 use SilverStripe\Reports\Report;
+use SilverStripe\View\Requirements;
 
 /**
  * Content side-report listing pages with external broken links
@@ -72,7 +73,7 @@ class BrokenExternalLinksReport extends Report
         if ($track) {
             return $track->BrokenLinks();
         }
-        return new ArrayList();
+        return ArrayList::create();
     }
 
     public function getCMSFields()
@@ -81,17 +82,19 @@ class BrokenExternalLinksReport extends Report
         $fields = parent::getCMSFields();
 
         $reportResultSpan = '</ br></ br><h3 id="ReportHolder"></h3>';
-        $reportResult = new LiteralField('ResultTitle', $reportResultSpan);
+        $reportResult = LiteralField::create('ResultTitle', $reportResultSpan);
         $fields->push($reportResult);
 
-        $button = '<button id="externalLinksReport" type="button">%s</button>';
+        $button = HTML::createTag(
-        $runReportButton = new LiteralField(
+            'button',
-            'runReport',
+            [
-            sprintf(
+                'id' => 'externalLinksReport',
-                $button,
+                'type' => 'button',
-                _t(__CLASS__ . '.RUNREPORT', 'Create new report')
+                'class' => 'btn btn-primary'
-            )
+            ],
+            _t(__CLASS__ . '.RUNREPORT', 'Create new report')
         );
+        $runReportButton = LiteralField::create('runReport', $button);
         $fields->push($runReportButton);
 
         return $fields;

src/Tasks/CheckExternalLinksTask.php

@@ -2,27 +2,23 @@
 
 namespace SilverStripe\ExternalLinks\Tasks;
 
-use DOMNode;
-
-
-
-
-
-use SilverStripe\Dev\Debug;
-use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack;
 use SilverStripe\ExternalLinks\Model\BrokenExternalLink;
-use SilverStripe\Core\Config\Config;
+use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack;
 use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus;
-use SilverStripe\Core\Injector\Injector;
-use SilverStripe\ORM\DB;
 use SilverStripe\Dev\BuildTask;
+use SilverStripe\Core\Config\Config;
+use SilverStripe\ORM\DB;
+use SilverStripe\Dev\Debug;
+use DOMNode;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\ExternalLinks\Tasks\LinkChecker;
 
 class CheckExternalLinksTask extends BuildTask
 {
 
-    private static $dependencies = array(
+    private static $dependencies = [
-        'LinkChecker' => '%$LinkChecker'
+        'LinkChecker' => '%$' . LinkChecker::class
-    );
+    ];
 
     /**
      * @var bool

src/Tasks/CurlLinkChecker.php

@@ -3,6 +3,7 @@
 namespace SilverStripe\ExternalLinks\Tasks;
 
 use Psr\SimpleCache\CacheInterface;
+use SilverStripe\Core\Injector\Injector;
 
 /**
  * Check links using curl

tests/ExternalLinksTest.php

@@ -2,16 +2,16 @@
 
 namespace SilverStripe\ExternalLinks\Tests;
 
-use SilverStripe\ExternalLinks\Tasks\LinkChecker;
-use SilverStripe\Core\Injector\Injector;
-use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask;
-use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus;
-use SilverStripe\i18n\i18n;
-use SilverStripe\Reports\Report;
 use SilverStripe\ExternalLinks\Reports\BrokenExternalLinksReport;
-use SilverStripe\Dev\SapphireTest;
+use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus;
+use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask;
 use SilverStripe\ExternalLinks\Tests\ExternalLinksTestPage;
-use Phockito;
+use SilverStripe\i18n\i18n;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\ExternalLinks\Tasks\LinkChecker;
+use SilverStripe\ExternalLinks\Tests\Stubs\PretendLinkChecker;
+use SilverStripe\Reports\Report;
+use SilverStripe\Dev\SapphireTest;
 
 class ExternalLinksTest extends SapphireTest
 {
@@ -22,67 +22,12 @@ class ExternalLinksTest extends SapphireTest
         ExternalLinksTestPage::class
     );
 
-    public function setUpOnce()
-    {
-        if (class_exists(Phockito::class)) {
-            Phockito::include_hamcrest(false);
-        }
-
-        parent::setUpOnce();
-    }
-
     protected function setUp()
     {
         parent::setUp();
 
-        // Check dependencies
+        // Stub link checker
-        if (!class_exists(Phockito::class)) {
+        $checker = new PretendLinkChecker;
-            $this->skipTest = true;
-            return $this->markTestSkipped("These tests need the Phockito module installed to run");
-        }
-
-        // Mock link checker
-        $checker = Phockito::mock(LinkChecker::class);
-        Phockito::when($checker)
-            ->checkLink('http://www.working.com')
-            ->return(200);
-
-        Phockito::when($checker)
-            ->checkLink('http://www.broken.com/url/thing') // 404 on working site
-            ->return(404);
-
-        Phockito::when($checker)
-            ->checkLink('http://www.broken.com') // 403 on working site
-            ->return(403);
-
-        Phockito::when($checker)
-            ->checkLink('http://www.nodomain.com') // no ping
-            ->return(0);
-
-        Phockito::when($checker)
-            ->checkLink('/internal/link')
-            ->return(null);
-
-        Phockito::when($checker)
-            ->checkLink('[sitetree_link,id=9999]')
-            ->return(null);
-
-        Phockito::when($checker)
-            ->checkLink('home')
-            ->return(null);
-
-        Phockito::when($checker)
-            ->checkLink('broken-internal')
-            ->return(null);
-
-        Phockito::when($checker)
-            ->checkLink('[sitetree_link,id=1]')
-            ->return(null);
-
-        Phockito::when($checker)
-            ->checkLink(Hamcrest_Matchers::anything()) // anything else is 404
-            ->return(404);
-
         Injector::inst()->registerService($checker, LinkChecker::class);
     }
 

tests/Stubs/PretendLinkChecker.php

@@ -0,0 +1,28 @@
+<?php
+namespace SilverStripe\ExternalLinks\Tests\Stubs;
+
+use SilverStripe\ExternalLinks\Tasks\LinkChecker;
+
+class PretendLinkChecker implements LinkChecker
+{
+    public function checkLink($href)
+    {
+        switch ($href) {
+            case 'http://www.working.com':
+                return 200;
+            case 'http://www.broken.com':
+                return 403;
+            case 'http://www.nodomain.com':
+                return 0;
+            case '/internal/link':
+            case '[sitetree_link,id=9999]':
+            case 'home':
+            case 'broken-internal':
+            case '[sitetree_link,id=1]':
+                return null;
+            case 'http://www.broken.com/url/thing':
+            default:
+                return 404;
+        }
+    }
+}