Skip to content

Commit 5d07f59

Browse files
author
Automated Bot
committed
Cover the pure helpers with tests; drop two copied functions
compare-feed-urls.py predates feed_probe.py and carried its own copies of validate_feed and dir_sha1, identical line for line apart from a hardcoded timeout. It now imports the module, which also gets it the caching variant of dir_sha1, worth having since each call costs a download. Tests for the three functions most able to be quietly wrong: normalise_url decides every match in every discovery source, and had none. The collapses it makes are pinned, and so are the ones it refuses: a trailing dot must keep two URLs apart, because a real submission differed from the registered URL by exactly that and guessing would have hidden it. calendar_status is where expiry decisions come from. Boundaries are covered, including that a feed expiring today is still active. Writing these corrected a wrong assumption of mine: GTFS YYYYMMDD dates parse fine, because date.fromisoformat accepts ISO 8601 basic format, so the test now pins that rather than asserting it fails. The feed-list extractors are covered because one already failed silently: a catalogue yielding nothing looks exactly like a catalogue that has not changed, which is how MnDOT's hub sat unwatched. Both the URL and json-values paths are pinned, including that URL extraction finds nothing in that shape. policy_for gained a branch when a policy was allowed to name no feeds; a position-only policy must never match by accident. 59 tests, all offline.
1 parent 26fabef commit 5d07f59

4 files changed

Lines changed: 289 additions & 40 deletions

File tree

scripts/compare-feed-urls.py

Lines changed: 5 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -10,54 +10,19 @@
1010
import os
1111
import sys
1212
import json
13-
import subprocess
1413
import concurrent.futures
1514
import urllib.request
1615
import urllib.error
1716
from datetime import date
17+
18+
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
19+
import feed_probe
1820
from rich.console import Console
1921
from rich.table import Table
2022
from rich.text import Text
2123
from rich.panel import Panel
2224

2325

24-
def dir_sha1_of(url: str) -> str | None:
25-
"""Content checksum, independent of how the archive was built."""
26-
try:
27-
proc = subprocess.run(["transitland", "checksum", "--raw-dir-sha1", url],
28-
capture_output=True, text=True, timeout=300)
29-
return proc.stdout.strip() if proc.returncode == 0 else None
30-
except (subprocess.TimeoutExpired, FileNotFoundError):
31-
return None
32-
33-
34-
def validate_feed(url: str) -> dict:
35-
try:
36-
result = subprocess.run(
37-
[
38-
"transitland", "validate",
39-
"-o", "-",
40-
"--include-entities",
41-
"--include-service-levels",
42-
url,
43-
],
44-
capture_output=True,
45-
text=True,
46-
timeout=180,
47-
)
48-
if result.returncode != 0 and not result.stdout.strip():
49-
return {"_error": result.stderr.strip() or "Command failed", "_url": url}
50-
data = json.loads(result.stdout)
51-
data["_url"] = url
52-
return data
53-
except subprocess.TimeoutExpired:
54-
return {"_error": "Timeout after 180s", "_url": url}
55-
except json.JSONDecodeError as e:
56-
return {"_error": f"Invalid JSON: {e}", "_url": url}
57-
except FileNotFoundError:
58-
return {"_error": "'transitland' command not found in PATH", "_url": url}
59-
60-
6126
ROUTE_TYPE_NAMES = {
6227
0: "Tram/Streetcar",
6328
1: "Subway/Metro",
@@ -247,14 +212,14 @@ def main():
247212
console.print("[dim]Validating feeds in parallel (this may take a minute)...[/dim]\n")
248213

249214
with concurrent.futures.ThreadPoolExecutor(max_workers=n * 2) as executor:
250-
val_futs = [executor.submit(validate_feed, url) for url in urls]
215+
val_futs = [executor.submit(feed_probe.validate_feed, url) for url in urls]
251216
results = [f.result() for f in val_futs]
252217
sha1s = [(r.get("details") or {}).get("sha1", "N/A") for r in results]
253218
arc_futs = [executor.submit(lookup_feed_version, sha1, api_key) for sha1 in sha1s]
254219
# The zip SHA1 above is the archive's primary key, and fragile: a server
255220
# that rebuilds the archive per request changes it while the data stays
256221
# put. The directory SHA1 is what actually answers "same feed?".
257-
dir_futs = [executor.submit(dir_sha1_of, url) for url in urls]
222+
dir_futs = [executor.submit(feed_probe.dir_sha1, url) for url in urls]
258223
arc_results = [f.result() for f in arc_futs]
259224
dir_sha1s = [f.result() for f in dir_futs]
260225

scripts/test_atlas_registry.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,59 @@ def test_split_ids_accepts_either_separator():
103103
assert atlas_registry.split_ids("") == []
104104
assert atlas_registry.split_ids(None) == []
105105
assert atlas_registry.split_ids(" ") == []
106+
107+
108+
# --- normalise_url ---------------------------------------------------------
109+
# Every match decision in every discovery source runs through this, so the
110+
# collapses it makes and the ones it refuses are both worth pinning.
111+
112+
@pytest.mark.parametrize("a,b", [
113+
("https://example.com/a.zip", "http://example.com/a.zip"), # scheme
114+
("https://example.com/a.zip", "https://www.example.com/a.zip"), # www.
115+
("https://example.com/a/", "https://example.com/a"), # trailing slash
116+
("https://EXAMPLE.com/A.zip", "https://example.com/a.zip"), # host and path case
117+
(" https://example.com/a.zip ", "https://example.com/a.zip"), # surrounding space
118+
("example.com/a.zip", "https://example.com/a.zip"), # missing scheme
119+
])
120+
def test_normalise_url_collapses_differences_that_do_not_change_the_file(a, b):
121+
assert atlas_registry.normalise_url(a) == atlas_registry.normalise_url(b)
122+
123+
124+
@pytest.mark.parametrize("a,b", [
125+
# A trailing dot is a different path, and a real submission once differed from
126+
# the registered URL by exactly this. Guessing here would merge two URLs that
127+
# a human needs to see separately.
128+
("https://example.com/a.zip.", "https://example.com/a.zip"),
129+
("https://example.com/a.zip?v=1", "https://example.com/a.zip"), # query is significant
130+
("https://example.com/a.zip", "https://example.com/b.zip"),
131+
("https://example.com/a.zip", "https://other.com/a.zip"),
132+
# www. is stripped only as a prefix, never mid-host
133+
("https://www.example.com/a", "https://example.www.com/a"),
134+
])
135+
def test_normalise_url_keeps_differences_that_might_matter(a, b):
136+
assert atlas_registry.normalise_url(a) != atlas_registry.normalise_url(b)
137+
138+
139+
def test_normalise_url_handles_empty_input():
140+
assert atlas_registry.normalise_url("") == ""
141+
assert atlas_registry.normalise_url(None) == ""
142+
143+
144+
# --- normalise_ntd_id ------------------------------------------------------
145+
146+
@pytest.mark.parametrize("raw,expected", [
147+
("1", "00001"),
148+
("90252", "90252"),
149+
(" 123 ", "00123"),
150+
("", ""),
151+
("R-12", "R-12"), # not all digits, so left alone rather than padded
152+
])
153+
def test_normalise_ntd_id(raw, expected):
154+
assert atlas_registry.normalise_ntd_id(raw) == expected
155+
156+
157+
def test_operator_feeds_can_filter_by_spec(feeds_dir):
158+
db = atlas_registry.load(feeds_dir)
159+
assert atlas_registry.operator_feeds(db, "o-one") == {"f-one", "f-one~rt"}
160+
assert atlas_registry.operator_feeds(db, "o-one", spec="gtfs-rt") == {"f-one~rt"}
161+
assert atlas_registry.operator_feeds(db, "o-one", spec="gtfs") == {"f-one"}

scripts/test_feed_probe.py

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
"""Tests for the pure parts of feed_probe.
2+
3+
Everything here runs offline. The functions that shell out to `transitland` or
4+
hit the network are deliberately not covered: their failure modes are timeouts
5+
and a missing binary, both already handled and neither cheap to simulate
6+
usefully.
7+
8+
Run: cd scripts && uv run --with pytest pytest -q
9+
"""
10+
11+
import os
12+
import sys
13+
from datetime import date
14+
15+
import pytest
16+
17+
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
18+
import feed_probe # noqa: E402
19+
20+
TODAY = date(2026, 8, 8)
21+
22+
23+
# --- calendar_status -------------------------------------------------------
24+
# Expiry is the field most registration decisions turn on, and the boundaries
25+
# are where date arithmetic goes wrong.
26+
27+
def test_calendar_active_midway():
28+
status, expired = feed_probe.calendar_status("2026-01-01", "2026-12-31", TODAY)
29+
assert status.startswith("active")
30+
assert expired is None
31+
32+
33+
def test_calendar_expiring_today_is_still_active():
34+
# The last day of service is inclusive; a feed is not expired until the day
35+
# after its latest date.
36+
status, expired = feed_probe.calendar_status("2026-01-01", "2026-08-08", TODAY)
37+
assert status.startswith("active")
38+
assert expired is None
39+
40+
41+
def test_calendar_expired_yesterday_reports_one_day():
42+
status, expired = feed_probe.calendar_status("2026-01-01", "2026-08-07", TODAY)
43+
assert expired == 1
44+
assert status == "expired 1d"
45+
46+
47+
def test_calendar_long_expired_reports_the_distance():
48+
# The distinction the docstring cares about: mid-refresh versus abandoned.
49+
_, expired = feed_probe.calendar_status("2021-09-01", "2024-10-01", TODAY)
50+
assert expired == 676
51+
52+
53+
def test_calendar_starting_in_future():
54+
status, expired = feed_probe.calendar_status("2026-09-01", "2026-12-31", TODAY)
55+
assert status.startswith("future")
56+
assert expired is None
57+
58+
59+
def test_calendar_single_day_span_does_not_divide_by_zero():
60+
status, expired = feed_probe.calendar_status("2026-08-08", "2026-08-08", TODAY)
61+
assert status.startswith("active")
62+
assert expired is None
63+
64+
65+
@pytest.mark.parametrize("earliest,latest", [
66+
("", "2026-12-31"),
67+
("2026-01-01", ""),
68+
("", ""),
69+
(None, None),
70+
])
71+
def test_calendar_missing_dates_are_unknown_not_expired(earliest, latest):
72+
assert feed_probe.calendar_status(earliest, latest, TODAY) == ("unknown", None)
73+
74+
75+
def test_calendar_accepts_gtfs_wire_format_dates():
76+
# GTFS writes dates as YYYYMMDD, which date.fromisoformat parses as ISO 8601
77+
# basic format. Worth pinning: it means dates lifted straight out of a feed
78+
# need no conversion, and a future tightening of the parser would break that
79+
# silently.
80+
status, expired = feed_probe.calendar_status("20260101", "20261231", TODAY)
81+
assert status.startswith("active")
82+
assert expired is None
83+
assert feed_probe.calendar_status("20260101", "20260807", TODAY)[1] == 1
84+
85+
86+
@pytest.mark.parametrize("earliest,latest", [
87+
("2026-13-01", "2026-12-31"), # not a real month
88+
("garbage", "2026-12-31"),
89+
("2026-01-01\r", "2026-12-31"), # stray carriage return, seen in a real feed
90+
])
91+
def test_calendar_unparseable_dates_are_reported_as_such(earliest, latest):
92+
# A feed whose dates cannot be read is a different finding from one with no
93+
# dates at all: a real archive was published with stray carriage returns in
94+
# its date fields, and reporting that as "unknown" would have hidden it.
95+
assert feed_probe.calendar_status(earliest, latest, TODAY) == ("unparseable", None)

scripts/test_scan_feed_sources.py

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
"""Tests for the pure helpers in scan-feed-sources.
2+
3+
Only the offline parts: extraction from a fetched body, and policy matching.
4+
The sources themselves talk to the network and to the registry, and are
5+
exercised by running them.
6+
7+
Imported by path because the script is named with hyphens and so is not a
8+
importable module name.
9+
10+
Run: cd scripts && uv run --with pytest pytest -q
11+
"""
12+
13+
import importlib.util
14+
import os
15+
import pathlib
16+
import sys
17+
18+
import pytest
19+
20+
HERE = pathlib.Path(__file__).resolve().parent
21+
sys.path.insert(0, str(HERE))
22+
23+
_spec = importlib.util.spec_from_file_location("scan_feed_sources", HERE / "scan-feed-sources.py")
24+
scan = importlib.util.module_from_spec(_spec)
25+
_spec.loader.exec_module(scan)
26+
27+
28+
# --- feed-list extraction --------------------------------------------------
29+
# A catalogue that yields nothing looks identical to a catalogue that has not
30+
# changed, which is how a real source sat silently unwatched: MnDOT's hub
31+
# contains no absolute URLs at all, so URL extraction returned an empty set for
32+
# a perfectly healthy page.
33+
34+
def test_urls_extracted_from_html_anchors():
35+
body = '''<a href="https://example.com/gtfs.zip">feed</a>
36+
<a href="https://example.com/about.html">about</a>'''
37+
assert scan._feed_list_urls(body) == ["https://example.com/gtfs.zip"]
38+
39+
40+
def test_urls_extracted_from_plain_text_and_csv():
41+
body = "agency,url\nExample,https://example.com/google_transit.zip\n"
42+
assert scan._feed_list_urls(body) == ["https://example.com/google_transit.zip"]
43+
44+
45+
@pytest.mark.parametrize("url", [
46+
"https://example.com/logo.png",
47+
"https://example.com/app.js",
48+
"https://example.com/style.css",
49+
"https://example.com/timetable.pdf",
50+
])
51+
def test_assets_that_match_the_hint_are_still_excluded(url):
52+
# "download" and "gtfs" appear in plenty of asset paths.
53+
assert scan._feed_list_urls(f'<a href="{url}">x</a>') == []
54+
55+
56+
def test_urls_are_deduplicated_and_sorted():
57+
body = ('https://b.example.com/gtfs.zip https://a.example.com/gtfs.zip '
58+
'https://b.example.com/gtfs.zip')
59+
assert scan._feed_list_urls(body) == [
60+
"https://a.example.com/gtfs.zip", "https://b.example.com/gtfs.zip"]
61+
62+
63+
def test_trailing_punctuation_is_stripped_from_prose_urls():
64+
body = "The feed is at https://example.com/gtfs.zip, updated weekly."
65+
assert scan._feed_list_urls(body) == ["https://example.com/gtfs.zip"]
66+
67+
68+
def test_json_values_extraction_finds_feeds_identified_by_id():
69+
# The MnDOT shape: identifiers, no URLs anywhere.
70+
body = ('[{"feed_id":"browncounty-mn-us","fileNames":["brown_county.zip"]},'
71+
'{"feed_id":"metro-mn-us","fileNames":["metro.zip"]}]')
72+
assert scan._feed_list_json_values(body) == [
73+
"brown_county.zip", "browncounty-mn-us", "metro-mn-us", "metro.zip"]
74+
75+
76+
def test_json_values_walks_nested_structures():
77+
body = '{"a":{"b":["x",{"c":"y"}]},"d":"z"}'
78+
assert scan._feed_list_json_values(body) == ["x", "y", "z"]
79+
80+
81+
def test_json_values_rejects_non_json_loudly():
82+
# Better to report a source as unreadable than to record an empty set and
83+
# report every feed as removed on the next run.
84+
with pytest.raises(ValueError):
85+
scan._feed_list_json_values("<html>not json</html>")
86+
87+
88+
def test_url_extraction_returns_nothing_for_the_json_shape():
89+
# Pins the bug that motivated json-values mode: this page is healthy, and
90+
# URL extraction still finds nothing.
91+
body = '[{"feed_id":"browncounty-mn-us","fileNames":["brown_county.zip"]}]'
92+
assert scan._feed_list_urls(body) == []
93+
94+
95+
# --- policy matching -------------------------------------------------------
96+
97+
def _policies():
98+
return [
99+
{"name": "by-operator", "operators": {"o-one"}, "url_prefixes": ()},
100+
{"name": "by-url", "operators": set(),
101+
"url_prefixes": ("https://api.example.org/transit/",)},
102+
# A policy that states a position rather than a place: it names no feeds
103+
# and no prefixes, so it must never match anything by accident.
104+
{"name": "position-only", "operators": set()},
105+
]
106+
107+
108+
def test_policy_matches_on_operator():
109+
assert scan.policy_for(_policies(), {"o-one"})["name"] == "by-operator"
110+
111+
112+
def test_policy_matches_on_url_prefix():
113+
p = scan.policy_for(_policies(), set(), "https://api.example.org/transit/x/gtfs")
114+
assert p["name"] == "by-url"
115+
116+
117+
def test_policy_url_match_ignores_scheme_and_host_case():
118+
p = scan.policy_for(_policies(), set(), "HTTP://API.Example.ORG/transit/x/gtfs")
119+
assert p["name"] == "by-url"
120+
121+
122+
def test_policy_accepts_a_bare_string_of_urls():
123+
assert scan.policy_for(_policies(), set(), "https://elsewhere.example/x") is None
124+
125+
126+
def test_policy_without_feeds_or_prefixes_never_matches():
127+
# position-only is last, so anything reaching it would return it.
128+
assert scan.policy_for(_policies(), {"o-unknown"}, ()) is None
129+
assert scan.policy_for(_policies(), set(), "https://nothing.example/") is None
130+
131+
132+
def test_policy_returns_none_when_nothing_applies():
133+
assert scan.policy_for([], {"o-one"}, "https://example.com/x") is None

0 commit comments

Comments
 (0)