MDL-50063 unittests: Remove some unnecesary gc_collect_cycles()

Now backup and restore operations free logger resources calling
to the destroy() method. This commit ensures:

1) That gc_collect_cycles() is not used anymore in backup-related tests.
2) That all backup and restore controllers in test do always call
   to the detroy() method.
3) Some unset() calls, needed to make gc_collect_cycles() are not used
   anymore.

admin/tool/uploadcourse/classes/course.php

@@ -740,7 +740,6 @@ public function proceed() {
                 $this->error('errorwhilerestoringcourse', new lang_string('errorwhilerestoringthecourse', 'tool_uploadcourse'));
             }
             $rc->destroy();
-            unset($rc); // File logging is a mess, we can only try to rely on gc to close handles.
         }
 
         // Proceed with enrolment data.

admin/tool/uploadcourse/tests/course_test.php

@@ -35,13 +35,6 @@
  */
 class tool_uploadcourse_course_testcase extends advanced_testcase {
 
-    /**
-     * Tidy up open files that may be left open.
-     */
-    protected function tearDown() {
-        gc_collect_cycles();
-    }
-
     public function test_proceed_without_prepare() {
         $this->resetAfterTest(true);
         $mode = tool_uploadcourse_processor::MODE_CREATE_NEW;

admin/tool/uploadcourse/tests/helper_test.php

@@ -129,7 +129,6 @@ public function test_get_restore_content_dir() {
         $this->assertTrue(isset($result['backup_destination']));
         $c1backupfile = $result['backup_destination']->copy_content_to_temp();
         $bc->destroy();
-        unset($bc); // File logging is a mess, we can only try to rely on gc to close handles.
 
         // Creating backup file.
         $bc = new backup_controller(backup::TYPE_1COURSE, $c2->id, backup::FORMAT_MOODLE,
@@ -139,7 +138,6 @@ public function test_get_restore_content_dir() {
         $this->assertTrue(isset($result['backup_destination']));
         $c2backupfile = $result['backup_destination']->copy_content_to_temp();
         $bc->destroy();
-        unset($bc); // File logging is a mess, we can only try to rely on gc to close handles.
 
         $oldcfg = isset($CFG->keeptempdirectoriesonbackup) ? $CFG->keeptempdirectoriesonbackup : false;
         $CFG->keeptempdirectoriesonbackup = true;

admin/tool/uploadcourse/tests/processor_test.php

@@ -36,13 +36,6 @@
  */
 class tool_uploadcourse_processor_testcase extends advanced_testcase {
 
-    /**
-     * Tidy up open files that may be left open.
-     */
-    protected function tearDown() {
-        gc_collect_cycles();
-    }
-
     public function test_basic() {
         global $DB;
         $this->resetAfterTest(true);

backup/moodle2/tests/moodle2_course_format_test.php

@@ -39,13 +39,6 @@
  */
 class core_backup_moodle2_course_format_testcase extends advanced_testcase {
 
-    /**
-     * Tidy up open files that may be left open.
-     */
-    protected function tearDown() {
-        gc_collect_cycles();
-    }
-
     /**
      * Tests a backup and restore adds the required section option data
      * when the same course format is used.
@@ -269,4 +262,4 @@ public function section_format_options($foreditform = false) {
             ),
         ) + parent::section_format_options($foreditform);
     }
-}
+}

backup/moodle2/tests/moodle2_test.php

@@ -38,13 +38,6 @@
  */
 class core_backup_moodle2_testcase extends advanced_testcase {
 
-    /**
-     * Tidy up open files that may be left open.
-     */
-    protected function tearDown() {
-        gc_collect_cycles();
-    }
-
     /**
      * Tests the availability field on modules and sections is correctly
      * backed up and restored.
@@ -161,11 +154,6 @@ public function test_restore_legacy_availability() {
                     $thrown->getFile() . ':' . $thrown->getLine(). "]\n\n";
         }
 
-        // Must set restore_controller variable to null so that php
-        // garbage-collects it; otherwise the file will be left open and
-        // attempts to delete it will cause a permission error on Windows
-        // systems, breaking unit tests.
-        $rc = null;
         $this->assertNull($thrown);
 
         // Get information about the resulting course and check that it is set

backup/util/checks/tests/checks_test.php

@@ -131,6 +131,7 @@ public function test_backup_check() {
             backup::INTERACTIVE_NO, backup::MODE_GENERAL, $this->userid);
         $this->assertTrue(backup_check::check_security($bc, true));
         $this->assertTrue($bc instanceof backup_controller);
+        $bc->destroy();
 
     }
 }

backup/util/plan/tests/plan_test.php

@@ -89,6 +89,8 @@ function test_backup_plan() {
         // Calculate checksum and check it
         $checksum = $bp->calculate_checksum();
         $this->assertTrue($bp->is_checksum_correct($checksum));
+
+        $bc->destroy();
     }
 
     /**

backup/util/plan/tests/step_test.php

@@ -88,6 +88,7 @@ function test_backup_step() {
         $this->assertTrue($bs instanceof backup_step);
         $this->assertEquals($bs->get_name(), 'stepname');
 
+        $bc->destroy();
     }
 
     /**
@@ -128,6 +129,8 @@ function test_backup_structure_step() {
         $this->assertTrue(strpos($contents, '<field2>value2</field2>') !== false);
         $this->assertTrue(strpos($contents, '</test>') !== false);
 
+        $bc->destroy();
+
         unlink($file); // delete file
 
         // Remove the test dir and any content

backup/util/plan/tests/task_test.php

@@ -93,6 +93,7 @@ function test_backup_task() {
         $checksum = $bt->calculate_checksum();
         $this->assertTrue($bt->is_checksum_correct($checksum));
 
+        $bc->destroy();
     }
 
     /**

course/tests/courselib_test.php

@@ -32,13 +32,6 @@
 
 class core_course_courselib_testcase extends advanced_testcase {
 
-    /**
-     * Tidy up open files that may be left open.
-     */
-    protected function tearDown() {
-        gc_collect_cycles();
-    }
-
     /**
      * Set forum specific test values for calling create_module().
      *
@@ -1917,7 +1910,6 @@ public function test_course_restored_event() {
         $filepath = $CFG->dataroot . '/temp/backup/test-restore-course-event';
         $file->extract_to_pathname($fp, $filepath);
         $bc->destroy();
-        unset($bc);
 
         // Now we want to catch the restore course event.
         $sink = $this->redirectEvents();
@@ -1955,7 +1947,6 @@ public function test_course_restored_event() {
 
         // Destroy the resource controller since we are done using it.
         $rc->destroy();
-        unset($rc);
     }
 
     /**

course/tests/externallib_test.php

@@ -47,13 +47,6 @@ protected function setUp() {
         require_once($CFG->dirroot . '/course/externallib.php');
     }
 
-    /**
-     * Tidy up open files that may be left open.
-     */
-    protected function tearDown() {
-        gc_collect_cycles();
-    }
-
     /**
      * Test create_categories
      */

lib/tests/questionlib_test.php

@@ -51,13 +51,6 @@ public function setUp() {
         $this->resetAfterTest();
     }
 
-    /**
-     * Tidy up open files that may be left open.
-     */
-    protected function tearDown() {
-        gc_collect_cycles();
-    }
-
     /**
      * Return true and false to test functions with feedback on and off.
      *
@@ -248,7 +241,6 @@ public function test_altering_tag_instance_context() {
         $filepath = $CFG->dataroot . '/temp/backup/test-restore-course';
         $file->extract_to_pathname($fp, $filepath);
         $bc->destroy();
-        unset($bc);
 
         // Now restore the course.
         $rc = new restore_controller('test-restore-course', $course2->id, backup::INTERACTIVE_NO,
@@ -262,6 +254,8 @@ public function test_altering_tag_instance_context() {
 
         // Check that there are two questions in the restored to course's context.
         $this->assertEquals(2, $DB->count_records('question', array('category' => $restoredcategory->id)));
+
+        $rc->destroy();
     }
 
     /**