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.
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:
- Moved to the level2 "privacy" namespace:
- \mod_assign\privacy\feedback_legacy_polyfill_test
- \mod_assign\privacy\submission_legacy_polyfill_test
- Moved to the level2 "task" namespace:
- \core_message\task\migrate_message_data_test
- \ltiservice_gradebookservices\task\cleanup_test
- \message_email\task\send_email_test
- \mod_lti\task\clean_access_tokens_test
- \mod_workshop\task\cron_task_test
- Moved to the level2 "event" namespace:
- \core_h5p\event\deleted_test
- \core_h5p\event\viewed_test
- Renamed to a better name:
- backup_forum_activity_task_test.php (missing "task")
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.
This applies the "whitelist" => "include" changes to all the core
phpunit_coverage_info occurrences, so core won't emit any deprecation
warning (see previous commit).
At the same time, modified a bunch of comments in coverage files
to be more readable/understandable.
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
In PHPUnit 9.1, the following regexp-related assertions
have been deprecated and there are new alternatives for
all them:
- assertRegExp() -> assertMatchesRegularExpression()
- assertNotRegExp() -> assertDoesNotMatchRegularExpression()
This is about to, simply, move all cases to the new alternatives.
Source: https://github.com/sebastianbergmann/phpunit/blob/9.1.0/ChangeLog-9.1.md
Regexp to find all them:
ag 'assertRegExp|assertNotRegExp' -li
While this is not strictly required, because removal will
happen in PHPUnit 9.0, we are already getting rid of all
uses in core.
From release notes:https://phpunit.de/announcements/phpunit-8.html
assertInternalType() is deprecated and will be removed in
PHPUnit 9. Refactor your test to use assertIsArray(), assertIsBool(),
assertIsFloat(), assertIsInt(), assertIsNumeric(), assertIsObject(),
assertIsResource(), assertIsString(), assertIsScalar(),
assertIsCallable(), or assertIsIterable() instead.
All the setup/teardown/pre/post/conditions template methods
now are required to return void. This was warned with phpunit 7
and now is enforced.
At the same time, fix a few wrong function names,
provider data and param types, return statements...
In MDL-62853 a new clean_param(PARAM_PATH) was added to the
moodle_content_writer->get_path() method. And this caused some
Windows tests to start failing.
The problem is that clean_param(PARAM_PATH) does normalise directory
separators to be always forward backslashes and that's normally ok
but the get_path() method has some DIRECTORY_SEPARATOR dependent code
that stopped working under windows.
After analysing various solutions, and trying to keep the behavior
EXACTLY like it was before MDL-62853, but with the cleaning included
we have applied 2 changes:
b) Move the clean_param() to later within the array_map() function,
that way the code there, that uses DIRECTORY_SEPARATOR will continue
working the same.
b) As far as there are more DIRECTORY_SEPARATOR dependent code later
in the function, also perform a str_replace() to convert back to the
OS directory separator.
Those 2 points together (a and b) make the behavior to be 100% the
original one, with separators being kept and the paths being cleaned.
This solution corresponds 100% with the proposed fixes named 3) and
4) in the issue.
Final note... all that DIRECTORY_SEPARATOR maybe needs a review because
it really shouldn't be used unless strictly needed. But that falls out
from this issue which goal was to keep things safely working like they
were before the regression (but with the cleaning applied).
