From 6a0cdf26eef5f17f0717b5b1e9960c7b165d083b Mon Sep 17 00:00:00 2001
From: Andrew Nicols <andrew@nicols.co.uk>
Date: Mon, 11 Nov 2019 07:08:28 +0800
Subject: [PATCH] MDL-67353 course: Fix more broken unit tests

---
 customfield/tests/api_test.php                |  10 +-
 .../tests/category_controller_test.php        |  32 ++--
 customfield/tests/data_controller_test.php    |  26 ++-
 customfield/tests/field_controller_test.php   |  32 ++--
 customfield/tests/generator_test.php          |  10 +-
 customfield/tests/privacy_test.php            | 169 +++++++++++-------
 6 files changed, 164 insertions(+), 115 deletions(-)

diff --git a/customfield/tests/api_test.php b/customfield/tests/api_test.php
index 4f8c3c26dbc..434eb355bae 100644
--- a/customfield/tests/api_test.php
+++ b/customfield/tests/api_test.php
@@ -38,17 +38,19 @@ use \core_customfield\category_controller;
 class core_customfield_api_testcase extends advanced_testcase {
 
     /**
-     * This method is called after the last test of this test class is run.
+     * Tear down to reset the singleton after each test.
      */
     public function tearDown() {
         core_course\customfield\course_handler::reset_after_test();
+        parent::tearDown();
     }
 
     /**
-     * Get generator
+     * Get generator.
+     *
      * @return core_customfield_generator
      */
-    protected function get_generator() : core_customfield_generator {
+    protected function get_generator(): core_customfield_generator {
         return $this->getDataGenerator()->get_plugin_generator('core_customfield');
     }
 
@@ -57,7 +59,7 @@ class core_customfield_api_testcase extends advanced_testcase {
      *
      * @param array $expected
      * @param array $array array of objects with "get($property)" method
-     * @param sring $propertyname
+     * @param string $propertyname
      */
     protected function assert_property_in_array($expected, $array, $propertyname) {
         $this->assertEquals($expected, array_values(array_map(function($a) use ($propertyname) {
diff --git a/customfield/tests/category_controller_test.php b/customfield/tests/category_controller_test.php
index 56aa2d0d5ce..210a9669ba3 100644
--- a/customfield/tests/category_controller_test.php
+++ b/customfield/tests/category_controller_test.php
@@ -37,29 +37,28 @@ use \core_customfield\field_controller;
 class core_customfield_category_controller_testcase extends advanced_testcase {
 
     /**
-     * This method is called after the last test of this test class is run.
+     * Tear down to reset the singleton after each test.
      */
-    public static function tearDownAfterClass() {
-        $handler = core_course\customfield\course_handler::create();
-        $handler->delete_all();
+    public function tearDown() {
+        core_course\customfield\course_handler::reset_after_test();
+        parent::tearDown();
     }
 
     /**
-     * Tests set up.
-     */
-    public function setUp() {
-        $this->resetAfterTest();
-    }
-
-    /**
-     * Get generator
+     * Get generator.
+     *
      * @return core_customfield_generator
      */
-    protected function get_generator() : core_customfield_generator {
+    protected function get_generator(): core_customfield_generator {
         return $this->getDataGenerator()->get_plugin_generator('core_customfield');
     }
 
+    /**
+     * Test for the field_controller::__construct function.
+     */
     public function test_constructor() {
+        $this->resetAfterTest();
+
         $c = category_controller::create(0, (object)['component' => 'core_course', 'area' => 'course', 'itemid' => 0]);
         $handler = $c->get_handler();
         $this->assertTrue($c instanceof category_controller);
@@ -83,6 +82,8 @@ class core_customfield_category_controller_testcase extends advanced_testcase {
      */
     public function test_constructor_errors() {
         global $DB;
+        $this->resetAfterTest();
+
         $cat = $this->get_generator()->create_category();
         $catrecord = $cat->to_record();
 
@@ -181,6 +182,7 @@ class core_customfield_category_controller_testcase extends advanced_testcase {
      * \core_customfield\category_controller::get()
      */
     public function test_create_category() {
+        $this->resetAfterTest();
 
         // Create the category.
         $lpg = $this->get_generator();
@@ -209,6 +211,8 @@ class core_customfield_category_controller_testcase extends advanced_testcase {
      * Tests for \core_customfield\category_controller::set() behaviour.
      */
     public function test_rename_category() {
+        $this->resetAfterTest();
+
         // Create the category.
         $params = ['component' => 'core_course', 'area' => 'course', 'itemid' => 0, 'name' => 'Cat1',
             'contextid' => context_system::instance()->id];
@@ -232,6 +236,8 @@ class core_customfield_category_controller_testcase extends advanced_testcase {
      * Tests for \core_customfield\category_controller::delete() behaviour.
      */
     public function test_delete_category() {
+        $this->resetAfterTest();
+
         // Create the category.
         $lpg = $this->get_generator();
         $category0 = $lpg->create_category();
diff --git a/customfield/tests/data_controller_test.php b/customfield/tests/data_controller_test.php
index 7cf0b7ebac0..ab5b9da3806 100644
--- a/customfield/tests/data_controller_test.php
+++ b/customfield/tests/data_controller_test.php
@@ -35,25 +35,19 @@ use core_customfield\data_controller;
 class core_customfield_data_controller_testcase extends advanced_testcase {
 
     /**
-     * This method is called after the last test of this test class is run.
+     * Tear down to reset the singleton after each test.
      */
-    public static function tearDownAfterClass() {
-        $handler = core_course\customfield\course_handler::create();
-        $handler->delete_all();
+    public function tearDown() {
+        core_course\customfield\course_handler::reset_after_test();
+        parent::tearDown();
     }
 
     /**
-     * Tests set up.
-     */
-    public function setUp() {
-        $this->resetAfterTest();
-    }
-
-    /**
-     * Get generator
+     * Get generator.
+     *
      * @return core_customfield_generator
      */
-    protected function get_generator() : core_customfield_generator {
+    protected function get_generator(): core_customfield_generator {
         return $this->getDataGenerator()->get_plugin_generator('core_customfield');
     }
 
@@ -62,6 +56,8 @@ class core_customfield_data_controller_testcase extends advanced_testcase {
      */
     public function test_constructor() {
         global $DB;
+        $this->resetAfterTest();
+
         // Create a course, fields category and fields.
         $course = $this->getDataGenerator()->create_course();
         $category0 = $this->get_generator()->create_category(['name' => 'aaaa']);
@@ -130,6 +126,8 @@ class core_customfield_data_controller_testcase extends advanced_testcase {
      */
     public function test_constructor_errors() {
         global $DB;
+        $this->resetAfterTest();
+
         // Create a category, field and data.
         $category = $this->get_generator()->create_category();
         $field = $this->get_generator()->create_field(['categoryid' => $category->get('id')]);
@@ -188,4 +186,4 @@ class core_customfield_data_controller_testcase extends advanced_testcase {
             $this->assertEquals(moodle_exception::class, get_class($e));
         }
     }
-}
\ No newline at end of file
+}
diff --git a/customfield/tests/field_controller_test.php b/customfield/tests/field_controller_test.php
index e490ef1b07a..ff26572fe05 100644
--- a/customfield/tests/field_controller_test.php
+++ b/customfield/tests/field_controller_test.php
@@ -39,25 +39,19 @@ use \core_customfield\field_controller;
 class core_customfield_field_controller_testcase extends advanced_testcase {
 
     /**
-     * This method is called after the last test of this test class is run.
+     * Tear down to reset the singleton after each test.
      */
-    public static function tearDownAfterClass() {
-        $handler = core_course\customfield\course_handler::create();
-        $handler->delete_all();
+    public function tearDown() {
+        core_course\customfield\course_handler::reset_after_test();
+        parent::tearDown();
     }
 
     /**
-     * Tests set up.
-     */
-    public function setUp() {
-        $this->resetAfterTest();
-    }
-
-    /**
-     * Get generator
+     * Get generator.
+     *
      * @return core_customfield_generator
      */
-    protected function get_generator() : core_customfield_generator {
+    protected function get_generator(): core_customfield_generator {
         return $this->getDataGenerator()->get_plugin_generator('core_customfield');
     }
 
@@ -66,6 +60,8 @@ class core_customfield_field_controller_testcase extends advanced_testcase {
      */
     public function test_constructor() {
         global $DB;
+        $this->resetAfterTest();
+
         // Create the category.
         $category0 = $this->get_generator()->create_category();
 
@@ -111,6 +107,8 @@ class core_customfield_field_controller_testcase extends advanced_testcase {
      */
     public function test_constructor_errors() {
         global $DB;
+        $this->resetAfterTest();
+
         // Create a category and a field.
         $category = $this->get_generator()->create_category();
         $field = $this->get_generator()->create_field(['categoryid' => $category->get('id')]);
@@ -183,6 +181,8 @@ class core_customfield_field_controller_testcase extends advanced_testcase {
      */
     public function test_create_field() {
         global $DB;
+        $this->resetAfterTest();
+
         $lpg = $this->get_generator();
         $category = $lpg->create_category();
         $fields = $DB->get_records(\core_customfield\field::TABLE, ['categoryid' => $category->get('id')]);
@@ -211,6 +211,8 @@ class core_customfield_field_controller_testcase extends advanced_testcase {
      */
     public function test_delete_field() {
         global $DB;
+        $this->resetAfterTest();
+
         $lpg = $this->get_generator();
         $category = $lpg->create_category();
         $fields = $DB->get_records(\core_customfield\field::TABLE, ['categoryid' => $category->get('id')]);
@@ -237,6 +239,8 @@ class core_customfield_field_controller_testcase extends advanced_testcase {
      * Tests for \core_customfield\field_controller::get_configdata_property() behaviour.
      */
     public function test_get_configdata_property() {
+        $this->resetAfterTest();
+
         $lpg = $this->get_generator();
         $category = $lpg->create_category();
         $configdata = ['a' => 'b', 'c' => ['d', 'e']];
@@ -251,4 +255,4 @@ class core_customfield_field_controller_testcase extends advanced_testcase {
         $this->assertEquals(['d', 'e'], $field->get_configdata_property('c'));
         $this->assertEquals(null, $field->get_configdata_property('x'));
     }
-}
\ No newline at end of file
+}
diff --git a/customfield/tests/generator_test.php b/customfield/tests/generator_test.php
index 059c868a8dd..38d8ebb9f84 100644
--- a/customfield/tests/generator_test.php
+++ b/customfield/tests/generator_test.php
@@ -36,18 +36,18 @@ defined('MOODLE_INTERNAL') || die();
 class core_customfield_generator_testcase extends advanced_testcase {
 
     /**
-     * This method is called after the last test of this test class is run.
+     * Tear down to reset the singleton after each test.
      */
-    public static function tearDownAfterClass() {
-        $handler = core_course\customfield\course_handler::create();
-        $handler->delete_all();
+    public function tearDown() {
+        core_course\customfield\course_handler::reset_after_test();
+        parent::tearDown();
     }
 
     /**
      * Get generator
      * @return core_customfield_generator
      */
-    protected function get_generator() : core_customfield_generator {
+    protected function get_generator(): core_customfield_generator {
         return $this->getDataGenerator()->get_plugin_generator('core_customfield');
     }
 
diff --git a/customfield/tests/privacy_test.php b/customfield/tests/privacy_test.php
index 69ea102f65d..bdb5d374b72 100644
--- a/customfield/tests/privacy_test.php
+++ b/customfield/tests/privacy_test.php
@@ -38,71 +38,67 @@ use core_customfield\privacy\provider;
  */
 class core_customfield_privacy_testcase extends provider_testcase {
 
-    /** @var stdClass[]  */
-    private $courses = [];
-    /** @var \core_customfield\category_controller[] */
-    private $cfcats = [];
-    /** @var \core_customfield\field_controller[] */
-    private $cffields = [];
-
     /**
-     * This method is called after the last test of this test class is run.
+     * Tear down to reset the singleton after each test.
      */
-    public static function tearDownAfterClass() {
-        $handler = core_course\customfield\course_handler::create();
-        $handler->delete_all();
+    public function tearDown() {
+        core_course\customfield\course_handler::reset_after_test();
+        parent::tearDown();
     }
 
     /**
-     * Set up
+     * Generate data.
+     *
+     * @return array
      */
-    public function setUp() {
+    protected function generate_test_data(): array {
         $this->resetAfterTest();
 
-        $this->cfcats[1] = $this->get_generator()->create_category();
-        $this->cfcats[2] = $this->get_generator()->create_category();
-        $this->cffields[11] = $this->get_generator()->create_field(
-            ['categoryid' => $this->cfcats[1]->get('id'), 'type' => 'checkbox']);
-        $this->cffields[12] = $this->get_generator()->create_field(
-            ['categoryid' => $this->cfcats[1]->get('id'), 'type' => 'date']);
-        $this->cffields[13] = $this->get_generator()->create_field(
-            ['categoryid' => $this->cfcats[1]->get('id'),
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_customfield');
+        $cfcats[1] = $generator->create_category();
+        $cfcats[2] = $generator->create_category();
+        $cffields[11] = $generator->create_field(
+            ['categoryid' => $cfcats[1]->get('id'), 'type' => 'checkbox']);
+        $cffields[12] = $generator->create_field(
+            ['categoryid' => $cfcats[1]->get('id'), 'type' => 'date']);
+        $cffields[13] = $generator->create_field(
+            ['categoryid' => $cfcats[1]->get('id'),
             'type' => 'select', 'configdata' => ['options' => "a\nb\nc"]]);
-        $this->cffields[14] = $this->get_generator()->create_field(
-            ['categoryid' => $this->cfcats[1]->get('id'), 'type' => 'text']);
-        $this->cffields[15] = $this->get_generator()->create_field(
-            ['categoryid' => $this->cfcats[1]->get('id'), 'type' => 'textarea']);
-        $this->cffields[21] = $this->get_generator()->create_field(
-            ['categoryid' => $this->cfcats[2]->get('id')]);
-        $this->cffields[22] = $this->get_generator()->create_field(
-            ['categoryid' => $this->cfcats[2]->get('id')]);
+        $cffields[14] = $generator->create_field(
+            ['categoryid' => $cfcats[1]->get('id'), 'type' => 'text']);
+        $cffields[15] = $generator->create_field(
+            ['categoryid' => $cfcats[1]->get('id'), 'type' => 'textarea']);
+        $cffields[21] = $generator->create_field(
+            ['categoryid' => $cfcats[2]->get('id')]);
+        $cffields[22] = $generator->create_field(
+            ['categoryid' => $cfcats[2]->get('id')]);
 
-        $this->courses[1] = $this->getDataGenerator()->create_course();
-        $this->courses[2] = $this->getDataGenerator()->create_course();
-        $this->courses[3] = $this->getDataGenerator()->create_course();
+        $courses[1] = $this->getDataGenerator()->create_course();
+        $courses[2] = $this->getDataGenerator()->create_course();
+        $courses[3] = $this->getDataGenerator()->create_course();
 
-        $this->get_generator()->add_instance_data($this->cffields[11], $this->courses[1]->id, 1);
-        $this->get_generator()->add_instance_data($this->cffields[12], $this->courses[1]->id, 1546300800);
-        $this->get_generator()->add_instance_data($this->cffields[13], $this->courses[1]->id, 2);
-        $this->get_generator()->add_instance_data($this->cffields[14], $this->courses[1]->id, 'Hello1');
-        $this->get_generator()->add_instance_data($this->cffields[15], $this->courses[1]->id,
+        $generator->add_instance_data($cffields[11], $courses[1]->id, 1);
+        $generator->add_instance_data($cffields[12], $courses[1]->id, 1546300800);
+        $generator->add_instance_data($cffields[13], $courses[1]->id, 2);
+        $generator->add_instance_data($cffields[14], $courses[1]->id, 'Hello1');
+        $generator->add_instance_data($cffields[15], $courses[1]->id,
             ['text' => '<p>Hi there</p>', 'format' => FORMAT_HTML]);
 
-        $this->get_generator()->add_instance_data($this->cffields[21], $this->courses[1]->id, 'hihi1');
+        $generator->add_instance_data($cffields[21], $courses[1]->id, 'hihi1');
 
-        $this->get_generator()->add_instance_data($this->cffields[14], $this->courses[2]->id, 'Hello2');
+        $generator->add_instance_data($cffields[14], $courses[2]->id, 'Hello2');
 
-        $this->get_generator()->add_instance_data($this->cffields[21], $this->courses[2]->id, 'hihi2');
+        $generator->add_instance_data($cffields[21], $courses[2]->id, 'hihi2');
 
-        $this->setUser($this->getDataGenerator()->create_user());
-    }
+        $user = $this->getDataGenerator()->create_user();
+        $this->setUser($user);
 
-    /**
-     * Get generator
-     * @return core_customfield_generator
-     */
-    protected function get_generator() : core_customfield_generator {
-        return $this->getDataGenerator()->get_plugin_generator('core_customfield');
+        return [
+            'user' => $user,
+            'cfcats' => $cfcats,
+            'cffields' => $cffields,
+            'courses' => $courses,
+        ];
     }
 
     /**
@@ -119,11 +115,17 @@ class core_customfield_privacy_testcase extends provider_testcase {
      */
     public function test_get_customfields_data_contexts() {
         global $DB;
-        list($sql, $params) = $DB->get_in_or_equal([$this->courses[1]->id, $this->courses[2]->id], SQL_PARAMS_NAMED);
+        [
+            'cffields' => $cffields,
+            'cfcats' => $cfcats,
+            'courses' => $courses,
+        ] = $this->generate_test_data();
+
+        list($sql, $params) = $DB->get_in_or_equal([$courses[1]->id, $courses[2]->id], SQL_PARAMS_NAMED);
         $r = provider::get_customfields_data_contexts('core_course', 'course', '=0',
             $sql, $params);
-        $this->assertEquals([context_course::instance($this->courses[1]->id)->id,
-            context_course::instance($this->courses[2]->id)->id],
+        $this->assertEquals([context_course::instance($courses[1]->id)->id,
+            context_course::instance($courses[2]->id)->id],
             $r->get_contextids(), '', 0, 10, true);
     }
 
@@ -131,6 +133,8 @@ class core_customfield_privacy_testcase extends provider_testcase {
      * Test for provider::get_customfields_configuration_contexts()
      */
     public function test_get_customfields_configuration_contexts() {
+        $this->generate_test_data();
+
         $r = provider::get_customfields_configuration_contexts('core_course', 'course');
         $this->assertEquals([context_system::instance()->id], $r->get_contextids());
     }
@@ -140,13 +144,20 @@ class core_customfield_privacy_testcase extends provider_testcase {
      */
     public function test_export_customfields_data() {
         global $USER, $DB;
+        $this->resetAfterTest();
+        [
+            'cffields' => $cffields,
+            'cfcats' => $cfcats,
+            'courses' => $courses,
+        ] = $this->generate_test_data();
+
         // Hack one of the fields so it has an invalid field type.
-        $invalidfieldid = $this->cffields[21]->get('id');
+        $invalidfieldid = $cffields[21]->get('id');
         $DB->update_record('customfield_field', ['id' => $invalidfieldid, 'type' => 'invalid']);
 
-        $context = context_course::instance($this->courses[1]->id);
+        $context = context_course::instance($courses[1]->id);
         $contextlist = new approved_contextlist($USER, 'core_customfield', [$context->id]);
-        provider::export_customfields_data($contextlist, 'core_course', 'course', '=0', '=:i', ['i' => $this->courses[1]->id]);
+        provider::export_customfields_data($contextlist, 'core_course', 'course', '=0', '=:i', ['i' => $courses[1]->id]);
         /** @var core_privacy\tests\request\content_writer $writer */
         $writer = writer::with_context($context);
 
@@ -155,7 +166,7 @@ class core_customfield_privacy_testcase extends provider_testcase {
         $invaldfieldischecked = false;
         foreach ($DB->get_records('customfield_data', []) as $dbrecord) {
             $data = $writer->get_data(['Custom fields data', $dbrecord->id]);
-            if ($dbrecord->instanceid == $this->courses[1]->id) {
+            if ($dbrecord->instanceid == $courses[1]->id) {
                 $this->assertEquals($dbrecord->fieldid, $data->fieldid);
                 $this->assertNotEmpty($data->fieldtype);
                 $this->assertNotEmpty($data->fieldshortname);
@@ -175,10 +186,17 @@ class core_customfield_privacy_testcase extends provider_testcase {
      */
     public function test_delete_customfields_data() {
         global $USER, $DB;
-        $approvedcontexts = new approved_contextlist($USER, 'core_course', [context_course::instance($this->courses[1]->id)->id]);
+        $this->resetAfterTest();
+        [
+            'cffields' => $cffields,
+            'cfcats' => $cfcats,
+            'courses' => $courses,
+        ] = $this->generate_test_data();
+
+        $approvedcontexts = new approved_contextlist($USER, 'core_course', [context_course::instance($courses[1]->id)->id]);
         provider::delete_customfields_data($approvedcontexts, 'core_course', 'course');
-        $this->assertEmpty($DB->get_records('customfield_data', ['instanceid' => $this->courses[1]->id]));
-        $this->assertNotEmpty($DB->get_records('customfield_data', ['instanceid' => $this->courses[2]->id]));
+        $this->assertEmpty($DB->get_records('customfield_data', ['instanceid' => $courses[1]->id]));
+        $this->assertNotEmpty($DB->get_records('customfield_data', ['instanceid' => $courses[2]->id]));
     }
 
     /**
@@ -186,9 +204,16 @@ class core_customfield_privacy_testcase extends provider_testcase {
      */
     public function test_delete_customfields_configuration() {
         global $USER, $DB;
+        $this->resetAfterTest();
+        [
+            'cffields' => $cffields,
+            'cfcats' => $cfcats,
+            'courses' => $courses,
+        ] = $this->generate_test_data();
+
         // Remember the list of fields in the category 2 before we delete it.
-        $catid1 = $this->cfcats[1]->get('id');
-        $catid2 = $this->cfcats[2]->get('id');
+        $catid1 = $cfcats[1]->get('id');
+        $catid2 = $cfcats[2]->get('id');
         $fids2 = $DB->get_fieldset_select('customfield_field', 'id', 'categoryid=?', [$catid2]);
         $this->assertNotEmpty($fids2);
         list($fsql, $fparams) = $DB->get_in_or_equal($fids2, SQL_PARAMS_NAMED);
@@ -216,9 +241,16 @@ class core_customfield_privacy_testcase extends provider_testcase {
      */
     public function test_delete_customfields_configuration_for_context() {
         global $USER, $DB;
+        $this->resetAfterTest();
+        [
+            'cffields' => $cffields,
+            'cfcats' => $cfcats,
+            'courses' => $courses,
+        ] = $this->generate_test_data();
+
         // Remember the list of fields in the category 2 before we delete it.
-        $catid1 = $this->cfcats[1]->get('id');
-        $catid2 = $this->cfcats[2]->get('id');
+        $catid1 = $cfcats[1]->get('id');
+        $catid2 = $cfcats[2]->get('id');
         $fids2 = $DB->get_fieldset_select('customfield_field', 'id', 'categoryid=?', [$catid2]);
         $this->assertNotEmpty($fids2);
         list($fsql, $fparams) = $DB->get_in_or_equal($fids2, SQL_PARAMS_NAMED);
@@ -246,12 +278,19 @@ class core_customfield_privacy_testcase extends provider_testcase {
      */
     public function test_delete_customfields_data_for_context() {
         global $DB;
+        $this->resetAfterTest();
+        [
+            'cffields' => $cffields,
+            'cfcats' => $cfcats,
+            'courses' => $courses,
+        ] = $this->generate_test_data();
+
         provider::delete_customfields_data_for_context('core_course', 'course',
-            context_course::instance($this->courses[1]->id));
+            context_course::instance($courses[1]->id));
         $fids2 = $DB->get_fieldset_select('customfield_field', 'id', '1=1', []);
         list($fsql, $fparams) = $DB->get_in_or_equal($fids2, SQL_PARAMS_NAMED);
-        $fparams['course1'] = $this->courses[1]->id;
-        $fparams['course2'] = $this->courses[2]->id;
+        $fparams['course1'] = $courses[1]->id;
+        $fparams['course2'] = $courses[2]->id;
         $this->assertEmpty($DB->get_records_select('customfield_data', 'instanceid = :course1 AND fieldid ' . $fsql, $fparams));
         $this->assertNotEmpty($DB->get_records_select('customfield_data', 'instanceid = :course2 AND fieldid ' . $fsql, $fparams));
     }