Commit graph

212 commits

Author SHA1 Message Date
jenkins-bot d056400800 Merge "Simplify ErrorReporter mocks in tests" 2024-08-12 09:17:19 +00:00
jenkins-bot 7955321a2f Merge "Remove unused test setup from ReferenceListFormatterTest" 2024-08-12 09:02:24 +00:00
thiemowmde fb8494b4dd Relax support for error/warning message keys with dashes
This is a direct follow up for I6f05842 where we already started
supporting dashes, but converted them to underscores. The only change
in this patch is that the CSS class will use the message key as it
is, without the dashes being converted to underscores. I added a test
case specifically for this.

Bug: T352676
Change-Id: Ic22e897c27b8371e3b1ed63932b619e7af71bd5c
2024-08-09 12:29:34 +00:00
thiemowmde f3b732bf93 Simplify ErrorReporter mocks in tests
In some tests we want to see the message parameters. But not here.
Simply echoing the message key (thats parameter number one) is
enough.

Change-Id: Id9824cbbe944c84c9fd1932b0863ac1b3f232b75
2024-08-07 18:22:59 +00:00
thiemowmde e078155e94 Remove unused test setup from ReferenceListFormatterTest
I think this was just a mistake in I5457304 when this test was
written. There was never an ->exists() call in the code, as far as
I can see.

This is motivated by our ongoing, probably year-long efforts to
clean this codebase up, see T335129.

Change-Id: I72d89213c5cff06d78ac119b3c79827afbd0b4f5
2024-08-05 16:25:03 +02:00
Umherirrender 3de8f65114 Use namespaced classes
Changes to the use statements done automatically via script

Change-Id: I4b49c4ab87f1fcd5cdf29d5f882dec3c3902e146
2024-06-08 23:31:42 +02:00
jenkins-bot 4f8e06b97b Merge "Make it possible to disable the "cite_error" wrapper message" 2024-05-27 08:16:04 +00:00
C. Scott Ananian fc5f22b32e CiteParserHooksTest: make test compatible with removal of dynamic property
Whether the dynamic property is present or not, it should have a null
value when 'unset' -- and don't use `unset` to delete an *actual*
property when one is present!

Change-Id: Ifcb9492cc5c814d702c6e61e8231abfd8ea0647c
2024-04-12 15:23:14 -04:00
jenkins-bot 1fdc65548b Merge "Add strictly typed RefGroupItem class to Parsoid code" 2024-04-01 03:05:52 +00:00
jenkins-bot 51e569d888 Merge "Allow visualeditor-cite-tool-name-… messages to be missing" 2024-03-28 15:27:14 +00:00
thiemowmde 9e1c4645be Add strictly typed RefGroupItem class to Parsoid code
This does the exact same as the previously used generic stdClass
object, just strictly typed. Turned out to be surprisingly
straightforward, as proven by the small size of this patch.

I'm intentionally not adding anything new in this patch. For
example, the new class is perfect to write longer documentation
for every field. But this is for a later patch.

Change-Id: Ibf696f6b5ef1bfdbe846b571fb7e9ded96693351
2024-03-14 11:59:34 +01:00
thiemowmde 650d2c296d Add basic PHPUnit tests for Parsoid implementation
There is much more to test, but it's a start.

Intentionally build as pure unit tests to make them as fast as
possible.

Bug: T354215
Bug: T358652
Change-Id: Iae1a8086b8f2b9e5b11e0117bd3f19fdaa087df0
2024-03-12 09:06:16 +00:00
thiemowmde 2b32f15c8c Allow visualeditor-cite-tool-name-… messages to be missing
The information read from the …cite-tool-definition.json files is
effectively user input, even if only interface administrators can
edit it. Usually we carefully validate user input. But as of now
this code starts failing with all kinds of uncatched errors.
* An entry with no name, an empty name, or a name that's not a
  string will cause all kinds of undefined behavior.
* An entry with an empty title results in an invisible button.
* A missing message results in a technical <…> placeholder, even if
  the name is usually a sensible fallback.

Note that hard-coding titles as plain text strings in the ….json
file was already possible.

Change-Id: Iddcedbe859e86ac4c3f79a53d36237daff86c0db
2024-03-06 14:21:08 +01:00
jenkins-bot 46499cd9c0 Merge "Rename ReferencesFormatter to ReferenceListFormatter" 2024-02-07 18:00:04 +00:00
thiemowmde 7c75d44b8a Rename ReferencesFormatter to ReferenceListFormatter
I always found the name a little ambiguous. The fact that it outputs
an actual HTML list and not just some "references" – whatever that
means – is relevant, in my opinion.

Change-Id: I0d169455c8d2b42d62da4dccb8376c09fb6902bc
2024-02-07 18:20:02 +01:00
thiemowmde 802b495160 Make it possible to disable the "cite_error" wrapper message
… as well as "cite_warning". Both are extremely trivial and don't
really do anything by default. All they do is to add the prefix
"Cite error:" or "Cite warning:" to all error messages.

This patch will make it possible to disable both messages by
default, i.e. replace their default in en.json with "-" without
breaking anything. That's part of the plan outlined in T353695.
Local on-wiki overrides will continue to work.

Bug: T353695
Change-Id: I374800d0d0b837cd17ed3a1fdde20b70325b06de
2024-02-01 17:36:27 +01:00
thiemowmde 5eea8ebd90 Simplify ErrorReporter PHPUnit test setup
This was slightly overengineered ever since I4b1f890 and slowly became
more and more complicated over time, notably when withConsecutive was
replaced in Icb951b4. Turns out this was never really needed. It's
impossible to get more than one tracking category from this code path.

While we might add more tracking categories later that will most
probably not happen in this code path.

Change-Id: Ie32d17bac8d3518c985b18f83a846c3fb2bd053f
2024-02-01 17:28:44 +01:00
Adam Wight c36d1b90a5 Move extendsCount into parent ref item
Eliminates a complex shared structure in favor of more encapsulation.

Change-Id: I70efabf0ee263ac578472e16dc35047b0601b7ff
2024-01-10 11:55:16 +01:00
Adam Wight 0e01e39061 Encapsulate ref: groupRefs returned as objects
This patch only affects the consumers of groupRefs.

Bug: T353451
Change-Id: I1eff735dbc26dda07aa8ac7af9ea4ddc0906f5a4
2024-01-09 10:22:04 +01:00
Adam Wight f148c65078 Encapsulate ref: pushRef returns an object
This patch affects a few methods which use the output of pushRef.

Bug: T353451
Change-Id: I10b3fe89406c11cdaede92f18a4b96586ecaf5a0
2024-01-09 10:18:57 +01:00
Adam Wight 262fbe24eb Encapsulate ref object: limited to ReferenceStack
This encapsulation gives us field name, type validation and code
documentation.

This patch only affects ReferenceStack and continues to return
approximately the same array outputs to callers.  Some additional
information is included and the placeholder column has a new name.

Bug: T353451
Change-Id: I405fe7ac241f6991fd4c526bfbb58fbc34f2e147
2024-01-09 09:59:16 +01:00
Adam Wight 1434dc5ca6 Switch to a 1-based "count"
The previous patch deprecated the last conditional depending on magic
meanings of 0 and -1, so now we're free to let "count" take on a more
natural meaning: the number of times a footnote mark appears in
article text.

Includes a small hack to avoid changing parser output, by
artificially decrementing the count by one during rendering.  The
hack can be removed and test output updated in a separate patch.

Bug: T353227
Change-Id: I6f76c50357b274ff97321533e52f435798048268
2024-01-08 11:45:36 +01:00
Adam Wight 86edddc8c2 Use semantic field to test ref type
Stop relying on the magic number distinction between "count" = 0 and -1,
by explicitly testing the "name" field instead.

Bug: T353227
Change-Id: I9dce16b01814e19f508d45b927de570049f0e0f5
2024-01-06 16:46:56 +01:00
Adam Wight a6cb979d88 Inline constant for "placeholder" key
Minor refactoring of an internal field, which can be treated like the
other columns.

Change-Id: I255578694c5ab9f2ad3cbe232217af3cea60669c
2024-01-05 11:22:30 +00:00
Adam Wight fd648aec98 Store group in ref items
Encapsulate all information about a ref inside of the internal
structure, rather than relying on the container to be organized by
group.

Bug: T353451
Change-Id: I4c91e8089638b7655bf120402a4a5fcbd1b35452
2024-01-05 11:22:12 +00:00
Adam Wight ca6414320f More explicit test fixtures: key and count
These fields get automatic values during normal operation, but we
should make this explicit in tests which meddle with internals.  This
seems to add some clarity, and helps prepare for encapsulation.

Bug: T353451
Change-Id: I8b012a270f16139671f77ea04645d627b2fba87d
2024-01-05 11:21:58 +00:00
Adam Wight 76e6e870d4 Include more information in missing parent placeholder
This allows the subreferences to be collected together under a heading.

Bug: T353451
Change-Id: Ibf28f0baca14de8140c87b03ad4aa86d2f81a20d
2024-01-05 11:21:12 +00:00
Adam Wight 5a69c54900 Render list-defined parent without a backlink
In this case, there was never a ref with this name in the article so
no backlinks should be rendered.

TODO:
* test case with empty parent backlink and LDR parent

Bug: T353451
Change-Id: I8a7abd05a48ce83da3beb92b15e894d53252bd33
2024-01-05 12:07:22 +01:00
Adam Wight ddf5cb2458 Remove test for a private method
Testing internal methods is brittle.  This code path is already
covered by parser test "Valid follow="…" after it's parent"

Bug: T353451
Change-Id: I3b7a4b9962de1f25a7b57f82d80813219d633594
2024-01-04 17:07:36 +01:00
jenkins-bot ab20cb3cdf Merge "Rename appendText() to resolveFollow()" 2024-01-04 15:29:44 +00:00
Adam Wight 5d1335e279 Explicit parameter names for all test fixtures
This is much more readable.  Patch changes nothing.

Bug: T353451
Change-Id: I72b58881a7329dbe98659553b84e53896ccafc2b
2023-12-21 20:59:25 +01:00
Adam Wight e0e8b2dc33 Drop constant parameters
Some tests didn't vary the key parameter, so converting to a
constant.

Change-Id: I9a42352573cc16fec11a799b878c06751eb03fc8
2023-12-21 20:52:43 +01:00
thiemowmde d73a76dce6 Rename appendText() to resolveFollow()
There is only 1 user left after Icf16965.

Bug: T353266
Change-Id: I4cafdcbe0a23dd7950613a385cb552e7a84e7f26
2023-12-19 14:49:52 +01:00
jenkins-bot 9b87bc717d Merge "Various cleanups to PHPUnit test mock setup" 2023-12-18 12:36:11 +00:00
thiemowmde 742a9ffbf5 Track warnings separately in ReferenceStack
Check out how this gets rid of so many "to do" as well as
"deprecated" comments.

Next qustion: The elements in the stack become more and more
complicated. It's probably worth converting them from arrays into
first-class objects. But this is for another patch.

Bug: T353266
Change-Id: If14acd1070617ca8c4d15be6b1759bd47ead4926
2023-12-15 16:41:04 +01:00
thiemowmde 9304e24551 Various cleanups to PHPUnit test mock setup
For example, use convenient upstream methods, and generally make the
test setup a bit more readable.

Bug: T353227
Change-Id: Ifab71041fcc3f804315793ca7b783f84829c7a0f
2023-12-15 11:45:35 +00:00
thiemowmde 4377f0923d More simple and consistent @covers and @license tags
Same arguments as in Iafa2412. The one reason to use more detailled
per-method @covers annotations is to avoid "accidental coverage"
where code is marked as being covered by tests that don't assert
anything that would be meaningful for this code. This is especially a
problem with older, bigger classes with lots of side effects.

But all the new classes we introduced over the years are small, with
predictable, local effects.

That's also why we keep the more detailled @covers annotations for
the original Cite class.

Bug: T353227
Bug: T353269
Change-Id: I69850f4d740d8ad5a7c2368b9068dc91e47cc797
2023-12-15 12:12:16 +01:00
thiemowmde c794962df7 Use short fn() syntax in tests where it makes sense
We can use this syntax now. It was introduced in PHP 7.4.

Bug: T353269
Change-Id: I5404b33b654efb01171fa2b4ad3925170ffd0e56
2023-12-14 08:05:01 +00:00
thiemowmde 04208b5fd1 Remove PHPDocs that just repeat what the code already says
We removed a bunch of now redundant docs already, see e.g. Ie0692fa.

Change-Id: I55c62d935bdec8bedaebc2666fca3eb17309b0c7
2023-12-13 12:44:41 +01:00
thiemowmde fee8606db6 Avoid a few isset() in favor of more recent syntax
As well as replacing a few `=== null` comparisons with the new ??=
operator.

Bug: T353227
Change-Id: I5b273f452d1e46d37fc28861b54c4e1f19a7a65a
2023-12-12 12:13:42 +00:00
jenkins-bot f455dd3f79 Merge "Re-arrange code in preparation for T298278" 2023-12-05 14:53:42 +00:00
thiemowmde 5f5e9ec9f0 Re-arrange code in preparation for T298278
This patch makes only sense together with I5a64ac4 where it is split
from. See I5a64ac4 for details.

The idea is that this patch just re-arranges the code without making
any changes to how the code behaves. This leaves a minimal change
behind that's much easier to revert, if needed.

Bug: T298278
Change-Id: Ie78313b7f3ac1ec7bce5ac7512e60a3bb011480a
2023-12-04 08:29:53 +01:00
gerritbot 3d34307f87 Update StaticUserOptionsLookup's FQN
User-options related classes are being moved to
the MediaWiki\User\Options namespace in MediaWiki Core;
reflect that change here.

Bug: T352284
Depends-On: I42653491c19dde5de99e0661770e2c81df5d7e84
Change-Id: I22ff2effcf9b7f2162f5d57608d8ec3651b48dd7
2023-11-29 17:55:07 +00:00
Daimona Eaytoy c71109b97b Update tests for PHPUnit 9.6
- Avoid withConsecutive()

Bug: T342110
Change-Id: Icb951b461c5e6ea1ced2ab8a183c53265d4a2b4f
2023-11-22 00:15:20 +01:00
gerritbot 198ae5b21c Replace some moved Title class uses, now MediaWiki\Title\Title
Bug: T321681
Change-Id: Idde5511e075fcec40cad12c7369abec3ee4d096c
2023-08-19 04:13:55 +00:00
jenkins-bot 0fc08a2ac7 Merge "Replace extremely slow parser test with fast unit tests" 2023-07-28 01:05:39 +00:00
thiemowmde 5aa6cb0c7b Replace extremely slow parser test with fast unit tests
This parser test is a bit obscure, in my opinion. We added it in
I8c4de96 to make sure we don't get thousand separators in most
places.

We continued reworking the code since then. By now it's effectively
impossible to "accidentally" get thousand separators. The
problematic methods from the Language class are not even accessible
any more from this code.

To make the tests more robust we now use createNoOpMock (done via
the previous patch) where it matters, specifically for all Language
and Parser mocks. This proves the problematic Language methods are
never called.

Bug: T253743
Bug: T238187
Change-Id: I9bfe1f4decfaf699996da63e19473c2c0d581d9d
2023-07-28 00:32:38 +00:00
jenkins-bot ebd355c067 Merge "Replace all Language and Parser mocks with no-op mocks" 2023-07-27 18:59:53 +00:00
thiemowmde 2aa421a021 Replace all Language and Parser mocks with no-op mocks
Both Language and Parser are extremely complex classes with hundreds
of public methods. We really want to make sure we are not depending
on anything unexpected from these classes. If calls are made into
these classes we want to know exactly what is called.

Doing this also showed that some mocked methods are not even needed.

Change-Id: Icdfff6c07be78a47bf7cadb1813a72581a51272a
2023-07-27 10:00:28 +00:00
thiemowmde e750cc2ed2 Remove unused "HTML message" cite_references_no_link
I believe there is no reason to keep this. The only reason might be
a known wiki that uses this as a feature. But such a wiki doesn't
exist:
https://global-search.toolforge.org/?q=.&regex=1&namespaces=8&title=Cite.*no.*link

Bug: T321217
Change-Id: I128d4383da48f9bdda92f03b212ef3997b3a4802
2023-07-27 11:55:13 +02:00