Commit graph

38 commits

Author SHA1 Message Date
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 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 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 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 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 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 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
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
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
thiemowmde 08814c8e38 Remove (revert) expensive parsing of 1-character message
This reverts a very tiny part of Ib3fdc89 from 2 weeks ago. The
reasons are explained in Ib3fdc89. Short version:
* The ->parse() calls have drastic performance implications.
* Allowing wikitext and HTML in this message also makes T321217
  worse.

The new message "cite_reference_backlink_symbol" is kept and still
used in the UI. Just not in these two messages any more. This is a
minor redundancy we want to get rid of at some point. But it's not
critical for the moment. This will be done as part of T321217.

Nothing will break on the wikis. Some wikis have customizations for
"cite_references_link_one" and "cite_references_link_many" in place.
This will continue to work as before Ib3fdc89.

Bug: T339973
Change-Id: I933771e3ad67cd530bcf5ee8469cef35ea1070d2
2023-07-21 14:05:31 +02:00
Jon Harald Søby c66371b3d9 Move Cite-specific settings from WikiEditor
The WikiEditor extension has a button and some help text that
is only applicable if the Cite extension is enabled. Move
that (with some modifications) to the Cite extension instead.

Bug: T339973
Depends-On: I8256660f9c6886d6764b45735284e00308fc56e5
Change-Id: Ib3fdc897dd3330f69c5832003d4c3cb1e6dba2f3
2023-06-28 20:22:14 +02:00
thiemowmde 269f726cff Remove inline @var type hints that are not needed
This is mostly because recent IDEs can understand createMock() quite
good. We usually don't add such hints every time we use createMock().
We would have a million of them. ;-)

Change-Id: If9e37807a6945c4408d374fc97664cd636020ffd
2023-06-05 16:37:03 +02:00
Umherirrender 66159e5b78 tests: Make PHPUnit data providers static
Initally used a new sniff with autofix (T333745)

Bug: T332865
Change-Id: Ib86d0fb2d3ea734db46b266faede7b4588fae075
2023-05-20 12:03:41 +02:00
thiemowmde b2be6f4ece Update incomplete ReferencesFormatterTest
The mock was incomplete, leaving a feature uncovered.

Change-Id: I657728f89c94602a7aa9b3fbfabc53e7fdac348c
2023-02-27 17:40:56 +01:00
libraryupgrader 3d79d3518b build: Updating composer dependencies
* mediawiki/mediawiki-codesniffer: 35.0.0 → 36.0.0
* php-parallel-lint/php-parallel-lint: 1.2.0 → 1.3.0

Change-Id: I409d4c564a02f537dbd1ceec1576410aefe0ea64
2021-05-04 01:29:20 +00:00
Thiemo Kreuz 7ce27b432f Stop using Language::formatNum to localize separators
This is done to make the discussion in If3dcfd7 easier.

When we introduced this code we actually used it to format
entire numbers. We had to change this later to *not* localize
digits, but only separators. Language::formatNum is and always
was able to do this, so we just continued to use it.

This is discussed now.

It turns out there is only a single place left where we use
formatNum, and it does nothing but localizing the decimal
point. There is another way to do the same.

Bug: T237467
Change-Id: I89b17a9e11b3afc6c653ba7ccc6ff84c37863b66
2020-09-02 09:40:33 +02:00
Thiemo Kreuz 53b043f28f Remove not needed code without changing anything
This removes a few tiny pieces of code, and a large chunk related to
incomplete follow="…" attributes (see T240858). It turns out we don't
need to insert elements at the top of the ReferenceStack::$refs
array, because this array is reordered anyway in
ReferencesFormatter::formatRefsList()!

Incomplete follow refs don't have a number, and are ordered to the top
because of this, as before. This doesn't change with this patch.

Change-Id: I43036420be22feb8f0f287d9ccee2afd317df2a9
2020-03-31 18:15:14 +02:00
Thiemo Kreuz 400ce89f30 Don't talk about follow being "broken" but "incomplete"
Bug: T240858
Change-Id: Iab6563fdf19d6e85795911e4140476fceabf7334
2020-02-05 16:38:49 +00:00
Adam Wight b15f1b81a0 Revert "Remove "follow" special case from ReferencesFormatter"
This reverts commit 38122d91cd.

Bug: T240858
Change-Id: I7198d5534acded94bc83962262c4cdfed9782454
2020-02-05 11:42:27 +01:00
Adam Wight 38122d91cd Remove "follow" special case from ReferencesFormatter
This is unreachable, now that broken follow refs fail validation.

Bug: T240858
Change-Id: I77faeaac4bc53632ab8b82bff7e335ee8c99dfa5
2020-02-03 12:27:57 +01:00
Thiemo Kreuz 51d55bb8de Introduce dedicated error message for nested <ref extends=…>
This resolves another TODO. Since this is an intentional limitation in
the design of the feature, I find it pretty signigicant to give it it's
own error message.

Note that the text does not need to be perfect, just good enough for now.
We will review all error messages later via T238188.

Bug: T242141
Change-Id: Id9c863061e855350320131e81f6702c8810736f4
2020-01-23 15:00:26 +01:00
jenkins-bot 45119f8c61 Merge "Move "dir" error handling to validation" 2019-12-19 10:18:24 +00:00
Thiemo Kreuz 92607eecfd Add missing @covers tags to tests
We forgot about these when restructuring the code and introducing these
new methods.

Change-Id: If856a7857f2d50d1dc66a57999d98baa8b924d51
2019-12-18 16:46:15 +00:00
Adam Wight 1e82f8f073 Move "dir" error handling to validation
Note that this patch changes behavior, an invalid "dir" will result in
a cite reference at the point where the <ref> is declared rather than
in the references section.  This is consistent with other errors.

Bug: T15673
Change-Id: Id10db40aa0b391f2f1d9274aa09d22a7278d65e3
2019-12-18 10:05:59 +01:00
Thiemo Kreuz e5cda65fbe Remove single use classes from the use section
The name of the base class in tests is guaranteed to only occur a
single time in a file. There is not much value in making it relative,
and requiring it to appear in the use section. Especially because it
is in the root namespace.

This reflects what I once encoded in the sniff
https://github.com/wmde/WikibaseCodeSniffer/blob/master/Wikibase/Sniffs/Namespaces/FullQualifiedClassNameSniff.php
I wish we could pick this rule and use it in our codebases. But it
seems it is to specific and can't be applied on all codebases, hence
it can't become part of the upstream MediaWiki rule set. At least not
at the moment.

Change-Id: I77c2490c565b7a468c5c944301fc684d20206ec4
2019-12-17 14:57:55 +01:00
Thiemo Kreuz 51ff3cc819 Several code cleanups after getting rid of cloning
* Use the Html class to safely create HTML code.
* $this->referenceStack can not be null any more.
* $this->inReferencesGroup is not needed during output, only when
  parsing tags.
* Replace ReferencesStack::getGroupRefs() as well as deleteGroup()
  with a combined popGroup() that does both things.
* Extract the code responsible for the "responsive" behavior to a
  separate function.
* Some TestingAccessWrapper are not needed.

Change-Id: Ie1cf2533d7417ae2f6647664ff1145e37b814a39
2019-12-16 15:47:23 +01:00
Adam Wight ab07a3253c Remove Parser state from CiteErrorReporter
Finishes breaking the circular reference between Cite and Parser.

This patch also demonstrates how evil it is to allow the error reporter
to be called from anywhere, and have side-effects.  At least it's explicit
now.

Also fixes a bug where the inner error message would not be in the
interface language.

Bug: T240431
Change-Id: Ic3325cafb503e78295d72231ac6da5c121402def
2019-12-12 11:15:07 +01:00
Adam Wight 22b2f78db6 Remove Parser state from ReferencesFormatter
Bug: T240431
Change-Id: I3477a64b9a9dba0cfe890c4b598a51c2f971c76c
2019-12-12 11:12:17 +01:00
Thiemo Kreuz 3f2aeb7e31 Rename two Cite… classes and clean up test setups
* All classes are in a Cite\ namespace now. No need to repeat the word
"Cite" all over the place.

* The "key formatter" is more an ID or anchor formatter. The strings it
returns are all used in id="…" attributes, as well as in href="#…" links
to jump to these IDs.

* This patch also removes quite a bunch of callbacks from tests that
don't need to be callbacks.

* I'm also replacing all json_encode().

* To make the test code more readable, I shorten a bunch of variable
names to e.g. $msg. The fact they are mocks is still relevant, and still
visible because these variable names are only used in very short scopes.

Change-Id: I2bd7c731efd815bcdc5d33bccb0c8e280d55bd06
2019-12-12 08:48:02 +01:00
Thiemo Kreuz 75016551e7 Rename formatNumNoSeparators() to localizeDigits()
Because that is what it does. Note our method is different from the one
in the Language class. We only accept strings.

Change-Id: I39107e837cc29f2d7c8867c1e602aa643f9e1a57
2019-12-10 16:21:12 +01:00
Thiemo Kreuz 71c6dc7fe4 Better naming for ReferenceFormatter class and methods
This class renders a <references> tag and everything inside. The
previous name sounds like it is responsible for rendering the contents
of a <ref>…</ref> tag. I mean, the class contains a method that does
exactly this. But this method is private.

Change-Id: I1cd06c9a11e0a74104f2874a34efa3e0843a0f70
2019-12-10 08:40:09 +01:00
Renamed from tests/phpunit/unit/FootnoteBodyFormatterTest.php (Browse further)