NEW: Refactor to provide unit tests for cache headers check.

_config/injector.yml

@@ -30,3 +30,6 @@ SilverStripe\Core\Injector\Injector:
 SilverStripe\EnvironmentCheck\Checks\SessionCheck:
     dependencies:
       client: '%$EnvCheckClient'
+SilverStripe\EnvironmentCheck\Checks\CacheHeadersCheck:
+    dependencies:
+      client: '%$EnvCheckClient'

src/Checks/CacheHeadersCheck.php

@@ -20,16 +20,8 @@ use SilverStripe\EnvironmentCheck\EnvironmentCheck;
  */
 class CacheHeadersCheck implements EnvironmentCheck
 {
-    use Configurable;
     use Fetcher;
 
-    /**
-     * URL to check
-     *
-     * @var string
-     */
-    protected $url;
-
     /**
      * Settings that must be included in the Cache-Control header
      *
@@ -61,17 +53,9 @@ class CacheHeadersCheck implements EnvironmentCheck
      */
     public function __construct($url = '', $mustInclude = [], $mustExclude = [])
     {
-        $this->url = $url;
+        $this->setURL($url);
         $this->mustInclude = $mustInclude;
         $this->mustExclude = $mustExclude;
-
-        $this->clientConfig = [
-            'base_uri' => Director::absoluteBaseURL(),
-            'timeout' => 10.0,
-        ];
-
-        // Using a validation result to capture messages
-        $this->result = new ValidationResult();
     }
 
     /**
@@ -81,8 +65,11 @@ class CacheHeadersCheck implements EnvironmentCheck
      */
     public function check()
     {
-        $response = $this->fetchResponse($this->url);
+        // Using a validation result to capture messages
-        $fullURL = Controller::join_links(Director::absoluteBaseURL(), $this->url);
+        $this->result = new ValidationResult();
+
+        $response = $this->client->get($this->getURL());
+        $fullURL = $this->getURL();
         if ($response === null) {
             return [
                 EnvironmentCheck::ERROR,

tests/Checks/CacheHeadersCheckTest.php

@@ -0,0 +1,101 @@
+<?php
+
+namespace SilverStripe\EnvironmentCheck\Tests\Checks;
+
+use GuzzleHttp\Client;
+use GuzzleHttp\HandlerStack;
+use GuzzleHttp\Psr7\Response;
+use SilverStripe\Dev\SapphireTest;
+use GuzzleHttp\Handler\MockHandler;
+use SilverStripe\EnvironmentCheck\EnvironmentCheck;
+use SilverStripe\EnvironmentCheck\Checks\CacheHeadersCheck;
+
+/**
+ * Test session checks.
+ */
+class CacheHeadersCheckTest extends SapphireTest
+{
+    /**
+     * Test that directives that must be included, are.
+     *
+     * @return void
+     */
+    public function testMustInclude()
+    {
+        // Create a mock and queue responses
+        $mock = new MockHandler([
+            new Response(200, ['Cache-Control' => 'must-revalidate', 'ETag' => '123']),
+            new Response(200, ['Cache-Control' =>'no-cache', 'ETag' => '123']),
+            new Response(200, ['ETag' => '123']),
+            new Response(200, ['Cache-Control' => 'must-revalidate, private', 'ETag' => '123']),
+        ]);
+
+        $handler = HandlerStack::create($mock);
+        $client = new Client(['handler' => $handler]);
+
+        $cacheHeadersCheck = new CacheHeadersCheck('/', ['must-revalidate']);
+        $cacheHeadersCheck->client = $client;
+
+        // Run checks for each response above
+        $this->assertContains(EnvironmentCheck::OK, $cacheHeadersCheck->check());
+        $this->assertContains(EnvironmentCheck::ERROR, $cacheHeadersCheck->check());
+        $this->assertContains(EnvironmentCheck::ERROR, $cacheHeadersCheck->check());
+        $this->assertContains(EnvironmentCheck::OK, $cacheHeadersCheck->check());
+    }
+
+    /**
+     * Test that directives that must be excluded, are.
+     *
+     * @return void
+     */
+    public function testMustExclude()
+    {
+        // Create a mock and queue responses
+        $mock = new MockHandler([
+            new Response(200, ['Cache-Control' => 'must-revalidate', 'ETag' => '123']),
+            new Response(200, ['Cache-Control' =>'no-cache', 'ETag' => '123']),
+            new Response(200, ['ETag' => '123']),
+            new Response(200, ['Cache-Control' =>'private, no-store', ' ETag' => '123']),
+        ]);
+
+        $handler = HandlerStack::create($mock);
+        $client = new Client(['handler' => $handler]);
+
+        $cacheHeadersCheck = new CacheHeadersCheck('/', [], ["no-store", "no-cache", "private"]);
+        $cacheHeadersCheck->client = $client;
+
+        // Run checks for each response above
+        $this->assertContains(EnvironmentCheck::OK, $cacheHeadersCheck->check());
+        $this->assertContains(EnvironmentCheck::ERROR, $cacheHeadersCheck->check());
+        $this->assertContains(EnvironmentCheck::OK, $cacheHeadersCheck->check());
+        $this->assertContains(EnvironmentCheck::ERROR, $cacheHeadersCheck->check());
+    }
+
+    /**
+     * Test that Etag header must exist in response.
+     *
+     * @return void
+     */
+    public function testEtag()
+    {
+        // Create a mock and queue responses
+        $mock = new MockHandler([
+            new Response(200, ['Cache-Control' => 'must-revalidate', 'ETag' => '123']),
+            new Response(200, ['Cache-Control' =>'no-cache']),
+            new Response(200, ['ETag' => '123']),
+            new Response(200, []),
+        ]);
+
+        $handler = HandlerStack::create($mock);
+        $client = new Client(['handler' => $handler]);
+
+        $cacheHeadersCheck = new CacheHeadersCheck('/');
+        $cacheHeadersCheck->client = $client;
+
+        // Run checks for each response above
+        $this->assertContains(EnvironmentCheck::OK, $cacheHeadersCheck->check());
+        $this->assertContains(EnvironmentCheck::ERROR, $cacheHeadersCheck->check());
+        $this->assertContains(EnvironmentCheck::OK, $cacheHeadersCheck->check());
+        $this->assertContains(EnvironmentCheck::ERROR, $cacheHeadersCheck->check());
+    }
+}