Skip to content

Commit c92db1d

Browse files
committed
Add a brief unittest (from Claude) of the template loop detection
1 parent 1f96d0d commit c92db1d

2 files changed

Lines changed: 277 additions & 0 deletions

File tree

tests/__init__.py

Whitespace-only changes.

tests/test_template_loop_guard.py

Lines changed: 277 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,277 @@
1+
"""
2+
Tests for the template self-inclusion loop guard in Extractor.expandTemplate().
3+
4+
Background
5+
----------
6+
wikiextractor bounds template expansion recursion by *depth*
7+
(maxTemplateRecursionLevels), but has no protection against a template
8+
whose own body re-invokes itself. On real wikis this happens in practice
9+
when a template's documentation subpage contains worked usage examples
10+
that literally re-invoke the template with fixed, hardcoded parameters
11+
(see e.g. the Urdu Wikipedia "Redirect-multi" template). Each level of
12+
expansion re-encounters those same examples, branching combinatorially
13+
(e.g. 4 examples per level -> ~4^30 expansions), which never completes in
14+
practice even though it is technically bounded by recursion depth.
15+
16+
The fix in expandTemplate() detects when a template title is about to be
17+
invoked again with the *same* parameters as an active ancestor in
18+
self.frame -- genuine zero-progress recursion -- and stops immediately
19+
instead of re-expanding, mirroring MediaWiki's own "Template loop
20+
detected" behavior.
21+
22+
Crucially, the guard compares (title, params), NOT title alone. Comparing
23+
title alone breaks a common, legitimate MediaWiki idiom: a template that
24+
recurses into itself with *different* (e.g. progressively shrinking)
25+
parameters to process a variable-length list of arguments (templates have
26+
no native loop construct). That recursion makes real progress each step
27+
and terminates on its own -- it must not be blocked. An earlier version
28+
of this fix that compared title alone was verified to turn correct output
29+
into completely empty output for exactly this pattern; that regression
30+
is captured here as test_legitimate_self_recursion_is_not_blocked.
31+
32+
Run with:
33+
python -m unittest tests.test_template_loop_guard -v
34+
or, from the tests/ directory:
35+
python -m unittest test_template_loop_guard -v
36+
"""
37+
38+
import logging
39+
import sys
40+
import time
41+
import unittest
42+
from io import StringIO
43+
44+
sys.path.insert(0, '..') # allow running directly from tests/ without installing
45+
46+
from wikiextractor.extract import Extractor
47+
import wikiextractor.extract as ex
48+
49+
50+
class TemplateLoopGuardTestCase(unittest.TestCase):
51+
"""Base class that resets wikiextractor's module-level template state
52+
before each test. `templates`, `templateCache`, and `redirects` are
53+
plain module globals in extract.py (populated by define_template),
54+
so tests must not leak state into one another.
55+
"""
56+
57+
def setUp(self):
58+
ex.templates.clear()
59+
ex.templateCache.clear()
60+
ex.redirects.clear()
61+
# Normally set by load_templates() when scanning a real dump's
62+
# first Template-namespace page; tests define templates directly
63+
# via define_template(), so it must be set explicitly here.
64+
ex.Extractor.templatePrefix = "Template:"
65+
# Normally set by WikiExtractor.py's main() (Extractor.to_json =
66+
# args.json) before any Extractor.extract() call; tests that call
67+
# .extract() directly need it set too, or it raises AttributeError
68+
# (note: the class also defines an unused `toJson` attribute --
69+
# a pre-existing, unrelated naming mismatch in extract.py).
70+
ex.Extractor.to_json = False
71+
72+
def make_extractor(self, article_text, article_id=1, title="Test Article"):
73+
return Extractor(article_id, str(article_id),
74+
f"https://test.wikipedia.org/wiki?curid={article_id}",
75+
title, [article_text])
76+
77+
78+
class LegitimateSelfRecursionTests(TemplateLoopGuardTestCase):
79+
"""A template recursing into itself with different/shrinking
80+
parameters is a real MediaWiki idiom (there is no native loop
81+
construct) and must fully resolve, not be blocked.
82+
"""
83+
84+
def test_legitimate_self_recursion_is_not_blocked(self):
85+
# Template:P2 formats its first argument, then recurses on the
86+
# rest of the argument list, shifted left by one, terminating
87+
# when no more arguments remain. Each recursive call uses
88+
# DIFFERENT parameters than its caller -- genuine progress.
89+
ex.define_template(
90+
"Template:P2",
91+
["{{{1}}}{{#if:{{{2|}}}|, {{P2|{{{2|}}}|{{{3|}}}|{{{4|}}}|{{{5|}}}}}|}}"]
92+
)
93+
article_text = "Population list: {{P2|CityA|CityB|CityC|CityD}}"
94+
extractor = self.make_extractor(article_text)
95+
96+
cleaned = extractor.clean_text(article_text, expand_templates=True)
97+
98+
self.assertIn("CityA, CityB, CityC, CityD", cleaned[0])
99+
self.assertEqual(extractor.template_loop_errs, 0,
100+
"legitimate varying-parameter recursion must not "
101+
"be flagged as a loop")
102+
103+
def test_deeper_legitimate_recursion_terminates(self):
104+
# A longer list, to make sure the guard doesn't fire partway
105+
# through a longer (but still legitimate, still-progressing)
106+
# recursive chain.
107+
ex.define_template(
108+
"Template:P2",
109+
["{{{1}}}{{#if:{{{2|}}}|, {{P2|{{{2|}}}|{{{3|}}}|{{{4|}}}|{{{5|}}}"
110+
"|{{{6|}}}|{{{7|}}}|{{{8|}}}}}|}}"]
111+
)
112+
article_text = ("{{P2|A|B|C|D|E|F|G|H}}")
113+
extractor = self.make_extractor(article_text)
114+
115+
cleaned = extractor.clean_text(article_text, expand_templates=True)
116+
117+
self.assertIn("A, B, C, D, E, F, G, H", cleaned[0])
118+
self.assertEqual(extractor.template_loop_errs, 0)
119+
120+
121+
class PathologicalSelfReferenceTests(TemplateLoopGuardTestCase):
122+
"""A template whose stored body contains fixed, hardcoded
123+
re-invocations of itself (e.g. worked examples from a documentation
124+
block that wasn't properly stripped) makes zero progress on each
125+
repeat and must be caught quickly rather than left to branch
126+
combinatorially for up to maxTemplateRecursionLevels.
127+
"""
128+
129+
def test_self_referencing_doc_examples_are_caught(self):
130+
# Mirrors the real-world case: Template:Redirect-multi's stored
131+
# body contains several literal re-invocations of itself with
132+
# fixed example parameters (as if from an unstripped /doc
133+
# section). Without the guard this branches ~4x per level for
134+
# up to 30 levels and never completes in practice.
135+
#
136+
# Note: the guard bounds this to a fast, finite amount of work by
137+
# cutting off each distinct (title, params) branch as soon as it
138+
# repeats -- it does not guarantee minimal or "clean" output for
139+
# the pathological template itself (some repeated fragments from
140+
# the doc examples may still appear before each branch is cut
141+
# off). What matters is that it completes quickly and the real
142+
# surrounding article content survives intact.
143+
ex.define_template("Template:Redirect-multi", ["""A redirect hatnote for {{{1}}}.
144+
145+
Usage examples:
146+
{{Redirect-multi|3|A|B|C}}
147+
{{Redirect-multi|3|A|B|C|use=x}}
148+
{{Redirect-multi|3|A|B|C|use=y}}
149+
{{Redirect-multi|3|A|B|C|use=z}}
150+
"""])
151+
article_text = "Some article text.\n\n{{Redirect-multi|2|X|Y}}\n\nMore text."
152+
extractor = self.make_extractor(article_text)
153+
154+
start = time.time()
155+
cleaned = extractor.clean_text(article_text, expand_templates=True)
156+
elapsed = time.time() - start
157+
158+
self.assertGreater(extractor.template_loop_errs, 0,
159+
"self-referencing doc examples should be detected")
160+
# Generous bound: this should complete in well under a second.
161+
# Without the guard, this pattern does not complete in any
162+
# practical amount of time (bounded only by maxTemplateRecursionLevels,
163+
# ~4**30 expansions in the worst case).
164+
self.assertLess(elapsed, 5.0,
165+
"template with self-referencing doc examples took "
166+
"too long -- loop guard may not be firing")
167+
full_text = "\n".join(cleaned)
168+
self.assertIn("Some article text.", full_text)
169+
self.assertIn("More text.", full_text)
170+
171+
def test_direct_immediate_self_reference_is_caught(self):
172+
# The simplest possible case: a template whose body invokes
173+
# itself with the exact same parameters, unconditionally.
174+
ex.define_template("Template:Self", ["before {{Self|x}} after"])
175+
article_text = "{{Self|x}}"
176+
extractor = self.make_extractor(article_text)
177+
178+
start = time.time()
179+
extractor.clean_text(article_text, expand_templates=True)
180+
elapsed = time.time() - start
181+
182+
self.assertGreater(extractor.template_loop_errs, 0)
183+
self.assertLess(elapsed, 5.0)
184+
185+
186+
class WarningLogDeduplicationTests(TemplateLoopGuardTestCase):
187+
"""The per-occurrence warning must be logged once per (article,
188+
title), not once per detected repeat -- a single stuck article can
189+
otherwise generate hundreds of identical log lines.
190+
"""
191+
192+
def test_warning_logged_once_despite_many_repeats(self):
193+
ex.define_template("Template:Redirect-multi", ["""A redirect hatnote for {{{1}}}.
194+
195+
{{Redirect-multi|3|A|B|C}}
196+
{{Redirect-multi|3|A|B|C|use=x}}
197+
{{Redirect-multi|3|A|B|C|use=y}}
198+
{{Redirect-multi|3|A|B|C|use=z}}
199+
"""])
200+
article_text = "{{Redirect-multi|2|X|Y}}"
201+
extractor = self.make_extractor(article_text)
202+
203+
log_stream = StringIO()
204+
handler = logging.StreamHandler(log_stream)
205+
root_logger = logging.getLogger()
206+
original_level = root_logger.level
207+
root_logger.addHandler(handler)
208+
root_logger.setLevel(logging.WARNING)
209+
try:
210+
extractor.clean_text(article_text, expand_templates=True)
211+
finally:
212+
root_logger.removeHandler(handler)
213+
root_logger.setLevel(original_level)
214+
215+
log_output = log_stream.getvalue()
216+
occurrences = log_output.count("Template loop detected")
217+
218+
self.assertGreater(extractor.template_loop_errs, 1,
219+
"test setup should produce multiple detections")
220+
self.assertEqual(occurrences, 1,
221+
"warning should be logged exactly once per "
222+
"(article, title) regardless of total repeat count")
223+
224+
225+
class ErrorSummaryReportingTests(TemplateLoopGuardTestCase):
226+
"""The per-article error summary line (used for the title/recursion
227+
counters) must also include the new loop counter.
228+
"""
229+
230+
def test_loop_errs_included_in_summary_when_present(self):
231+
ex.define_template("Template:Self", ["{{Self|x}}"])
232+
article_text = "{{Self|x}}"
233+
extractor = self.make_extractor(article_text)
234+
235+
log_stream = StringIO()
236+
handler = logging.StreamHandler(log_stream)
237+
root_logger = logging.getLogger()
238+
original_level = root_logger.level
239+
root_logger.addHandler(handler)
240+
root_logger.setLevel(logging.WARNING)
241+
try:
242+
out = StringIO()
243+
extractor.extract(out, html_safe=True)
244+
finally:
245+
root_logger.removeHandler(handler)
246+
root_logger.setLevel(original_level)
247+
248+
log_output = log_stream.getvalue()
249+
self.assertIn("loop(", log_output,
250+
"per-article error summary should report the loop count")
251+
252+
def test_no_spurious_errors_reported_for_clean_article(self):
253+
ex.define_template("Template:Plain", ["just plain text"])
254+
article_text = "{{Plain}}"
255+
extractor = self.make_extractor(article_text)
256+
257+
log_stream = StringIO()
258+
handler = logging.StreamHandler(log_stream)
259+
root_logger = logging.getLogger()
260+
original_level = root_logger.level
261+
root_logger.addHandler(handler)
262+
root_logger.setLevel(logging.WARNING)
263+
try:
264+
out = StringIO()
265+
extractor.extract(out, html_safe=True)
266+
finally:
267+
root_logger.removeHandler(handler)
268+
root_logger.setLevel(original_level)
269+
270+
self.assertEqual(log_stream.getvalue(), "",
271+
"a clean article with no template errors should "
272+
"produce no warnings")
273+
self.assertEqual(extractor.template_loop_errs, 0)
274+
275+
276+
if __name__ == '__main__':
277+
unittest.main()

0 commit comments

Comments
 (0)