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
This has always been a hack. PHPUnit provides hook methods exactly for
this. It might have been a bit better 13 years ago [1], when using
PHPUnit 3 and this method only had 3 lines, but today it's become
bloated.
PHPUnit 10 makes run() final, so we'll have to move away sooner or
later.
Move code to the @before and @after methods, which are run relatively
close to run().
Make MediaWikiIntegrationTestCaseTest use @depends to test that the
tearDown logic works correctly. The previous code was too tightly
coupled with the specific implementation of mediaWikiTearDown.
[1] - https://static-codereview.wikimedia.org/MediaWiki/79173.html
Bug: T342259
Change-Id: I4dee53176fc90a7884600422c30dc904cbd1a7d6
If a test needs to do something with the database, it should be in the
`Database` group. Therefore, there should be no need to setup the DB if
that isn't the case.
Also make sure that the real DB cannot be accessed if the test is not in
the Database group.
MediaWikiIntegrationTestCaseNoDbTest was partly copied from I2b2eff78.
Bug: T155147
Depends-On: I11824a635fb43cfac8560b8fdecd556c82b9ce42
Depends-On: Ia12e28b4e23f111c0b4288e9ac19d3ad10f30d6c
Change-Id: I96ecf9ffd5fbf18db3e15e8774f7dc2fe5b9fd2a
Subclasses are not supposed to override this to change the behaviour.
Any test that needs the database, even if conditionally, should be in
the Database group to allow filtering.
Also remove the $tablesUsed check: we already throw an exception if the
test has $tablesUsed but is not in the Database group, so there's no
need to check that twice. This also allows us to make the method static.
Move said check to the setup method, as that's where all the setup
should be eventually.
Update the documentation on various methods to reflect the fact that the
only way to mark a test as using the database is via the Database group.
Note that making a method static is a backwards-compatible change, but
callers in MediaWikiIntegrationTestCase are updated anyway for
consistency.
Bug: T342259
Depends-On: Ied6a69a1e4beb7fbb4d5ce75b57f54382d929684
Change-Id: Ie72a0dff902feee92a183b2dc2926bd2ad1df071
Mostly things related to the test database. Also adjust visibility and
remove getTestPrefixFor(), which is only used in core according to
codesearch.
Making a method static is a backwards compatible change, as invoking the
method non-statically is valid.
Bug: T342259
Change-Id: I6111fb5ff5f3c87d5d3f9188b3f50351391a29c3
UTPage is badly named, because it doesn't give any information as to
what test caused the page to be created. It also sort of encourages test
authors to rely on this "UTPage" page being created by the framework for
them.
Both these things are dangerous, or at least very questionable. Use a
random page title instead, but include the caller name in case someone
needs to investigate where a test page is coming from.
Do the same for summary and content, too.
In getExistingTestPage, add a check to make sure that the page was
created successfully. Do not use assert* to avoid adding assertions
extraneous to the test.
addCoreDBData is not changed because that method will be removed
entirely (T342428).
Fix tests that are now failing:
- ParsoidOutputAccessTest was relying on the content of
getExistingTestPage to be UTContent.
- HTMLHandlerTestTrait did not account for spaces in the page name (also
change the signature to reflect the fact that WikiPage is always
passed in).
- HtmlInputTransformHelperTest was relying on the fake test page to be
there.
- PoolWorkArticleViewTest is leaving pages behind, and for some reason
that's making SpecialRecentchangesTest fail.
Bug: T341344
Change-Id: I9c2dc1cf1f184c8062864756d2747ee56e886086
These methods create the test users, meaning they need a database, and
shouldn't be used if the test is not in the database group.
Note that TestUser::assertNotReal() already checks that the user is
not in the real database, but it doesn't warn the test author that
their (potentially non-database) test is using the test database.
Bug: T155147
Depends-On: I211cb60296e5c2446128fcdf2caaadc728a8c272
Change-Id: Ic4a72fbfaee730b8417848ae0603443d4995fefc
