My personal best practice is to not document @params when there
is a @dataProvider. I mean, these test…() functions are not
meant to be called from anywhere. They do not really need
documentation. @param tags don't do much but duplicate what the
@dataProvider does. This is error-prone, as demonstrated by the
examples in this patch.
This patch also removes @throws tags from tests. A test…() can
never throw an exception. Otherwise the test would fail.
Most of these are found by the not yet released I10559d8.
Change-Id: I3782bca43f875687cd2be972144a7ab6b298454e
* parent::setUp() should be first, and ::tearDown()
should be last
* Move tests that directly extend PHPUnit\Framework\TestCase
to /unit
Change-Id: I1172855c58f4f52a8f624e6d596ec43beb8c93ff
Deconstructing non-sparse, numerically indexed arrays directly in
foreach (a.k.a. using the list() syntax in foreach) is possible since
PHP 5.5.
The possibility to use string array keys as well as non-sequential
numeric keys in array deconstruction was added in PHP 7.1.
Change-Id: I56a48552a45f61cedc291b306cad8548fc70d485
* Remove ILoadBalancer from UserEditCountInitJob constructor,
since it is not a parameter and not a Job property.
It is not needed during Job construction.
* Use ConvertibleTimestamp instead of MWTimestamp since no
MW-specific methods or behaviours are needed here.
* I don't know if UserEditCountInitJob is always safe to de-duplicate,
but this should be decided in the Job class, and is not the
responsibility of calling code that creates/queues a job.
I don't know for sure why this is publicly part of JobSpecification,
but I guess it is there for internal use when serialising jobs,
not for external use in the way it was used here.
I'm assuming for now that its use means it is safe and I moved it to the
UserEditCountInitJob class. If this is not the case and de-dupability is
actually dependent on the parameters, then that logic should reside
in the UserEditCountInitJob constructor.
* Document for which internal use clearUserEditCache() is public.
This way during refactoring the method can be easily found and
made private if that caller is no longer needed.
* Remove needless Job::factory() indirection in the
UserEditCountInitJob test suite. This added overhead that is not
part of the test's purpose, and also risks making the test break
because Job::factory() allows types to be mapped to different
implementations. But, this test suite is meant to cover the
UserEditCountInitJob class implementation specifically.
Change-Id: I6fef4d297b1c0169f95906822e30b4addab7eaf4
The method UserEditTracker::getUserEditCount (as well as the old User class
logic it replaced) calculates the user's edit count and writes it to the
database if it was not computed yet. However, it attempts this write even if
MediaWiki is in read-only mode, causing errors as this method is frequently
called on read requests as well.
As a fix, move the edit count initialization to the job queue, which will avoid
trying to open a source DB connection (and thus cause a read-only error) on
installs that do not use the DB-based job queue. This change requires a
workaround in UserGroupManagerTest.
Bug: T259719
Change-Id: I6d1c8e9038ae1f98f47bdb2495aecc21654b24c0
The name change happened some time ago, and I think its
about time to start using the name name!
(Done with a find and replace)
My personal motivation for doing this is that I have started
trying out vscode as an IDE for mediawiki development, and
right now it doesn't appear to handle php aliases very well
or at all.
Change-Id: I412235d91ae26e4c1c6a62e0dbb7e7cf3c5ed4a6
Done with `composer fix` and suppressing the rest (i.e. sniffs for
global variables, which for core should be suppressed anyway).
Additionally, add `-p` to `phpcbf`, as otherwise it just seems stuck.
Change-Id: Ide8d6cdd083655891b6d654e78440fbda81ab2bc
I'm not sure what the benefit was of having this service being injectible
via the constructor. I mean, I wish *all* services would be injected. But
there are more services used in this class, and no other is injected.
Also it seems the only test that was ever setting the ParserCache service
to something else was not even using it: As far as I can see the test
case testJobSpecRemovesDuplicates() is not triggering the code that uses
the ParserCache service.
Change-Id: I65f5c654d36ffecfd914e97cd8069eb99f76c5c6
The ClearUserWatchlistJob removes all of a user's watchlist items.
This adds the watchlist_expiry table to this, to avoid leaving
any orphan rows there. Any existing orphans would be cleaned up by
the purgeExpiredWatchlistItems.php maintenance script.
Bug: T249782
Change-Id: I06d22618d80cc91a75192a8bb80eb5f4345828ed
This should be the exact same. Its more a style change than anything.
So why do it then?
* I believe this is much less confusing than code mentioning a weird
"standard class". Barely anybody knows what this is, and what the
difference between "object" and "stdClass" is.
* The code is shorter.
* It's even faster. In my micro benchmark it's twice as fast.
Change-Id: I7ee0e8ae6d9264a89b6cd1dd861f0466ae620ccc
After I86d26e494924eec24e7b1fb32c424ac1284be478 the job is
no longer instantiated on submission, only upon execution,
so deduplication flags and dedup info are no longer available
to kafka queue.
Bug: T204761
Depends-On: Ieb2604e65177736606aed351c6658b7df748dcee
Change-Id: Ibf95638a2ad218a83347db6749e2e7c9e8dbe0db
Add public, protected or private to function missing a visibility
Enable the tests folder for the phpcs sniff
Change-Id: Ibefce76ea9984c47e08c94889ea2eafca7565e2c
assertSame() is guaranteed to not do any type conversion. This can be
critical when acciden tially comparing, for example, 0 to 0.0.
Change-Id: Iffcc9bda69573623ba14af655dcd697d0fcce525
* Remove logic for saving slow-to-render parser output. This has
not worked ever since DerivedPageDataUpdater was introduced.
* Make the logic to use cached output actually work. This was
also broken since DerivedPageDataUpdater was added. In order
to pass the output, add a known-revision-output parameter
to both WikiPage::doSecondaryUpdates() and
DerivedPageDataUpdater::prepareUpdate().
* Also factored out some helper methods from runForTitle() in
RefreshLinksJob to make it more readable and avoid the need
for multiple transaction round commit calls. This makes the
case of multiple-title jobs less likely to break again.
* Make use of RefreshLinksJob::runForTitle() return value.
* Add unit tests for multiple-title job case.
Change-Id: I0cd13c424a87653b5a7253c42cd48fe43befd692
This changeset implements T89432 and related tickets and is based on exploration
done at the Prague Hackathon. The goal is to identify tests in MediaWiki core
that can be run without having to install & configure MediaWiki and its dependencies,
and provide a way to execute these tests via the standard phpunit entry point,
allowing for faster development and integration with existing tooling like IDEs.
The initial set of tests that met these criteria were identified using the work Amir did in
I88822667693d9e00ac3d4639c87bc24e5083e5e8. These tests were then moved into a new subdirectory
under phpunit/ and organized into a separate test suite. The environment for this suite
is set up via a PHPUnit bootstrap file without a custom entry point.
You can execute these tests by running:
$ vendor/bin/phpunit -d memory_limit=512M -c tests/phpunit/unit-tests.xml
Bug: T89432
Bug: T87781
Bug: T84948
Change-Id: Iad01033a0548afd4d2a6f2c1ef6fcc9debf72c0d
This reduces confidence in the test. There is no guruantee that
it won't return the same value twice during the duration of a full
PHPUnit run of all test suites, whether twice in a row or 20 minutes
apart.
For a test that needs a string of any kind, use an explicit, consinstent
and cheap literal value.
For a test that specifically needs some kind of uniqueness compared to
something else within the same test case, do so explicitly.
Tests that require something globally unique (for some undefined/vague
definition of "global") were not found, and should not exist anyway.
Also, in libs/objectcache tests, fix order of parameters in some
assertions (expected first, then actual), and use assertFalse/assertSame
instead of assertEqual for cases where false is expected to remove
tolerance of other loosely equal values.
Change-Id: Ifc60e88178da471330b94bfbf12e2731d2efc77d
* Remove duplicate $params check from Job::factory done in Job::__construct.
* In Job::factory(), restore use of a valid title as default for passing as
constructor arg to old job classes. Their constructor may expect it to
be valid.
Keep the invalid dummy in Job::__construct, and document why.
* tests: Update test case for failure mode when using Job::factory
with a class that requires a title. It asserted getting an invalid
title. This now restores the behaviour prior to fc5d51f129,
which is that job classes that require a title, get a valid one.
* tests: Remove test case for testToString that used
an explicitly passed but invalid params value. I've converted
that to expect the exception we now throw instead.
* tests: Update getMockJob(), also used by testToString, which was
relying on undocumented behaviour that 'new Title' is public
and gets namespace=0 and title=''. Before fc5d51f129,
title params weren't in toString() and it asserted outputting
three spaces (delimiter, empty string from formatted title,
delimiter).
In fc5d51f129, this changed to asserting "Special:" which
seems unintentional as we didn't pass it the internally reserved
NS_SPECIAL/'' value, and yet was caught by the dbkey=='' check.
Given this test case doesn't deal with titles, omit it for now.
A job can either have a $title and title/namespace in params,
or neither. This test was asserting an in-memory scenario
where $title can be an object, but title/namespace absent from
params.
Bug: T221368
Depends-On: I89f6ad6967d6f82d87a62c15c0dded901c51b714
Change-Id: I2ec99a12ecc627359a2aae5153d5d7c54156ff46
Simplify the code of jobs that do not care about titles and removes
the direct Title dependency from JobQueue. Remove getTitle() from
IJobSpecification itself. Move all the Job::factory calls into a
single JobQueue::factoryJob() method.
Depends-on: Iee78f4baeca0c0b4d6db073f2fbcc56855114ab0
Change-Id: I9c9d0726d4066bb0aa937665847ad6042ade13ec
Also moved some WikiMap/$wgJobClasses checks to JobQueueGroup::pop
which is the method callers are supposed to use.
Change-Id: I2ab82d8adc4ae1f54697d2935afa2053539cf2db
This already requires a DB domain ID, so there is no reason to have
hacks for trying to handle a wiki ID being passed in instead. If the
provided domain has a schema, it should not simply be ignored in the
comparison.
Change-Id: I9ced7a46fa05f32843a9a7d17391c5d0576b099c
These new classes provide a mechanism for defining the
behavior of slots, like the content models it supports.
This acts as an extension point for extensions that need
to define custom slots, like the MediaInfo extension
for the SDC project.
Bug: T194046
Change-Id: Ia20c98eee819293199e541be75b5521f6413bc2f
Note that we will still be re-parsing either the old or the new
revision. Keeping the rendered version of the old revision cached
for a bit would be nice, but ParserCache currently does not
support this.
Bug: T205369
Change-Id: I86d26e494924eec24e7b1fb32c424ac1284be478
Using domains means thats JobQueueDB has the right value to use for calls
like LoadBalancer::getConnection(). The full domain includes the schema in
the case of Postgres. This makes calls to getConnection() less awkward by
not relying on the fallback logic in reallyOpenConnection() for null schemas.
Make getWikiIdFromDomain/isCurrentWikiDomain account for the schema if it
is both defined and is not simply the generic "mediawiki" schema MediaWiki
uses by default. If all wikis use the default schema, the wiki IDs can get
by with DB/prefix alone, which various config and methods may be built around.
Otherwise, the config callbacks must account for schema and the config must
include it in various wiki domain ID lists to properly disambiguate wikis.
Also, clean up SiteConfiguration::siteFromDB() since it is not meant
to handle schemas unless the callback method was taylored to do so.
Finally, add more comments to DefaultSettings.php about already existing
limitations of wiki domain IDs and their components.
Change-Id: I8d94a650e5c99a19ee50551c5be9544318eb05b1
So far, RefreshLinksJob re-generated the ParserOutput ba calling
Content::getParserOutput. This only works for the main slot. It
needs to instead get the parser output for all slots combiend,
by using RevisionRenderer and RenderedRevision.
Bug: T174035
Change-Id: I253dda26bfa5aefa15f8b1dcc59e69fc7e9d0cb7
The "AUTO" means AUTOCOMMIT, not "automatic transactions"/DBO_TRX,
which is basically the opposite concept. The new name does not
suffer from that ambiguity.
Keep the old constant as an alias for backwards compatibility.
Also remove LoadBalancer comment about non-existing field
Change-Id: I63beeb061fc9be73f320308e4d6393b58628b8c8
