Note that there wasn't any case of multiple testcase classes in
1 file. All the cases reported in the issue were false positives
caused but other "mock/fixture" files being named _test.
So all this issue does is:
1) rename any _test suffixed class in test files, because we are
going to start renaming a lot of test classes to _test.
2) ensure that the 2 test case classes modified in this issue,
are already observing the filename = classname rule that will
be implemented soon (and verigy it works).
This class would belong more appropriately within the 'user' API
(core_user) instead of within the 'core' API, since it is
directly related to user data.
Since the class has only just been added to Moodle, now is a good
time to move it.
Apart from adding new unit tests for covering new IMS OBv2.1 issuer,
some extra tests have been added to test existing issuers and confirm
they are still working as expected after the changes.
IMS OBv2.1 services have a registration endpoint to get client id
and secret.
This patch adds and implements the "register" method for the IMS
Open Badges Connect discovery system, to get the proper client id
and secret values.
In order to make easier to create and maintain new OAuth2 services,
a couple of classes have been added:
- discovery\* contains methods related to the discovery system. Until
now, only OpenID Connect was supported by Moodle so all the code was
centralised in api.php.
With this patch, as IMS OBv2.1 has a different discovery system, a new
abstract class (base_definition) has been added to be called and let
every discovery system (OpenID Connect, IMS Badge Connect...)
implement their own methods.
- service\* . Instead of keep adding methods to the api, the service
namespace has been created to store all the standard issuer services.
An interface (issuer_interface) has been created and all the services
should implement it.
This patch creates the "IMS OBv2.1" and "Custom" services and moves
the methods for "Google" service because it uses the OpenID connect
discovery system.
I've gone over a few of the mofified files (those
which were showing warnings and errors to CiBoT. Some of them
have been fixed completely, while others only have fixed
for the lines belonging to this issue (lib/tests/moodlelib_test.php)
for example.
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
In all cases changes have been kept to a minimum while not making
the code completely horrible. For example, there are many instances
where it would probably be better to rewrite a query entirely, but
I have not done that (in order to reduce the risk of changes).
Some of the current behat tests were wrong but not failing because of the bug in 'should not exist' step.
Once the bug is fixed, we must fix also wrong behats.
After uninstalling a plugin, the translated strings remain in
tool_customlang table, throwing an exception when trying to localise
any strings. Currently there is no mechanism to
clean up customlang tables and files during the uninstall process,
so with this patch the invalid components will be ignored.
As discussed in MDLSITE-6397, all Moodle code files should end with
single Unix-style end of line character. Files created and edited
through the XMLDB editor should follow this rule, too.
* When a label is passed to an (advanced) checkbox and no text
was provided, the label will be displayed on the right of the the
checkbox element. So there's no point in passing an empty string for
the label just to render the text to the right of the checkbox.
Uses of the httpsreplace and customlang tools are known to be extremely
slow.
In our CI infrastructure on slwoer DBs, the customlang tool can take
over 90 seconds to load the page, whilst the httpsreplace tool can take
up to about 60 seconds.
These changes set appropriate timeout factors to increase the timeout
accordingly.
These steps were completely pointless and were failing with the W3C
Webdriver becuase focus was never on the Update Profile button in the
first place.
At a minimum we should remove the Focus change step as it not valid, but
the following step is also pointless.
