Commit graph

290 commits

Author SHA1 Message Date
Bartosz Dziewoński
e4c7272976 Change uses of getDBLoadBalancerFactory() to getConnectionProvider()
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
2024-01-22 22:27:45 +01:00
Aaron Schulz
129547680b Avoid phpunit failures when using --filter=PageUpdaterTest
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
2024-01-11 18:13:35 -08:00
jenkins-bot
50c37120ee Merge "modify getMutableTestUser() and use it in api query all users test" 2024-01-09 07:12:37 +00:00
Novem Linguae
f34b654631 MediaWikiIntegrationTestCase: add comment
Took some digging to figure this out. State it more
explicitly.

Change-Id: I66d52e54f3f4ee054d94c81510cfc85e1410db38
2024-01-07 00:35:30 -08:00
Ariel T. Glenn
b0c82d3784 modify getMutableTestUser() and use it in api query all users test
Change-Id: I237f0aac25435514711ad529a273a69ea91a5b30
2024-01-05 11:14:30 +02:00
Tim Starling
c23829db2b Improve ChangedTablesTracker domain handling
* 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
2024-01-02 17:23:05 +11:00
jenkins-bot
d972c573d0 Merge "phpunit: Load MW settings in bootstrap.php if running integration tests" 2023-12-21 07:40:16 +00:00
Tim Starling
c55379d5b8 Deprecate and stop using Database::listViews()
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
2023-12-14 14:35:36 +11:00
Daimona Eaytoy
0796b95ee5 phpunit: Load MW settings in bootstrap.php if running integration tests
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
2023-12-13 21:54:19 +00:00
jenkins-bot
e7782b52f3 Merge "tests: Skip expensive resetServices() when nothing changed" 2023-12-11 21:31:28 +00:00
Umherirrender
388b0374fa tests: Use namespaced classes
Changes to the use statements done automatically via script
Addition of missing use statements and changes to docs done manually

Change-Id: Ib326ae1e5c8409a98398c721e8b8ce42c73bd012
2023-12-11 15:59:55 +01:00
thiemowmde
881c39ac3f tests: Skip expensive resetServices() when nothing changed
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
2023-12-11 11:26:19 +00:00
Lucas Werkmeister
74a8d8e10a tests: Only set $dbSetup if setupTestDB() ends without throwing
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
2023-12-08 17:46:29 +01:00
Daimona Eaytoy
71ff052677 Replace MediaWikiIntegrationTestCase::$tablesUsed with automatic query tracking
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
2023-11-21 10:35:59 +11:00
jenkins-bot
b90d704e3a Merge "Introduce and use DynamicPropertyTestHelper" 2023-11-20 03:02:35 +00:00
Reedy
2d68224cc4 phpunit: Minor cleanup of MediaWikiIntegrationTestCase class
* Consistent docs in getTestUser/getMutableTestUser

Change-Id: I209d88c6950c4558cf1bba446227784203171ccb
2023-11-10 15:13:31 +00:00
Máté Szabó
97bb70de85 Introduce and use DynamicPropertyTestHelper
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
2023-11-03 13:50:53 -04:00
thiemowmde
ea2f0b651e Replace generic new Exception with more generic ones
… 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
2023-10-17 07:59:50 +00:00
Amir Sarabadani
f5abfb8d58 Bump codesniffer to 42.0.0
Most noisily, this enables MediaWiki.Arrays.OneSpaceInlineArray.

Change-Id: I8ab11399c67ce7e3ab1b6249b591452774393428
2023-09-27 15:06:32 -04:00
Amir Sarabadani
3735943a95 tests: Migrate away from LoadBalancer::getConnection
It's deprecated.

Bug: T330641
Change-Id: I3cebab9cb4f30e0ee6ddfd5cc16beb994fac5118
2023-09-26 21:05:17 +02:00
James D. Forrester
c1599c91b3 Namespace Config-related classes under \MediaWiki\Config
Bug: T166010
Change-Id: I4066885a7ea071d22497abcdb3f95e73e154d08c
2023-09-21 05:41:58 +00:00
James D. Forrester
1d0b7ae1e2 Namespace User under \MediaWiki\User
Bug: T166010
Change-Id: I7257302b485588af31384d4f7fc8e30551f161f1
2023-09-19 19:18:16 +00:00
Amir Sarabadani
5bd33d46ef Reorg: Move WebRequest to includes\Request
This has been approved as part of RFC T166010

Bug: T321882
Change-Id: I6bbdbbe6ea48cc1f50bc568bb8780fc7c5361a6f
2023-09-11 21:44:34 +01:00
Daimona Eaytoy
03165c038f Make MediaWikiIntegrationTestCase::resetDB static
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
2023-09-05 01:06:06 +00:00
Daimona Eaytoy
f83e611efa Make MediaWikiIntegrationTestCase::addCoreDBData a noop
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
2023-09-05 00:36:36 +00:00
Daimona Eaytoy
df6291bc3a Make more methods static in MediaWikiIntegrationTestCase
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
2023-09-05 00:36:07 +00:00
Daimona Eaytoy
406b844a42 Do not override run() in MediaWikiIntegrationTestCase
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
2023-09-05 00:33:30 +00:00
Daimona Eaytoy
f73bcc2d0d phpunit: Do not setup the test DB for tests that don't need it
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
2023-08-26 00:08:33 +00:00
James D. Forrester
123bbe5dff Follow-Up 77d4c2c: Make getExistingTestPage() check content type first
Bug: T341344
Change-Id: Ia381fb729e1da6af90b8c3165fa072dfe5675b17
2023-08-17 09:17:44 +08:00
Daimona Eaytoy
979d4a7b6b Make MediaWikiIntegrationTestCase::needsDB() final and static
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
2023-08-15 23:43:54 +02:00
Daimona Eaytoy
c1298c9ed8 Make some methods static in MediaWikiIntegrationTestCase
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
2023-08-15 20:53:13 +00:00
Daimona Eaytoy
77d4c2c454 phpunit: Randomize and improve default test page names
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
2023-08-15 20:39:25 +00:00
jenkins-bot
83f1972c29 Merge "tests: Clear recentchanges automatically" 2023-08-12 21:33:31 +00:00
Daimona Eaytoy
501de3902e Throw exception in getTestUser etc. if the test doesn't need the DB
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
2023-08-06 23:41:46 +00:00
Daimona Eaytoy
b03fc0147e phpunit: Throw exception in getDb() for non-database tests
If a test is not in the Database group, it should not need a database
connection.

Change-Id: Iabca65ae545400ecb893e5c1b506cc289da58b03
2023-07-31 00:06:21 +00:00
Daimona Eaytoy
4c72c161d8 Deprecate MediaWikiIntegrationTestCase::$users
This property shouldn't be needed in modern code, where most things can
use Authority. So much so because TestUser adds a dependency on the
database, but many tests that use TestUser don't even need the database.
ApiTestCase, in particular, sets this property in setUp, thus adding a
database dependency to all API tests, including those that don't need
users or the database at all.

Deprecate the property and replace existing usages in core. The one in
ApiTestCase is much harder to migrate, but this patch replaces the array
with an anonymous ArrayAccess class to allow lazy initialization and
remove DB dependencies.

Bug: T155147
Change-Id: I59c4ed1f6a7572d3a92387b15b8e56625bc376a2
2023-07-25 12:12:43 +00:00
Daimona Eaytoy
67a7658e43 phpunit: Make getTestUser and friends non-static
Making these methods static was a mistake. They really shouldn't be
static, because they interact with the database, and should only be
called after initialization.

The goal is to add a needsDB() check to them, but this will be done in
another patch. 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.

Also make them protected, like the other helper methods. If someone
needs to use these outside of MediaWikiIntegrationTestCase, provided
that their use case is sensible, they can use TestUserRegistry directly.

This change should mostly work as long as getTestUser etc. are only
called in non-static methods, regardless of whether callers call them
statically. But if these methods are called in a static method it might
be a bad sign (e.g., if it's a data provider).

For historical reasons, many places already call these methods
non-statically, which is a good thing. A quick codesearch suggests that
around 464 files call them non-statically, and 181 call them statically.

Change-Id: I04dfa8a1a4831767b90bc5a30c4a706069670c9d
2023-07-25 12:12:36 +00:00
Daimona Eaytoy
3bedffa87b phpunit: Do not call addCoreDBData if the test doesn't need the DB
This is a small step towards T155147. For now we clone the database even
if the test doesn't say it needs it, but we stop adding core data.

If a non-database test depends on this data, it means that it really
should be in the database group, or that it's otherwise doing something
wrong. For example, both MediaWikiTest and
RefreshSecondaryDataUpdateTest were depending on the 'UTPage' page being
created by the test framework, although they did that in such a way that
the needsDB() check in getExistingTestPage() weren't triggered.

Bug: T155147
Depends-On: I1a94c6ca2f335ee5a2d7d57df6dc46b65ca1f767
Change-Id: Ia04ff528628a7ae8b350e3122f5a06dbc1ff753b
2023-07-25 11:59:31 +00:00
Lucas Werkmeister
94cca036d6 tests: Clear recentchanges automatically
The recentchanges table, as a summary table of revision and logging,
should be cleared if either of those tables are cleared. Implement this
as a special case – if it was done via $extraTables, then we would clear
logging whenever revision is cleared and vice versa, which isn’t
necessary.

Change-Id: I7b412ac32ae5453a0a1e1716b252e57a5639d5da
2023-07-14 11:54:14 +02:00
Daimona Eaytoy
56dd99570a phpunit: Streamline loading of Setup.php
Add utility methods to TestSetup and call them from the bootstrap file.
Also call the same method to load settings from
MediaWikiIntegrationTestCase when invoked via the unit tests entry
point. This also fixes a couple bug in the deferred setup scenario where
setLoadTestClassesAndNamespaces wasn't called, and the error handler
wasn't reset (T227900#9003435).

Make MediaWikiIntegrationTestCase not try to load settings more than
once. Trying to do so should probably be mostly harmless because of the
require_once usage, but it's also unnecessary and this change avoids any
chance of unwanted side effects.

Also use MW_INSTALL_PATH instead of $IP in bootstrap.php.

Bug: T227900
Change-Id: I7090976435e7e2d1264ee345e2670baaccf532ea
2023-07-13 03:21:45 +02:00
Daimona Eaytoy
97690990e1 phpunit: More improvements for PHPUnit bootstrap files
- Move handling of PHPUNIT_WIKI to the common bootstrap, the values
  might be needed in integration tests.
- Add method to TestSetup for checking composer.lock, and use it in both
  bootstraps. This cannot be moved to the common bootstrap yet, because
  it needs to run after loading MW.
- In MediaWikiIntegrationTestCase::
  initializeForStandardPhpunitEntrypointIfNeeded, use global $IP instead
  of redefining it.

Bug: T227900
Change-Id: I050dcc36b814abd7b909eaaf6d356ecd0d3a0c80
2023-07-13 00:48:31 +02:00
Daimona Eaytoy
3b32e90931 phpunit: Improve PHPUnit bootstrap files
- Remove display_errors override. This was copied from the Maintenance
  class in Id6d7e9db, but it's unnecessary here because we configure
  PHPUnit to fail if a PHP notice/warning is emitted. This causes the
  warning/notice to be converted to a failed assertion handled by
  PHPUnit.
- Remove code that turned off output buffering, also copied from
  Maintenance. I'd say it's unlikely for output buffering to enabled at
  that point, and most importantly, we would enable output buffering a
  bit later anyway.
- Call ob_start earlier, so that it also works for anything that prints
  messages even before dataProviders are run (e.g., setup or
  bootstrap code).
- Move the ob_start call to the common bootstrap, as unit tests are
  still affected by the same issue that the call aimed to fix.
- Print the PHP version immediately, and do that for unit tests too.
- Get rid of MediaWikiCliOptions in favour of private code in
  MediaWikiIntegrationTestCase, which is the only class supposed to
  access those settings.
- Set wgCommandLineMode in the common bootstrap. Assume that nobody is
  changing its value randomly in config files because that'd be
  ill-advised.
- Inline code for setting memory_limit and max_execution_time, and run
  it only after loading the settings. Doing that earlier is pointless,
  because these values are already set in suite.xml.
- Update obsolete comment mentioning PHPUnitMaintClass, which is long
  gone. In the current code, this is the only way (besides PHPUnit
  hooks/events) to run code before PHP exits. Also remove the second
  part because we're now on PHPUnit 9, so this should be actionable now.
  Remove link to the PHPUnit documentation as it no longer works. There
  doesn't seem to be an equivalent link for PHPUnit 9, perhaps because
  hooks were replaced with events. There //is// an equivalent link in
  the PHPUnit 10 documentation (for events), but we're not yet using it.

Change-Id: I79bbbfcfe4eab3ff5ae81b2deb6450ba06ad7611
2023-07-13 00:46:13 +02:00
jenkins-bot
82d9415b2c Merge "tests: Avoid array_key_exists on $GLOBALS in stashMwGlobals" 2023-07-12 22:02:30 +00:00
jenkins-bot
6a09550bbb Merge "Move array destructuring into foreach" 2023-07-10 08:16:38 +00:00
Umherirrender
1b66ab1b7d tests: Move loop-unrelated code in MediaWikiIntegrationTestCase
Just do the check outside of the loop

Change-Id: Iadb0021336f743f745efd9daeffd1c18e295e51b
2023-07-08 22:46:01 +02:00
Umherirrender
28ae6795f8 Move array destructuring into foreach
Change-Id: I54c98085b21f1fe48ccf575d1b9dd60d3b855c58
2023-07-08 19:52:46 +00:00
Umherirrender
93040c84fa tests: Avoid array_key_exists on $GLOBALS in stashMwGlobals
It is slow in php8.1 as $GLOBALS get passed as copy to the function
Similiar change as done in b318366b (T317951)

Change-Id: I43532efa45ab9aea173b2a97bd042e341520268e
2023-07-08 00:40:39 +02:00
Tim Starling
5e665d8cdb Migrate assertSelect() to SelectQueryBuilder
* Add MediaWikiIntegrationTestCase::newSelectQueryBuilder(), which
  creates a subclass of SelectQueryBuilder with assert methods.
* Migrate most callers of assertSelect() to this new query builder
  interface.

Bug: T311866
Change-Id: I7392b37988067020d5f684276320dae0a474631a
2023-06-28 09:03:58 +10:00
jenkins-bot
3aef756418 Merge "Drop revision_comment_temp" 2023-06-09 10:55:21 +00:00
Alexander Vorwerk
b3611755d7 Drop revision_comment_temp
Bug: T299954
Change-Id: I85d21b1eff70a7d70e8ce14f25d66f7e7c76e5fe
2023-06-07 15:34:57 +00:00