Skip to content

Commit

Permalink
remove old workaround (trying to use debugging for logging) and fixed…
Browse files Browse the repository at this point in the history
… some code

- proper Mockery integration into PHPUnit
- fix upload_course.php parameters to align with moodle standards
- fix tests sometimes configuring completionlib on course level wrong
- in tests: dynamically declaring class level variables is deprecated in more recent php versions -> fixing
  • Loading branch information
Glutamat42 committed Apr 3, 2024
1 parent a4cacd5 commit 945e19a
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 17 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
/.idea
/old
/vendor
/composer.lock
/composer.lock
/.phpdoc
6 changes: 3 additions & 3 deletions classes/external/upload_course.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ class upload_course extends external_api {
public static function execute_parameters(): external_function_parameters {
return new external_function_parameters(
array(
'category_id' => new external_value(PARAM_INT, 'ID of the category in which the course should be created. If null, the course will be created in the category with the lowest available ID. Please note that even if a user has restore permissions for a category other than the one with the lowest ID, the course restoration process will not be successful.', VALUE_OPTIONAL),
'only_check_permissions' => new external_value(PARAM_BOOL, 'Check only if user has the permissions for restore. No mbz needed. Will return generic data for course name and id.', VALUE_OPTIONAL),
'mbz' => new external_value(PARAM_FILE, 'Required (moodle tag "optional" is due to moodle limitations), except if only_check_permissions is true. MBZ as file upload. Upload the file in this field. Moodle external_api wont recognize it / this field will be empty but it can be loaded from this field via plain PHP code.', VALUE_OPTIONAL),
'category_id' => new external_value(PARAM_INT, 'ID of the category in which the course should be created. If null, the course will be created in the category with the lowest available ID. Please note that even if a user has restore permissions for a category other than the one with the lowest ID, the course restoration process will not be successful.', VALUE_DEFAULT, null),
'only_check_permissions' => new external_value(PARAM_BOOL, 'Check only if user has the permissions for restore. No mbz needed. Will return generic data for course name and id.', VALUE_DEFAULT, false),
'mbz' => new external_value(PARAM_FILE, 'Required (moodle tag "optional" is due to moodle limitations), except if only_check_permissions is true. MBZ as file upload. Upload the file in this field. Moodle external_api wont recognize it / this field will be empty but it can be loaded from this field via plain PHP code.', VALUE_DEFAULT, null),
)
);
}
Expand Down
8 changes: 6 additions & 2 deletions tests/adler_score_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use mod_h5pactivity\local\grader;
use moodle_exception;
use ReflectionClass;
use stdClass;
use Throwable;

global $CFG;
Expand Down Expand Up @@ -59,15 +60,18 @@ public function test_get_score_item() {


class adler_score_test extends adler_testcase {
// TODO: Deprecated: Creation of dynamic property local_adler\adler_score_test::$course is deprecated in /home/runner/work/MoodlePluginLocal/MoodlePluginLocal/moodle/local/adler/tests/adler_score_test.php on line 69 (https://github.com/ProjektAdLer/MoodlePluginLocal/actions/runs/7917995524/job/21615389855)
private stdClass $course;
private stdClass $module;
private stdClass $user;

public function setUp(): void {
parent::setUp();

// create user
$this->user = $this->getDataGenerator()->create_user();

// create course
$this->course = $this->getDataGenerator()->create_course();
$this->course = $this->getDataGenerator()->create_course(['enablecompletion' => 1]);

// create module
$this->module = $this->getDataGenerator()->create_module('url', ['course' => $this->course->id, 'completion' => 1]);
Expand Down
2 changes: 1 addition & 1 deletion tests/external/score_primitive_learning_element_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function setUp(): void {
require_once(__DIR__ . '/deprecated_mocks.php');

// init test data
$this->course = $this->getDataGenerator()->create_course(array('enablecompletion' => 1));
$this->course = $this->getDataGenerator()->create_course(['enablecompletion' => 1]);

$this->course_module = $this->getDataGenerator()->create_module('url', array('course' => $this->course->id, 'completion' => 1));
$this->course_module = get_coursemodule_from_id(null, $this->course_module->cmid, 0, false, MUST_EXIST);
Expand Down
11 changes: 1 addition & 10 deletions tests/lib/adler_testcase.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,7 @@ public function setUp(): void {
public function tearDown(): void {
parent::tearDown();

// Moodle thinks debugging messages should be tested (check for debugging messages in unit tests).
// Imho this is very bad practice, because devs should be encouraged to provide additional Information
// for debugging. Checking for log messages in tests provides huge additional effort (e.g. tests will fail because
// a message was changed / an additional one was added / ...). Because logging should NEVER affect the
// functionality of the code, this is completely unnecessary. Where this leads can be perfectly seen in all
// moodle code: Things work or do not work and there is no feedback on that. Often things return null if successfully
// and if nothing happened (also categorized as successful), but no feedback is available which of both cases happened.
// Users and devs very often can't know why something does not work.
// If something went wrong either the code should handle the problem or it should throw an exception.
$this->resetDebugging();
Mockery::close();
}
}

Expand Down

0 comments on commit 945e19a

Please sign in to comment.