I believe this makes the code less brittle, and also makes it a bit
more obvious what these strings are meant to represent.
Change-Id: Ia39b5c80af4b495931d0a68fd091b783645dd709
This method is called relatively often (especially now that $this->db is
being replaced with $this->getDb() everywhere, T316841), and it’s not
entirely cheap: the underlying isTestInDatabaseGroup() method needs to
create a ReflectionClass for the test class and run a regex on its doc
comment. As the result is a constant for any single class (its doc
comment cannot change), we can easily cache it.
In a simple benchmark, caching the result reduces the time for 10
million self::needsDB() calls from 7.97 seconds to 2.28 seconds, and for
10 million $this->getDb() calls from 68.70 seconds to 61.60 seconds –
not a huge difference, but not nothing either. Some logging in changes
Ia7f622ff87 and I4454c00958 indicates that there are about a dozen test
classes (in the extensions pulled in by MediaWiki core and/or Wikibase)
that call needsDB() at least a thousand times, and two test classes
(ResourcesTest and ScopeStructureTest) that call it at least ten
thousand times; based on this, I think the expected savings are
somewhere between a tenth and a hundredth of a second per test run.
The “indexed by class name” trick to get per-class static method
variables is directly taken from the PHP RFC [1]; prior to PHP 8.1, a
plain static boolean would have worked as we need it in this case.
[1]: https://wiki.php.net/rfc/static_variable_inheritance#backward_incompatible_changes
Change-Id: I7917cb58aa0c86428798a207f7ad63484e8eb8ad
Why:
- With temp accounts enabled, IP addresses cannot be the performer of a log entry.
- The RequestContext user by default is an IP address, so needs to be set to a named user
to allow the test to pass when temporary accounts are enabled.
What:
- Set the context user to a named account
Bug: T365669
Change-Id: I2af80141c6c63073aaa16f8f84b9bf98fcd4b4ac
With the ObjectCacheFactory refactor in T358346, this service is
pretty much used in 2 places, (1) service wiring which can be replaced
and (2) in MediaWikiIntegrationTestCase.
The service can be removed entirely and the local cluster cache gotten
from the ObjectCacheFactory service injected via service wiring. Hence,
usage of this internal service can be removed.
NOTE: See followup: 7475063 for the TODO to drop this service entirely.
Change-Id: I173f04456df6cc1782cec2f16d6653c6b0826dce
This patch completes the rest of the ObjectCache refactor and
migrates methods to the appropriate class while deprecating them
in `ObjectCache.php`.
It also moves the `_LocalClusterCache` internal service logic
into ObjectCacheFactory and calls that instead making sure that
wiring code stays wiring code and let the class do the heavy lifting.
`::makeLocalServerCache()` is retained as a static method in the
ObjectCacheFactory class because it's called early in Setup.php
before the services container is available (so it needs to be stand-
alone).
To add, we also converts all global variables that were used in the
`ObjectCache.php` class into the config schema approach and retrieves
them using ServiceOptions injected in service wiring.
NOTE: MediaWikiIntegrationTestCase::setMainCache() was slightly
rewritten to take care of service reset which throws away the cache
object preserved by setInstanceForTesting() after service reset.
Instead, we preserve the object via MainConfigNames::ObjectCaches
setting with a factory closure which returns the correct cache object.
As a nice side effect of the above, the setInstanceForTesting() method
was removed entirely.
As a follow-up to this patch, I would like to remove the internal
_LocalClusterCache service in a stand-alone patch.
Bug: T363770
Change-Id: Ia2b689243980dbac37ee3bcfcbdf0683f9e1779b
`MessageCache::parse` pulls in global `$wgTitle` on line 1492 and
sets the page for the parse to this value of none is set. Some
tests (for example SummaryFormatterTest in Wikibase that extends
MediaWikiLangTestCase) interact with the MessageCache in contexts
where the page is not set, making the test unreliable if other
test cases have run beforehand and set `$wgTitle`.
Bug: T365130
Change-Id: Ib4ee7dc5102d5998aa65dcafb4efa531c2533c69
According to the dictionary, "per" (or more conventionally "as per")
means "according to". Refer OED "per" sense II.3.a. For example:
"No value was passed, so return null, as per default".
In this sentence, we are not specifying the default, we are referring
to the default. This correct usage of "per default" was used nowhere
in MediaWiki core as far as I can see.
Instead we have "per default" being used to mean "by default", that is,
giving the value to use when no explicit value was specified.
In OED, the phrase "by default" is blessed with its own section just
for computing usage:
"P.1.e. Computing. As an option or setting adopted automatically by a
computer program whenever an alternative is not specified by the user
or programmer. Cf. sense I.7a."
There are highly similar pre-computing usages of the same phrase,
whereas the phrase "per default" is not mentioned.
As a matter of style, I think "per default" should not be used even
when it is strictly correct, since the common incorrect usage makes it
ambiguous and misleading.
Change-Id: Ibcccc65ead864d082677b472b34ff32ff41c60ae
Changed some inserts to use multi-row insert for small performance
benefit where possible and not already used.
InsertQueryBuilder does not return a value, deprecated since 1.33
Bug: T353219
Change-Id: I2380ebc8ec8db178dd790247aefbdd798b6d62ff
ObjectCache is already doing a lot of factory pattern logic like
creating instances of the various BagOStuff, this should really be
the responsibility of the factory servicet.
This patch introduces a proper factory (ObjectCacheFactory) to handle
the responsibility of creating various instances of BagOStuff. Since
`newFromParams()` is a static function that gets passed in configuration
of $wgObjectCaches, that can stay that way (to keep supporting how we do
this in prod today).
Technical Breaking Change: `ObjectCache::makeLocalServerCache()` now has
a parameter and requires it but there are no callers of this method outside
MW core hence it is safe to change (and this patch update all callers) to
work correctly. Cache prefix is gotten from global state because sometimes
at this stage, the services container is not available.
Bug: T358346
Change-Id: I3179a387486377c6a575d173f39f82870c49c321
FileBackendIntegrationTest was running tests against different backends
in an unconventional way, using a combination of wrapper test cases that
run tests against two different classes, and CLI options which don't
really exist anymore and have an associated fixme.
So:
* Move the bulk of FileBackendIntegrationTest to a new abstract base
class under tests/phpunit/integration.
* Add subclasses for the FS and multiwrite test cases. This allows us to
eliminate the wrappers.
* Add a subclass for MemoryFileBackend.
* Add a Swift subclass which replaces the main use case for
the CLI option --use-filebackend. It is automatically enabled when
a Swift backend is configured, similar to PostgreSQL tests.
* Some miscellaneous tests with a medium level of integration, not
requiring backend setup and teardown, were moved to new classes
FileBackendMultiWriteTest and FileBackendStoreTest.
Change-Id: I0da531349d7627970a7bcb34f3c1f5fd7e05cb21
For various reasons such as support and simplicity, multi-table
TRUNCATE ends up just being a for-loop anyway. The interface is
simpler if it just takes a string table name.
Migrate callers and deprecate the old method.
Change-Id: I37ee054ca24e6ba547f8c76aec5408aaedab364b
Update cases where one of the IConnectionProvider methods is called
immediately.
This doesn't really change anything, but I hope it helps promote
getConnectionProvider() as the common way to do this.
Follow-up to 8604c384f6.
Change-Id: Id0e7d02bab0c570343c2b1f03c70b44ee39db112
This fixes errors in paratest since the test class previously relied
on some prior test happening to cause site_stats to be initialized,
an assumption which does not hold when classes are run concurrently.
Change-Id: Icf24db250dd3b743efbaf925c075b4e1aed25f44
* Have ChangedTablesTracker take a domain as a parameter. Getting the ID
out of it is deferred to save some nanoseconds in production.
* Split getTablesAndStop() into getTables() and stopTracking(). Make
getTables() take a domain ID parameter.
Change-Id: I52ade87e2f0305e6f2be541df5b38c7d76c409a7
Follow up Id9ab64fc8b09d9 which made listTables() consistently exclude
views.
Hard deprecate Database::listViews() which was only used for view
filtering of listTables(), conditional on database type.
Add an integration test for the new listTables() behaviour.
Change-Id: I3402a227f92b35192c6385c6aeab461de43b9f58
Do that in the bootstrap and remove
initializeForStandardPhpunitEntrypointIfNeeded. The problem with the
latter is that it runs after data providers, so it wouldn't be possible
to read config etc. there. You may argue that dataproviders shouldn't do
complex things such as reading config. That's a valid concern, but it's
not easy to achieve in the current state of things. This decision might
be reconsidered if the MW settings infrastructure is improved and moved
away from globals.
Also, doing a deferred initialization in MediaWikiIntegrationTestCase
means that (unit) tests that run before said initialization and the ones
running after would see different states of the system (one where config
has been loaded, one where MediaWiki might not even be installed). Doing
it earlier guarantees that there won't be such differences inside a test
run.
Ideally, we would use PHPUnit events to determine if we need settings
once we have a list of tests to execute. But that can't happen until we
migrate to PHPUnit 10.
Print a message saying whether MW settings before starting PHPUnit, as
this might be useful when debugging failures.
Change-Id: I467f82ed9a88cf2cfa63f76abac9c264904eb492
Changes to the use statements done automatically via script
Addition of missing use statements and changes to docs done manually
Change-Id: Ib326ae1e5c8409a98398c721e8b8ce42c73bd012
The `resetServices` call here is not the primary, documented effect
of `setMwGlobals` (as well as all the other functions that call
`setMwGlobals`). It's a necessary side-effect. But it's only
necessary when something actually changed.
A lot of tests do a lot of `setMwGlobals` calls just to be sure the
configuration is as expected. But more often than not it is already
fine.
Tests that depend on this `resetServices` call even when no
configuration change is made shouldn't rely on a hidden side-effect
but must do something else about this.
According to my local tests this makes a huge difference. I have
seen the runtime of test suites drop by as much as 1/3.
Change-Id: Id53c8da65d08730aa9536ea2ae71211c5170134a
If anything goes wrong during the test database setup, we don’t want
future tests to think that the test DB setup is done, skip repeating it,
and blithely run against the real DB. It’s better to have them repeat
the setup (and potentially just get the same errors again and again).
The assignment was last moved upwards in change I2119b02333 (commit
9d74ee8bd8); at the time, this was necessary to deal with an early
return in the method. (Though I would argue that the assignment should
have been duplicated in the early return instead.) But there’s no early
return here anymore, so that shouldn’t be a problem.
Bug: T352695
Change-Id: I9f0f5bcd8cd5ae11f0ff2e23b2ca2e63dd798854
This patch deprecates the $tablesUsed property, and introduces a new
utility, ChangedTablesTracker, that automatically keeps track of all
tables changed in a test. Every table used is now guaranteed to be reset
after the test.
Note that tables changed in addDBDataOnce are only cleared at the end of
the test class, or the data would be lost.
Fix a test in SpecialBlockTest which would fail with this patch.
$tablesUsed is now a no-op and can be removed from all tests that
declare it.
Bug: T342301
Change-Id: Ie2f1809dac243ef06ba0c34f039ce4e62cbf99cf
PHP 8.2 has deprecated dynamic property creation on classes that do not
explicitly allow it via the #[AllowDynamicProperties] annotation. The
recommended migration path for associating arbitrary data with objects
is the WeakMap class, but it is only available on PHP 8.0 and above.
Since MediaWiki still supports PHP 7.4, and the test framework needs to
be able to associate some state with objects it does not own, like DB
connection handles, introduce and use a new DynamicPropertyTestHelper
shim class that uses WeakMap if available and falls back to regular
dynamic property creation otherwise. Convert the DB setup-related
dynamic property usage in MediaWikiIntegrationTestCase to use this
class.
Bug: T326466
Change-Id: I1054f6f944d491b536949cada33e2ac670e026f8
… or with $this->fail() from the PHPUnit TestCase base class.
I hope this makes the code more readable, i.e. communicate the
intention better. The output should be the same, i.e. the test fails
as before in case of an error.
Change-Id: Ied8a045141ac92d6af6398682bb5d9ca7ca88c49
The method no longer has non-static dependencies. By making it static,
it will be possible to call it in the tearDownAfterClass hook method to
clear tables filled in addDBDataOnce.
Also make the IDatabase parameter not nullable, none of the two callers
is ever passing null. And also make the method return early if
$tablesUsed is empty.
Change-Id: I4bac154cf6bb4007abd4be7ca31090de5d33e7d5
The method should never be called directly, so make it throw an exception.
Nonetheless, mark it as deprecated and detect overrides in the
constructor, so that anyone who tries to override this method will see a
warning.
Fix the few tests that were relying on the existence of the test page.
Bug: T342428
Depends-On: Ic64ded5e2c0b59e7c888ece9566076058a125be4
Change-Id: I308617427309815062d54c14f3438cab31b08a73
None of these methods needs to access the class state. For some of them
(e.g., truncateTables), the change is needed so that we can call the
method from static hooks (like tearDownAfterClass).
Bug: T342259
Change-Id: I25b09a22eebd0c6db7ea7058800681e43e4ce43e
