Standardise the same test cases, e.g. make final, covers notation,
static data providers, namespaces, etc.
Once the tests run, a couple of them failed. They required changes
to assertions to make them pass.
While this change is not 100% required now, it's good habit
and we are checking for it since Moodle 4.4.
All the changes in this commit have been applied automatically
using the moodle.PHPUnit.TestReturnType sniff and are, exclusively
adding the ": void" return types when missing.
This has been generated running the following Sniff,
part of the Moodle's CodeSniffer standard:
- PSR2.Methods.MethodDeclaration
It just ensures all the function declarations have
the correct order for:
- abstract and final.
- visibility (public, protected, private).
- static.
So, all the lines modified by this commit are function declarations
and the only changes are in the positions of those keywords.
This has been generated running the following Sniffs, all
them part of the Moodle's CodeSniffer standard:
- PSR12.Functions.ReturnTypeDeclaration
- PSR12.Functions.NullableTypeDeclaration
- moodle.Methods.MethodDeclarationSpacing
- Squiz.Whitespace.ScopeKeywordSpacing
All them are, exclusively, about correct spacing, so the changes
are, all them, only white space changes.
Only exceptions to the above are 3 changes what were setting the
return type in a new line, and, when that happens, the closing
parenthesis (bracket) has to go to the same line than the colon.
This prevents automatic casting from int to double on 32 bit systems
(which have a maximum for integers of 2147483647)
Whilst Moodle will not support 32-bit systems for much longer, this is
still worth doing to prevent future issues.
This test apparently fails sporadically on Oracle, without obvious
causes. The failure however was hard to interpret, and the test
itself made a lot of assumptions about the system re. naming of
categories/courses. This could be prone to errors, so improve that.
In PHP 8.2 and later, setting a value to an undeclared class property is
deprecated and emits a deprecation notice.
So we need to add missing class properties that still need to be declared.
PHP before version 8.1 automatically converted stdClass or 'false' to arrays if
function parameter expects array (for example, "reset").
PHP 8.1 shows notices in these situations
Applied the following changes to various testcase classes:
- Namespaced with component[\level2-API]
- Moved to level2-API subdirectory when required.
- Fixed incorrect use statements with leading backslash.
- Remove file phpdoc block
- Remove MOODLE_INTERNAL if not needed.
- Changed code to point to global scope when needed.
- Fix some relative paths and comments here and there.
- All them passing individually.
- Complete runs passing too.
Special mention to:
- The following task tests have been moved within the level2 directory:
- \core\adhoc_task_test => \core\task\adhoc_task_test
- \core\scheduled_task_test => \core\task\scheduled_task_test
- \core\calendar_cron_task_test => \core\task\calendar_cron_task_test
- \core\h5p_get_content_types_task_test => \core\task\h5p_get_content_types_task_test
- \core\task_database_logger_test => \core\task\database_logger_test
- \core\task_logging_test => \core\task\logging_test
- The following event tests have been moved within level2 directory:
- \core\event_context_locked_test => \core\event\context_locked_test
- \core\event_deprecated_test => \core\event\deprecated_test
- \core\event_grade_deleted_test => \core\event\grade_deleted_test
- \core\event_profile_field_test => \core\event\profile_field_test
- \core\event_unknown_logged_test => \core\event\unknown_logged_test
- \core\event_user_graded_test => \core\event\user_graded_test
- \core\event_user_password_updated_test => \core\event\user_password_updated_test
- The following output tests have been moved within level2 directory:
- \core\mustache_template_finder_test => \core\output\mustache_template_finder_test
- \core\mustache_template_source_loader_test => \core\output\mustache_template_source_loader_test
- \core\output_mustache_helper_collection_test => \core\output\mustache_helper_collection_test
- The following tests have been moved to their correct tests directories:
- lib/tests/time_splittings_test.php => analytics/tests/time_splittings_test.php
- All the classes and tests under lib/filebrowser and lib/filestorage
belong to core, not to core_files. Some day we should move
them to their correct subsystem.
- All the classes and tests under lib/grade belong to core, not
to core_grades. Some day we should move them to their correct
subsystem.
- The core_grades_external class and its \core\grades_external_test
unit test should belong to the grades subsystem or, alternatively,
to \core\external, they both should be moved together.
- The core_grading_external class and its \core\grading_external_test
unit test should belong to the grading subsystem or, alternatively,
to \core\external, they both should be moved together.
- The \core\message\message and \core\message\inbound (may be others)
classes, and their associated tests should go to the core_message
subsystem.
- The core_user class, and its associated tests should go to the
core_user subsystem.
- The \core\update namespace is plain wrong (update is not valid API)
and needs action 1) create it or 2) move elsewhere.
Applied the following changes to various testcase classes:
- Namespaced with component[\level2-API]
- Moved to level2-API subdirectory when required.
- Fixed incorrect use statements with leading backslash.
- Remove file phpdoc block
- Remove MOODLE_INTERNAL if not needed.
- Changed code to point to global scope when needed.
- Fix some relative paths and comments here and there.
- All them passing individually.
- Complete runs passing too.
Special mention to:
- When belonging to other components and being valid api:
- analytics related tests have been moved to tests/analytics subdir.
- backup & restore related tests have been moved to tests/backup subdir.
- events related tests have been moved to tests/event subdir.
- privacy related tests have been moved to tests/privacy.
- task related tests have been moved to tests/task subdir.
- Some simple renames, not including the component part anymore (not
needed now that they are namespaced):
- some xxxlib_test.php have been renamed lib_test.php
(when they where testing the corresponding lib.php).
- cache stores tests have been all renamed store_test, originally
each one had its own name (file_test, apcu_test, redis_test...)
- assign feedback tests have been all renamed feedback_test, originally
each one had its own name (file_test, editpdf_test...)
All restore_date_test, api_test, rule_test, plugin_test,
manager_test, helper_test testcase classes:
- Namespaced with component[\level2-API]
- Moved to level2-API subdirectory when required.
- Fixed incorrect use statements with leading backslash.
- Remove file phpdoc block
- Remove MOODLE_INTERNAL if not needed.
- Changed code to point to global scope when needed.
- Fix some relative paths and comments here and there.
- All them passing individually.
- Complete runs passing too.
Special mention to:
- All restore_date_test cases have been put under xxx\backup
level 2 (valid API) namespace.
As far as now all them have correct privacy level2 namespace:
- Move them to "privacy" subdir.
- Rename the files to "provider_test.php", this includes old
privacy_test.php and privacy_provider_test.php files
- Rename the testcase to provider_test too (to match file name)
Also, change some relative paths and comments to point to new
locations.
All privacy_test and privacy_provider_test classes:
- Namespaced with component\privacy.
- Fixed incorrect use statements with leading backslash.
- Changed code to point to global scope when needed.
- Renamed a few files to make all be privacy_test or privacy_provider_test.php
- All them passing individually.
- Complete runs passing too.
Added parameter type hint to make_request_directory()
Removed invalid parameters from calls to make_request_directory()
Removing these parameters should have no effect on behaviour.
Even if a prediction is hidden from the report once is flagged,
it can be flagged several times if the user visits detailed view via URL.
We remove the checkbox to select a prediction and flag it
once it has already been flagged.
The current ->setMethods() has been silently (won't emit any
warning) in PHPUnit 9. And will stop working (current plans)
in PHPUnit 10.
Basically the now deprecated method has been split into:
- onlyMethods(): To point to existing methods in the mocked artifact.
- addMethods(): To point to non existing (yet) methods in the mocked
artifact.
In practice that means that all our current setMethods() calls can be
converted to onlyMethods() (existing) and done. The addMethods() is
mostly useful on development phases, not final testing.
Finally note that <null> isn't accepted anymore as parameter to
double all the methods. Instead empty array [] must be used.
Link: https://github.com/sebastianbergmann/phpunit/issues/3770
The methods assertContains() and assertNotContains() now perform
strict (type and value) comparison, pretty much like assertSame()
does.
A couple of new assertContainsEquals() and assertNotContainsEquals()
methods have been created to provide old (non-strict) behavior, pretty
much like assertEquals() do.
Apart from replacing the calls needing a relaxed comparison to those
new methods, there are also a couple of alternative, about how to
fix this, depending of every case:
- If the test is making any array_values() conversion, then it's better
to remove that conversion and use assertArrayHasKey(), that is not
strict.
- Sometimes if may be also possible to, simply, cast the expectation
to the exact type coming in the array. I've not applied this technique
to any of the cases in core.
Link: https://github.com/sebastianbergmann/phpunit/issues/3426
