From 4faece5db7f5bd57c4a0f010fa1c5bd41ee4bf3b Mon Sep 17 00:00:00 2001
From: Kateryna Degtyariova <katerynadegtyariova@catalyst-au.net>
Date: Thu, 10 Mar 2022 11:37:17 +1100
Subject: [PATCH] Fix unit tests to pass sanity checks. Add PHPDocs

The sanity checks require that assertEquals(), assertNull() are not called after expectException().
Also, there should be only one expectException() call per test.
---
 tests/cache_config_test.php           | 42 +++++++++++++++++++--------
 tests/fixtures/mode_mappings_data.php |  2 +-
 tests/fixtures/stores_data.php        | 17 ++++++++++-
 3 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/tests/cache_config_test.php b/tests/cache_config_test.php
index 17ac70d..3b42560 100644
--- a/tests/cache_config_test.php
+++ b/tests/cache_config_test.php
@@ -25,6 +25,13 @@
 
 namespace tool_forcedcache\tests;
 
+/**
+ * Tests for forced cache configuration.
+ *
+ * @package     tool_forcedcache
+ * @author      Peter Burnett <peterburnett@catalyst-au.net>
+ * @copyright   Catalyst IT
+ */
 class tool_forcedcache_cache_config_testcase extends \advanced_testcase {
 
     /**
@@ -41,6 +48,9 @@ public function copy_to_valid_config_location(string $source): string {
         return realpath($dest);
     }
 
+    /**
+     * Tests reading config file from invalid path.
+     */
     public function test_read_config_file_from_invalid_path() {
         global $CFG;
         $this->resetAfterTest(true);
@@ -66,6 +76,9 @@ public function test_read_config_file_from_invalid_path() {
         $method->invoke($config);
     }
 
+    /**
+     * Tests reading invalid config file.
+     */
     public function test_read_valid_config_file() {
         global $CFG;
         $this->resetAfterTest(true);
@@ -90,6 +103,9 @@ public function test_read_valid_config_file() {
         $this->assertArrayHasKey('definitionoverrides', $configarr1);
     }
 
+    /**
+     * Tests reading garbled config file.
+     */
     public function test_read_garbled_config_file() {
         global $CFG;
         $this->resetAfterTest(true);
@@ -112,6 +128,9 @@ public function test_read_garbled_config_file() {
         $method->invoke($config);
     }
 
+    /**
+     * Tests reading non-existing config file.
+     */
     public function test_read_non_existent_config_file() {
         global $CFG;
         $this->resetAfterTest(true);
@@ -134,7 +153,9 @@ public function test_read_non_existent_config_file() {
         $method->invoke($config);
     }
 
-
+    /**
+     * Tests bad type of a generated store instance.
+     */
     public function test_generate_store_instance_config() {
         // Directly create a config.
         $config = new \tool_forcedcache_cache_config();
@@ -155,20 +176,13 @@ public function test_generate_store_instance_config() {
         // Now test with 0 stores declared and confirm its just the defaults.
         $this->assertEquals($storezero['expected'], $method->invoke($config, $storezero['input']));
 
+        // Now test store with where store isn't ready, don't instantiate (APCu doesn't work from CLI).
+        $this->assertEquals($storereqsnotmet['expected'], $method->invoke($config, $storereqsnotmet['input']));
+
         // Now test a store with a bad type.
         $this->expectException(\cache_exception::class);
         $this->expectExceptionMessage(get_string('store_bad_type', 'tool_forcedcache', 'faketype'));
         $storearr1 = $method->invoke($config, $storebadtype['input']);
-        $this->assertNull($storearr1);
-
-        // Now test a store with a missing required field.
-        $this->expectException(\cache_exception::class);
-        $this->expectExceptionMessage(get_string('store_missing_fields', 'tool_forcedcache', 'apcu-test'));
-        $storearr1 = $method->invoke($config, $storemissingfields['input']);
-        $this->assertNull($storearr1);
-
-        // Now test store with where store isn't ready, don't instantiate (APCu doesn't work from CLI).
-        $this->assertEquals($storereqsnotmet['expected'], $method->invoke($config, $storereqsnotmet['input']));
     }
 
     /**
@@ -196,10 +210,11 @@ public function test_generated_mode_mappings_for_definitionmatchtopruleset() {
         include(__DIR__ . '/fixtures/mode_mappings_data.php');
         include(__DIR__ . '/fixtures/definition_mappings_data.php');
 
-        $expected = $generatedmodemappingagainstdefinitionmatchtoprulesetexpected;
+        $expected = $generatedmodemapping;
         $rules = $definitionmatchtopruleset['rules'];
         $this->assertEquals($expected, $method->invoke($config, $rules));
     }
+
     /**
      * Tests output of mode mpapings once rules included in the mix.
      */
@@ -217,6 +232,9 @@ public function test_generated_mode_mappings_for_definitionnoruleset() {
         $this->assertEquals($defaultsexpected, $method->invoke($config, $rules));
     }
 
+    /**
+     * Tests definition mappings generated from rules.
+     */
     public function test_generate_definition_mappings_from_rules() {
         $config = new \tool_forcedcache_cache_config();
 
diff --git a/tests/fixtures/mode_mappings_data.php b/tests/fixtures/mode_mappings_data.php
index 5164d9b..37f9cc1 100644
--- a/tests/fixtures/mode_mappings_data.php
+++ b/tests/fixtures/mode_mappings_data.php
@@ -34,7 +34,7 @@
     )
 );
 
-$generatedmodemappingagainstdefinitionmatchtoprulesetexpected = array(
+$generatedmodemapping = array(
     array(
         'mode' => cache_store::MODE_APPLICATION,
         'store' => 'file-test',
diff --git a/tests/fixtures/stores_data.php b/tests/fixtures/stores_data.php
index a7296be..73cb285 100644
--- a/tests/fixtures/stores_data.php
+++ b/tests/fixtures/stores_data.php
@@ -290,6 +290,21 @@
                 'default' => true,
                 'class' => 'cachestore_static',
                 'lock' => 'cachelock_file_default',
-            )
+            ),
+        'apcutest' =>
+            array (
+                'name' => 'apcutest',
+                'plugin' => 'apcu',
+                'configuration' =>
+                array (
+                    'prefix' => 'test_',
+                ),
+                'class' => 'cachestore_apcu',
+                'features' => 4,
+                'modes' => 3,
+                'default' => false,
+                'mappingsonly' => false,
+                'lock' => 'cachelock_file_default',
+            ),
     )
 );