MDL-50063 unittests: Remove some unnecesary gc_collect_cycles()
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Fri, 13 May 2016 00:50:45 +0000 (02:50 +0200)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Fri, 13 May 2016 00:50:45 +0000 (02:50 +0200)
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.

13 files changed:
admin/tool/uploadcourse/classes/course.php
admin/tool/uploadcourse/tests/course_test.php
admin/tool/uploadcourse/tests/helper_test.php
admin/tool/uploadcourse/tests/processor_test.php
backup/moodle2/tests/moodle2_course_format_test.php
backup/moodle2/tests/moodle2_test.php
backup/util/checks/tests/checks_test.php
backup/util/plan/tests/plan_test.php
backup/util/plan/tests/step_test.php
backup/util/plan/tests/task_test.php
course/tests/courselib_test.php
course/tests/externallib_test.php
lib/tests/questionlib_test.php

index 4486cde..7b111cb 100644 (file)
@@ -740,7 +740,6 @@ class tool_uploadcourse_course {
                 $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.
index 20bbaa3..0b62e87 100644 (file)
@@ -35,13 +35,6 @@ global $CFG;
  */
 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;
index 41a838c..6768507 100644 (file)
@@ -129,7 +129,6 @@ class tool_uploadcourse_helper_testcase extends advanced_testcase {
         $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 @@ class tool_uploadcourse_helper_testcase extends advanced_testcase {
         $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;
index 2d2ec04..1dd20dc 100644 (file)
@@ -36,13 +36,6 @@ require_once($CFG->libdir . '/csvlib.class.php');
  */
 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);
index 47b9971..bac5859 100644 (file)
@@ -39,13 +39,6 @@ require_once($CFG->libdir . '/completionlib.php');
  */
 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 @@ class format_test_cs2_options extends format_test_cs_options {
             ),
         ) + parent::section_format_options($foreditform);
     }
-}
\ No newline at end of file
+}
index 4842d4a..5231082 100644 (file)
@@ -38,13 +38,6 @@ require_once($CFG->libdir . '/completionlib.php');
  */
 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 @@ class core_backup_moodle2_testcase extends advanced_testcase {
                     $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
index 6bcbda4..8e4ced3 100644 (file)
@@ -131,6 +131,7 @@ class backup_check_testcase extends advanced_testcase {
             backup::INTERACTIVE_NO, backup::MODE_GENERAL, $this->userid);
         $this->assertTrue(backup_check::check_security($bc, true));
         $this->assertTrue($bc instanceof backup_controller);
+        $bc->destroy();
 
     }
 }
index bfc98f3..3e9b2fe 100644 (file)
@@ -89,6 +89,8 @@ class backup_plan_testcase extends advanced_testcase {
         // Calculate checksum and check it
         $checksum = $bp->calculate_checksum();
         $this->assertTrue($bp->is_checksum_correct($checksum));
+
+        $bc->destroy();
     }
 
     /**
index 826b349..0b1ba6d 100644 (file)
@@ -88,6 +88,7 @@ class backup_step_testcase extends advanced_testcase {
         $this->assertTrue($bs instanceof backup_step);
         $this->assertEquals($bs->get_name(), 'stepname');
 
+        $bc->destroy();
     }
 
     /**
@@ -128,6 +129,8 @@ class backup_step_testcase extends advanced_testcase {
         $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
index efc1b60..40ee6ea 100644 (file)
@@ -93,6 +93,7 @@ class backup_task_testcase extends advanced_testcase {
         $checksum = $bt->calculate_checksum();
         $this->assertTrue($bt->is_checksum_correct($checksum));
 
+        $bc->destroy();
     }
 
     /**
index 26400e3..f0d4869 100644 (file)
@@ -32,13 +32,6 @@ require_once($CFG->dirroot . '/enrol/imsenterprise/tests/imsenterprise_test.php'
 
 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 @@ class core_course_courselib_testcase extends advanced_testcase {
         $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 @@ class core_course_courselib_testcase extends advanced_testcase {
 
         // Destroy the resource controller since we are done using it.
         $rc->destroy();
-        unset($rc);
     }
 
     /**
index 9ca911b..9ad5d73 100644 (file)
@@ -47,13 +47,6 @@ class core_course_externallib_testcase extends externallib_advanced_testcase {
         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
      */
index 1635010..969e487 100644 (file)
@@ -51,13 +51,6 @@ class core_questionlib_testcase extends advanced_testcase {
         $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 @@ class core_questionlib_testcase extends advanced_testcase {
         $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 @@ class core_questionlib_testcase extends advanced_testcase {
 
         // 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();
     }
 
     /**