This commit:
- introduces a \tests\ sub-namespace for use in unit tests only
- the path to this the tests/classes directory of the owning parent
- files here are excluded from unit test runs
This is agreed per policy in MDL-80855.
This change moves the reset of global test state to the finally section
rather than doing it only if the test passes.
Previously if a test which modifies the `core_component` internals
failed, it would not reset the internal state and impact subsequent
tests.
This is the final (3rd) commit, where we are adding missing
return types to a few tests (using @depends) which return
types are non-void, but something really returned (for consume
by the dependent test).
This commit includes more changes, all them also adding the :void
return type to unit tests missing them.
The difference is that all these changes, while also detected
perfectly by the moodle.PHPUnit.TestReturnType sniff, were not
auto-fixed (like the previous commit ones), because all them
do include some "return" statement and, for safety, we don't
fix them.
All the cases have been visually inspected and confirmed that
the existing "return" statements always belong to anon
functions within the test body and not the test own return statement.
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.
After some tests, it seems that we can safely cover
phpunit_util::reset_all_data() executing it within
own basic_test self tests.
That way we can confirm that the reset code is doing its job
and detecting unexpected changes at various levels (database,
globals, ...).
Note that, in order to catch the E_USER_WARNINGS, for PHPUnit 9.6
and up, we have to convert them to exceptions, because the notice/
warning/error expectations have been deprecated and will be removed
in PHPUnit 10. So we are using a trick, already used also by
advanced_test. And, no matter that we are repeating the trick
a few times, that's ok in order to have all its uses controlled.
There are a few tests in core that are commented since the
beginning. That's not useful at all, so with this commit we
are un-commenting them instead.
Note that this is an initial step to have them back with skipped
outcome. Later in this issue we'll decide about to keep them or
completely remove them (each test can have a different outcome).
There are inherent issues with task blocking which mean that it has
never worked properly. It is also very buggy and can lead to massive
performance issues with task processing.
It is almost impossible to deprecate this in a staged fashion because
various APIs use the methods and it is not possible to determine which
are API uses and which are other valid uses.
In reality there has been little-to-no uptake on the use of this feature
and it should just be removed.
Using DI for all hook access means that it becomes significantly easier
to mock hooks and callbacks for unit testing without fundamentally
altering the structure of the code purely for the purposes of unit
testing.
This commit adds the PSR-20 ClockInterface to core, with a
moodle-specific extension to the Interface at `\core\clock`, and a
standard clock at `\core\system_clock`.
Further clocks are provided as `\incrementing_clock` and `\frozen_clock`
which are available to unit tests using:
- `$this->mock_clock_with_incrementing(?int $starttime = null);`
- `$this->mock_clock_with_frozen(?int $time = null);`
For the incrementing clock, every call to fetch the time will bump the
current time by one second.
For the frozen clock the time will not change, but can be modified with:
- `$clock->set_to(int $time);`; and
- `$clock->bump(int $seconds = 1);`
PHPUnit 9.6 has deprecated all the expect(Deprecation|Notice|Warning|Error)
assertions, so we have to move away from them.
In core, we only had 2 cases, one easily fixed by getting rid of it,
because, for php >= 80 it's an assert-able exception.
And the other replaced with code that, temporarily, sets a custom
error handler that converts any specified E_ to an asset-able
exception.
Note that tests playing with error handlers should, always, be
run in separate process, to avoid problems or conflicts with
PHPUnit / Moodle own error handlers.
To be integrated as part of MDL-81266
When running PHPUnit 9.6 we get the following deprecation warnings:
"assertObjectNotHasAttribute() is deprecated and will be removed in PHPUnit
10. Refactor your test to use assertObjectNotHasProperty() instead."
So we replace all instances of assertObjectNotHasAttribute with
assertObjectNotHasProperty.
PHPUnit justifies the change with:
> PHPUnit currently refers to "fields" (see above) as "attributes". This
> is (or will become) confusing considering the introduction of
> attributes in PHP 8 and their support in PHPUnit. PHPUnit will be
> changed to use the term "property" instead of "attribute" where "field"
> is meant.
Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
This adds a few changes to the old test_context_not_used test:
- Move it to become an advanced_test, because its mission
is to verify that the assertEventContextNotUsed() assertion
works as expected.
- For consistency, also move the fixtures to own phpunit fixtures.
- Add proper coverage tags, to verify that the assertion is being
covered.
- Add a data provider to provide all the current cases and ease
any future case that may be needed in the future. One by one
because previously there was code never executed with the
warning expectation causing the test to stop.
- Run them in isolation, while this is not strictly required, it's
including external fixtures and, we'll need that isolation soon
(for changes coming when moving the test to PHPUnit 9.6 in MDL-81266).
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.
The only tests that have COMPLETE* support for the debugging sink
are the advanced_testcase and the database_driver ones (store and
report). So we must ensure that the rest of tests don't use the
debugging sink at all.
Right now we are using it for storing, but later there is not
reporting, so any debugging happening within non advanced tests
is not detected.
This commit just ensures that we stop making that storing for
non advanced/database_driver tests. Nothing more, nothing less.
* Note that we have had to add a few missing bits to the
database_driver testcase because it was not 100% complete. Now
it behaves 100% the same than the advanced_testcase one regarding
the debugging sink.
The \core\di class is a Moodle wrapper to php-di which is intended to
allow Moodle to switch to an alternate DI solution in the future if
required. All interaction with the container uses the PSR-11 Container
interfaces, which allows for normalisation of configuration, setting,
and retrieving of DI container-identified classes.
- Added a new method called get_messages_by_component()
This method will help to retreive the redirected messages
of specific component only
- Added a new method called get_messages_by_component_and_type()
This method will help to retreive the redirected messages
of specific component and type only
During the bootstrap of PHPUnit we ensure that the database has been
reset to its initial state.
We do this by checking the internally-stored DB write count between
runs. If the count is not yet set (null), or it has been increased, we
force a reset.
When running an isolated test the test runner resets the database, it
then sets up a new isolated test environment by writing a new PHPUnit
test case and passing it to a new PHP Process using standard in. As part
of this, the bootstrap is run for that process.
Because we are in a new process, the db write count is fresh and not yet
set. This has been leading to an additional db reset before the isolated
test.
To handle this we want to _not_ perform a reset during the
initialisation for isolated runs. We know that the DB is in a fresh
state before we start the run.
To support this we need to know whether the test is an isolated test
during the bootstrap, which means we cannot use the previous approach to
calculating this.
Instead we look at the PHP_SELF value. PHP sets this to "Standard input
code" when run from stdin, instead of running a file.
There should not be any other legitimate reason to run a PHPUnit
bootstrap via this stdin approach.
Unfortunately this approach is a little bit risky as it depends on the
presence of a specific string, however this string has been in place
since 2016, and there is no legitimate way of calculating this.
I did consider looking at whether the called script included `/vendor/`
and `/phpunit`, but this is also likely a risky approach if someone
calls PHPUnit in an unexpected way.
This approach is itself unit tested so any change to PHP's stdin string
before we deprecate this approach entirely in 12 months time will be
caught.
When we deprecate the use of a file, we often include tests which ensure
that the legacy behaviour is maintained. There are also legacy uses
in the community where people would like to use the deprecated API for a
period.
The issue that we face is that, if the deprecated file is included once,
then it will be included for all other, unrelated, tests. This means
that other tests may not detect cases where the deprecated file was
included.
We can solve these cases by running the test that performs the inclusion
in a deprecated process. This means that the inclusion is only performed
in that isolated process, and other unrelated tests do not include the
file.
However, we also then need to detect which files which are including the
file and which we do not know about.
This change introduces:
- an override to the TestCase::setInIsolation method to define a
constant when the test is running in isolation
- a new function that a file can call when it is included, to make sure
that the test process was isolated, where there is any test.
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.
