For example, documenting the method getUser() with "get the User
object" does not add any information that's not already there.
But I have to read the text first to understand that it doesn't
document anything that's not already obvious from the code.
Some of this is from a time when we had a PHPCS sniff that was
complaining when a line like `@param User $user` doesn't end
with some descriptive text. Some users started adding text like
`@param User $user The User` back then. Let's please remove
this.
Change-Id: I0ea8d051bc732466c73940de9259f87ffb86ce7a
The editPage() convenience function in MediaWikiIntegrationtestCase
should optionally accept a WikiPage instance as a parameter. This
allows edits to be made conveniently, without any WikiPage instance
held by the caller going out of sync.
Change-Id: I68ad6df5f6dc357658fe112957bea8b11cf2471e
This reverts commit c17ddfd786.
This time, also: Trying to get mock http to work in parser test
Bug: T262443
Change-Id: Id708cdfa3d3c27f30543c6bf212df96e6b0a69e1
Tests should not make real HTTP requests. Mock out the
HTTPRequestFactory service to prevent this.
Bug: T262443
Depends-On: I63bfd54c3de55d678e8b862b85c0adfb5fc94d91
Change-Id: I1702c11928f8760bb41b41f4c7c04d7af03f62e2
This is causing problems for Parsoid CI, as parser tests fail when
phpunit runs the tests at a different point than they are run in
core's CI due to the side-effects of content-language changes made in
other phpunit tests. (For example, phpunit runs all extension tests
after core tests, so the same parsertest can pass if included in core
and then fail when included in an extension.)
SpecialPageFactory::$aliases has a dependency on the current content
language, with no way to reset it other than to recreate the
SpecialPageFactory.
Change-Id: I278580ed5cf2c85403cbaf601f8af4753e14a9d0
The PHPDoc for the editPage method stated that if $user is null,
the test sysop user will be used instead. This is not the case in
practice, as the method simply passes null to the doEditContent
method.
As Ammarpad has pointed out in a comment, this confusion is due to
I5a10163 assuming the sysop would be used, which is incorrect.
It's also probably better for the default user to be a "regular"
user, not a sysop, so I changed it to getTestUser() and noted this
in the doc block.
Depends-On: I7a79e0eaa1617e4d87a8d615a5391723c0e30b6a
Change-Id: I9f77474f40e0f6901aa2c6f846e471b822636aa5
Since we reset all services between test cases, no special effort is
needed to restore the state of the HookContainer.
Bug: T255056
Change-Id: I364f2943d56644f9c7c3acc053e6b585bd8f535c
Previously, clear() and scopedRegister() would not disable hook handlers
registered using the new mechanism. Only old style hook handlers would
be suppressed.
The new implementation is based on tombstones: Instead of actually
removing registered handlers to override them, tombstone markers are
put into place to disable existing handlers until the tombstone is
removed again. This allows hook handlers to be disabled temporarily
in a consistent way.
This removes HookContainer::getOriginalHooksForTest(), which is
incompatible with the new logic.
Bug: T255056
Change-Id: I08c1824797ac60a5098a52b5781af8ac4dd38928
Migrate all callers of Hooks::run() to use the new
HookContainer/HookRunner system.
General principles:
* Use DI if it is already used. We're not changing the way state is
managed in this patch.
* HookContainer is always injected, not HookRunner. HookContainer
is a service, it's a more generic interface, it is the only
thing that provides isRegistered() which is needed in some cases,
and a HookRunner can be efficiently constructed from it
(confirmed by benchmark). Because HookContainer is needed
for object construction, it is also needed by all factories.
* "Ask your friendly local base class". Big hierarchies like
SpecialPage and ApiBase have getHookContainer() and getHookRunner()
methods in the base class, and classes that extend that base class
are not expected to know or care where the base class gets its
HookContainer from.
* ProtectedHookAccessorTrait provides protected getHookContainer() and
getHookRunner() methods, getting them from the global service
container. The point of this is to ease migration to DI by ensuring
that call sites ask their local friendly base class rather than
getting a HookRunner from the service container directly.
* Private $this->hookRunner. In some smaller classes where accessor
methods did not seem warranted, there is a private HookRunner property
which is accessed directly. Very rarely (two cases), there is a
protected property, for consistency with code that conventionally
assumes protected=private, but in cases where the class might actually
be overridden, a protected accessor is preferred over a protected
property.
* The last resort: Hooks::runner(). Mostly for static, file-scope and
global code. In a few cases it was used for objects with broken
construction schemes, out of horror or laziness.
Constructors with new required arguments:
* AuthManager
* BadFileLookup
* BlockManager
* ClassicInterwikiLookup
* ContentHandlerFactory
* ContentSecurityPolicy
* DefaultOptionsManager
* DerivedPageDataUpdater
* FullSearchResultWidget
* HtmlCacheUpdater
* LanguageFactory
* LanguageNameUtils
* LinkRenderer
* LinkRendererFactory
* LocalisationCache
* MagicWordFactory
* MessageCache
* NamespaceInfo
* PageEditStash
* PageHandlerFactory
* PageUpdater
* ParserFactory
* PermissionManager
* RevisionStore
* RevisionStoreFactory
* SearchEngineConfig
* SearchEngineFactory
* SearchFormWidget
* SearchNearMatcher
* SessionBackend
* SpecialPageFactory
* UserNameUtils
* UserOptionsManager
* WatchedItemQueryService
* WatchedItemStore
Constructors with new optional arguments:
* DefaultPreferencesFactory
* Language
* LinkHolderArray
* MovePage
* Parser
* ParserCache
* PasswordReset
* Router
setHookContainer() now required after construction:
* AuthenticationProvider
* ResourceLoaderModule
* SearchEngine
Change-Id: Id442b0dbe43aba84bd5cf801d86dedc768b082c7
The new HookContainer.php introduces a scopedRegister() method for
temporarily setting hooks. Let's use that in MediaWikiUnitTestCase
and MediaWikiIntegrationTestCase instead of directly accessing
global $wgHooks to do so.
Also introduces setTemporaryHook() and removeTemporaryHook()
methods in MWIntegrationTestCase for easily adding/removing of
temporary hooks.
Bug: T250300
Change-Id: I8cefd41b66f882c53646b76de76c51f0d8730f72
This existed on MediaWikiIntegrationTestCase, but not on
MediaWikiUnitTestCase. As a result of that, I spent about four
days tracking down a dangling AtEase::suppressWarnings with
missing AtEase::restoreWarnings (as part of Ib6758d724c).
Move it to the common MediaWikiTestCaseTrait instead so that we
get it on unit/ as well.
Example:
> There was 1 failure:
>
> 1) Pbkdf2PasswordTest::testCryptThrows
> PHP error_reporting setting found dirty.
> Did you forget AtEase::restoreWarnings?
Change-Id: I7dc3fe90385c8066b89a5e06c55f5455edfbb4ca
This converts user options management to a separate
service for use in DI context.
User options are accessed quite early on in installation
process and full-on options management depends on the
database. Prior we have protected from accessing the DB
by setting a hacky $wgUser with 0 id, and relying on the
implementation that it doesn't go into the database to
get the default user options. Now we can't really do that
since DBLoadBalancer is required to instantiate the options
manager. Instead, we redefine the options manager with
a DefaultOptionsManager, that only provides access to
default options and doesn't require DB access.
UserOptionsManager uses PreferencesFactory, however
injecting it will produce a cyclic dependency. The problem
is that we separate options to different kinds, which are
inferred from the PreferencesFactory declaration for those
options (e.g. if it's a radio button in the UI declaration,
the option is of multiselect kind). This is plain wrong,
the dependency should be wise versa. This will be addressed
separately, since it's requires larger refactoring. For now
the PreferencesFactory is obtained on demand. This will be
addressed in a followup.
Bug: T248527
Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
On PHP 7.4 and above, a memory leak can be observed in
ServiceContainer::loadWiringFiles when running PHPUnit tests. This patch aims to
work around the leak by changing
MediaWikiIntegrationTestCase::installMockMwServices to cache and reuse the
original service wirings if no config overrides were supplied. This
significantly reduces the amount of times that the wiring closures need to be
included; on PHP 8, it caused memory usage to drop to 894.25 MB from 4+ GB when
running the PHPUnit tests for MediaWiki core.
However, it seems to be causing some test failures and so it is currently a WIP.
Bug: T247990
Change-Id: I24971221b8accf78d61479d286e907c1111212a0
* Split MWDebug::sendRawDeprecated() from MWDebug::deprecated(). The new
function can be used to send arbitrary messages to the deprecation
log, rather than being constrained by the fixed format of
MWDebug::deprecated().
* Split formatCallerDescription() from sendMessage() to allow the caller
of sendRawDeprecated() to do its own caller formatting.
* Use the new function in MWLBFactory::logDeprecation()
* In tests, replace the ugly implementation of hideDeprecated() with one
that works by setting a list of regexes to filter. hideDeprecated()
now filters deprecation warnings that are a string match to the
supplied function. filterDeprecated() can be used to filter a regex,
and is intended to be used to filter warnings sent via
sendRawDeprecated(). The filter list is reset at the start of each
test, instead of leaking across tests as before.
Change-Id: I0d0df86db2e61cdd1769426bfa7bad4c2ae5e977
Only support passing RevisionRecord or int, remove use of
Revision::newFromId
Bug: T249021
Bug: T249561
Change-Id: Id4a8f64f239d0664865056887fe0a11c7e468d5f
Allow truncation of multiple tables. This also provides for
a way to avoid risky keywords like CASCADE for Postgres.
For Postgres, use RESTART IDENTITY, which has been supported
since Postgres 8.4.
Avoid TRUNCATE/DELETE queries for empty temp tables, which is
useful for integrations tests that frequently call this method.
Reorganize and tweak the regexes in Database::getTempWrites().
It now recognizes multi-table DROP/TRUNCATE (Postgres-style).
Change-Id: Idd49f118b20ea5a0f7a3e8c00369aabcd45dd44e
To disable mail in tests, use setTemporaryHook after the
service container setup, instead of the soon-to-be deprecated
Hooks::register().
Change-Id: I84ea69feb0ebf81855b745c14fdba50d4e121de8
== Motivation
Mute a log channel, for which the Logger object is injected by
service wiring, for a service that is overridden by default,
such as 'DBLoadBalancerFactory'. For that, calling setLogger()
mid-test would be too late.
== Changes
* Add a test-only method to LegacyLogger that makes it possible
to change its `minimumLevel` attribute, thus making it turn
itself into a NullLogger if raised to infinity. This is the
same principle we use already for disabled log channels when
using MediaWiki normally (see LegacyLogger::__construct).
* Previously, the developer's LocalSettings.php was loaded
which includes the Spi configuration. This meant other Spi's
could be configured which means we might not be dealing with
a LegacyLogger object.
Similar to what we do with ObjectCache and JobQueue already,
make the default Spi in tests the same as the normal MW default.
* Add setNullLogger() which makes use of these two.
Bug: T248195
Change-Id: Ieade3585812de47342259afa765e230fff06f526
Use it in MediaWikiIntegrationTestCase for resetting tables.
Also create Database::resetSequencesForTable() helper method from
the resetSequenceForTable() methods in the SQLite/Postgres classes.
Change-Id: I20945e20590e69340b1ce75f6bb2f6972375b00c
Also fix PHPUnit 9 warning in PNGMetadataExtractorTest about $delta.
This should fix all of the integration test warning spam.
Bug: T244095
Change-Id: I0e2a76d5df2685ae5ad1498864e0b5f9db60c0cc
Already soft-deprecated in 726f10b, which also removed the
unused assertTypeOrValue() method without deprecation.
Change-Id: I0d417bf79f03e899dccfa469f639c04d4cf999cf
assertArrayEquals is a generic assertion that does not have anything
to do with integration testing, and is quite useful for isolated
testing of classes which return unordered sets.
Change-Id: I45af77402a1bc922579aff14b21874ec8680e765
*assertType is marked as deprecated, and should ideally be removed soon
(i.e. no hard deprecation to follow)
*Most usages of assertType in core were autofixed by using I8ef556b630812aeea77c5606713f53d9af609f1b
*assertTypeOrValue was removed because only used in SiteTest
(codesearch: https://codesearch.wmflabs.org/search/?q=assertTypeOrValue&i=nope&files=&repos=)
*SiteTest::assertTypeOrFalse was removed because unused
Bug: T192167
Change-Id: Icb3014b8fe7d1c43e64a37e0bdaaffec18bb482f
We've decided to switch to PHPUnit 7 to make things easier. This also
means that we shouldn't suppress PHPUnit 8 warnings, otherwise we cannot
know if tests are already compatible with PHPUnit 8.
Note that the trait is not deleted, because it may still be necessary
for single tests -- we're just not including it by default. However, it
is now deprecated, and its use is discouraged.
Bug: T192167
Change-Id: If90e6b6ffe03dfb117ec2b4b13c79983fe1bd0f2
The class PHPMaintClass is meant to be loaded when running phpunit.php
which is only done when running tests with it. When running tests by
using the bootstrap file only, e.g. in an IDE, this class will not be
available. Relying on it in other classes will therefore break them.
Moving the required parts to another outside class and add it to the
test autoloader. My feeling says, that adding the PHPMaintClass to the
autoloader says "NOOOO", that's why I added a new one.
There also seems to be some CI builds failing because of that:
https://integration.wikimedia.org/ci/hob/quibble-vendor-mysql-php72-docker/30642/console
Bug: T151101
Change-Id: I33e27009657a951173694fc847973560a1ce967b
This is moved out of the main PHPUnit upgrade commit to allow
extensions to silence their warnings beforehand, to avoid breaking
their tests.
Bug: T192167
Change-Id: I60379a933a3a1b018d9f5ef6b51019002389dda3
