MDL-41889 tool_generator: Fixing problem detecting the last used cour…

…se shortname

admin/tool/generator/classes/site_backend.php

@@ -104,7 +104,7 @@ public function make() {
         // Create courses.
         $prevchdir = getcwd();
         chdir($CFG->dirroot);
-        $ncourse = $this->get_last_testcourse_id();
+        $ncourse = self::get_last_testcourse_id();
         foreach (self::$sitecourses as $coursesize => $ncourses) {
             for ($i = 1; $i <= $ncourses[$this->size]; $i++) {
                 // Non language-dependant shortname.
@@ -177,26 +177,29 @@ protected function run_create_course($shortname, $coursesize) {
      *
      * @return int The last generated numeric value.
      */
-    protected function get_last_testcourse_id() {
+    protected static function get_last_testcourse_id() {
         global $DB;
 
         $params = array();
         $params['shortnameprefix'] = $DB->sql_like_escape(self::SHORTNAMEPREFIX) . '%';
         $like = $DB->sql_like('shortname', ':shortnameprefix');
 
-        if (!$testcourses = $DB->get_records_select('course', $like, $params, 'shortname DESC')) {
+        if (!$testcourses = $DB->get_records_select('course', $like, $params, '', 'shortname')) {
             return 0;
         }
+        // SQL order by is not appropiate here as is ordering strings.
+        $shortnames = array_keys($testcourses);
+        rsort($shortnames, SORT_NATURAL);
 
         // They come ordered by shortname DESC, so non-numeric values will be the first ones.
-        foreach ($testcourses as $testcourse) {
-            $sufix = substr($testcourse->shortname, strlen(self::SHORTNAMEPREFIX));
-            if (is_numeric($sufix)) {
+        $prefixnchars = strlen(self::SHORTNAMEPREFIX);
+        foreach ($shortnames as $shortname) {
+            $sufix = substr($shortname, $prefixnchars);
+            if (preg_match('/^[\d]+$/', $sufix)) {
                 return $sufix;
             }
         }
-
-        // If all sufixes are not numeric this is the fist make test site run.
+        // If all sufixes are not numeric this is the first make test site run.
         return 0;
     }
 

admin/tool/generator/tests/maketestsite_test.php

@@ -0,0 +1,106 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Unit test for the site generator
+ *
+ * @package tool_generator
+ * @copyright 2013 David Monllaó
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Silly class to access site_backend internal methods.
+ *
+ * @package tool_generator
+ * @copyright 2013 David Monllaó
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class testable_tool_generator_site_backend extends tool_generator_site_backend {
+
+    /**
+     * Public accessor.
+     *
+     * @return int
+     */
+    public static function get_last_testcourse_id() {
+        return parent::get_last_testcourse_id();
+    }
+}
+
+/**
+ * Unit test for the site generator
+ *
+ * @package tool_generator
+ * @copyright 2013 David Monllaó
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class tool_generator_maketestsite_testcase extends advanced_testcase {
+
+    /**
+     * Checks that site courses shortnames are properly generated.
+     */
+    public function test_shortnames_generation() {
+
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        $generator = $this->getDataGenerator();
+
+        // Shortname common prefix.
+        $prefix = tool_generator_site_backend::SHORTNAMEPREFIX;
+
+        $record = array();
+
+        // Without courses will be 0.
+        $lastshortname = testable_tool_generator_site_backend::get_last_testcourse_id();
+        $this->assertEquals(0, $lastshortname);
+
+        // Without {$prefix} + {no integer} courses will be 0.
+        $record['shortname'] = $prefix . 'AA';
+        $generator->create_course($record);
+        $record['shortname'] = $prefix . '__';
+        $generator->create_course($record);
+        $record['shortname'] = $prefix . '12.2';
+        $generator->create_course($record);
+
+        $lastshortname = testable_tool_generator_site_backend::get_last_testcourse_id();
+        $this->assertEquals(0, $lastshortname);
+
+        // With {$prefix} + {integer} courses will be the higher one.
+        $record['shortname'] = $prefix . '2';
+        $generator->create_course($record);
+        $record['shortname'] = $prefix . '20';
+        $generator->create_course($record);
+        $record['shortname'] = $prefix . '8';
+        $generator->create_course($record);
+
+        $lastshortname = testable_tool_generator_site_backend::get_last_testcourse_id();
+        $this->assertEquals(20, $lastshortname);
+
+        // Numeric order.
+        for ($i = 9; $i < 14; $i++) {
+            $record['shortname'] = $prefix . $i;
+            $generator->create_course($record);
+        }
+
+        $lastshortname = testable_tool_generator_site_backend::get_last_testcourse_id();
+        $this->assertEquals(20, $lastshortname);
+    }
+
+}