This is the default anyway when using ->with(). The test code
becomes so much more readable without this, I would like to
argue. Let it just say "with these values".
Because of the way I split my changes into multiple patches
there are a few other changes in this patch I could not split,
e.g. removing unnecessary ->any(). This is the default anyway
and doesn't make the test more specific.
Change-Id: I34990799fa9258ba8dc64c7e78ec43f7903b7681
It is not entirely meaningless. It might be an indicator that
the number of calls to a method is intentionally unlimited.
This is similar to e.g. an @inheritDoc PHPDoc comment that
marks a method as being "intentionally undocumented".
However, what's the meaning of being "intentionally
unconstrained"? Let's just not have any constraint then.
I feel all these ->expects( $this->any() ) bloat the test
code so much that it's never worth it.
Change-Id: I9925e7706bd03e1666f6eb0b284cb42b0dd3be23
It's the same and makes the test code much more readable, I
would like to argue.
Because of the was I split all the changes I made into smaller
patches this patch contains some other changes in the same
lines where I could not split them off. E.g. removal of
->any(), which is the default anyway and doesn't do anything.
Change-Id: Ib297b989d4aec33b31a4e33fe9d5032865b39be0
Ended up using
grep -Prl '\->setMethods\(' . | xargs sed -r -i 's/setMethods\(/onlyMethods\(/g'
special-casing setMethods( null ) -> onlyMethods( [] )
and then manual fix of failing test (from PS2 onwards).
Bug: T278010
Change-Id: I012dca7ae774bb430c1c44d50991ba0b633353f1
One tricky piece left when we get the Request from user
to check whether it was posted, needs a more thorrow
investigation.
Depends-On: Iab96a35be8f50fdbc66194bd8956d98b5b6b0032
Change-Id: I7164b914299441bd0f82e764252c8b5d30b45fbe
Called with "CSPFalsePositiveUrls" instead of
"wgCSPFalsePositiveUrls", so not currently changing the
actual global that matters, and since the test is passing
as-is no need to fix, we can just remove
Change-Id: I1b599bc0f7eff05e3ca68e1a0a326264a192bdd7
ConvertibleTimestamp::setFakeTime is the standard for this kind of
tests. This specific test just caused
f069704e517f749f3fd7de0f6e801c145e6cfab1 to fail the merge-and-submit,
which delayed other merges too.
Change-Id: I3fc92923fee7148f3b3dce610cdb21b1712c9982
Follows-up 5e6cccc3a4.
- Let PHPUnit do the diffing instead of exporting expected value
into the description.
- Use a stricter and complete assertion of the result, not just
one key existence only. Specifically, this means the value (true)
is asserted, and any additional or unexpected properties result in
failure. For example, if all of them were marked as missing+invalid
the old test would pass as it wasn't checking absence. In general,
assertArrayHasKey() is almost always indicates a lax test that can
be improved, or a redundant assertion (such as here, where it was
used to check 'purge' exists before use, but PHP and PHPUnit naturally
validate that already).
Change-Id: Ie7067633e4df0b9a1b451ce4c53a98e8ee3c3ae7
Switch test case with services to use a
service other than ObjectFactory, to make
it easier to mock.
Change-Id: I374a516b6bcc8111b2432245faf935f8287d45a7
Replaces calls directly to PermissionManager with calls to
the Authority object available from Context or the
GroupPermissionLookup service.
This patch does not address use of PermissionManager for
blocks.
Deprecations:
- ApiBase::checkUserRightsAny deprecated passing optional
User parameter
- ApiBase::checkTitleUserPermissions deprecated passing
LinkTarget as first parameter, takes PageIdentity instead
Bug: T271462
Bug: T271854
Change-Id: I5d7cac1c28a37e074750c46cda03283980a07fca
For the optional message in the "Account data" section, introduced in
d4357d621f.
Passes the user name escaped with underscores, so that links can be
built correctly in the message translation.
This means we have to expand yet further the list of User methods called
in ApiOptionsTest, but so be it.
Bug: T272412
Change-Id: Ia1c135144763db9e92f1b555f009229bd1a9d3c8
The timestamp in the year 3030 was presumably chosen so as to
always be in the future (since expiry in the past would be ignored).
However, while seemingly clever, this backfires because it gets
shrunk by the default wgWatchlistExpiryMaxDuration value to some
time in August 2021, around the "current" time of day.
And "current" generally is thought of as moving forward at a rate
of 1s/s, bending of spacetime notwithstanding. Thus the test would
sometimes fail in ways such as:
> 1) ApiQueryInfoTest::testExecute
> Failed asserting that two strings are identical.
> --- Expected
> +++ Actual
> @@ @@
> -'2021-08-13T01:23:19Z'
> +'2021-08-13T01:23:18Z'
Fix this by using a mock current time, paired with a suitable
max duration and expiry value.
Also fix some of the other asserted values to not be reflections
of the source code but actually explicit assertions (at least the
details that we know to be constant, given the environment ensured
by ApiTestCase and MediaWikiLangTestCase). This reduces the chances
of false positives from looking at itself too much, and also speeds
up the test a little.
Change-Id: If361aa98bef789b1f841741a7c83bcb2ac9edf05
This is micro-optimization of closure code to avoid binding the closure
to $this where it is not needed.
Created by I25a17fb22b6b669e817317a0f45051ae9c608208
Change-Id: I0ffc6200f6c6693d78a3151cb8cea7dce7c21653
* Remove notes about being broken, the tests are in better shape now.
* Don't return $data from testGetRollbackToken(), it's not used in
testWatchRollback() as it's being overshadowed.
* Just pass the minimum required values to the dependent test.
Bug: T273603
Change-Id: I7e23fe4769b45cb9ec4e7f3ac8980ccf63e5d492
Required to be overriden by every subclass via direct override
or the indirect static property, which is used only for this
purpose.
The benefit of that is this automatically allow us to remove the
custom Logic exception/property null handling. It also obsoletes
the property itself, which is already a form of redundancy.
Additionaly it removes duplication in classes that are providing
the property and duplicating the base class method code, like
StringDefTest.php
It's better if every child class provide the needed instance via
consistent way and that which is not redundant with another.
Change-Id: Ibe10c2c01b381190cc92e7ee2e1e0888c16e5bf4
We intend to apply de-duplication of errors to StatusValue.
ApiErrorFormatterTest would break because it uses the same message
codes for errors and warnings, expecting them to both be retained.
This is fixed by using different error codes for testing errors and
warnings.
Change-Id: Iaa4811884efa2be076bcfa33911c3dc14c3af5d1
This was disabled 10 years ago because Jenkins was not happy.
We should reenable it and see whether it's happy now, otherwise
the test should be deleted.
Use MediaWikiTestCase::getExistingTestPage() to make sure the page
exists which is another reason given for skipping the test even if
not broken in whole.
Change the test name to more correct 'testPurgePage'
See https://phabricator.wikimedia.org/rMW7a0353b01080
Bug: T273603
Change-Id: Ief2017751256c683c8e4e4df375d5a82bcb4849c
For a decade (since https://phabricator.wikimedia.org/rMWa11cf96d0688)
this test has not been executed. It's safe to assume that it's useless
now and this code is better deleted.
It still fails if re-enabled and it seems to me the reason for its
brokenness is still there, so it's simply not fixable.
Bug: T273603
Change-Id: I6887cb88ae0a2d6665b018246215855498a7ff42
