TudorandClaude Opus 5
d1358cc00f
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
PR Checks / Backend Smoke (pull_request) Successful in 8s
PR Checks / Build Backend (no push) (pull_request) Successful in 17s
PR Checks / Build Frontend (no push) (pull_request) Successful in 44s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 2m37s
Every place page built its phase links as /schools/[slug]/[phase], the shape that belongs to towns alone. On an authority page that pointed into the town namespace. For 87 of the 151 authorities the target does not exist and the link 404s; for the other 64 it resolves to the town of the same name — a different set of schools, which is precisely the near-duplicate the two namespaces were introduced to prevent. On an outcode page it 404s outright. Two causes behind it, both a rule written twice and inherited by only one of the places that needed it. The authority phase route was in the spec and dropped by the plan, which built the three bare routes and no fourth. The sitemap is generated from the place registry, which was right about them all along, so 302 authority phase URLs have been submitted to Google and every one 404s. Adding the route makes the sitemap true and serves a real query — admissions are authority-run, so "primary schools in Kent" is how a parent searches before they have settled on a town. The outcode variants were the opposite: the registry computed phases for outcodes although the spec gives them no route, and the sitemap knew to skip them while the API did not. The registry now decides alone, and the sitemap's duplicate of that rule is gone. Also: an authority under the five-school threshold has no page, so the API sends a null slug for it and the page names it without linking. Two English authorities are in that position. It was unreachable in today's data — verified across the EC and TR outcodes — but the thin place redirect would have sent a reader to a 404 the year it isn't. The e2e journey now walks every /schools link a page of each family emits and requires a 200, which is the check that was missing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
307 lines
11 KiB
Python
307 lines
11 KiB
Python
"""Tests for sitemap generation (spec 2026-08-20, workstream W1).
|
|
|
|
The sitemap is built from the in-memory school DataFrame, so these inject a
|
|
small frame via monkeypatch rather than touching a database.
|
|
"""
|
|
|
|
import numpy as np
|
|
import pandas as pd
|
|
import pytest
|
|
|
|
|
|
def _schools_df() -> pd.DataFrame:
|
|
"""Two schools: one with results, one with neither results nor Ofsted."""
|
|
base = {
|
|
"local_authority": "Testshire",
|
|
"school_type": "Academy",
|
|
"phase": "Primary",
|
|
"year": 202425,
|
|
"ofsted_date": None,
|
|
}
|
|
return pd.DataFrame(
|
|
[
|
|
{**base, "urn": 100001, "school_name": "Alpha Primary",
|
|
"rwm_expected_pct": 62.0, "attainment_8_score": np.nan,
|
|
"ofsted_grade": 2.0},
|
|
{**base, "urn": 100002, "school_name": "Ghost Primary",
|
|
"rwm_expected_pct": np.nan, "attainment_8_score": np.nan,
|
|
"ofsted_grade": np.nan},
|
|
]
|
|
)
|
|
|
|
|
|
@pytest.fixture()
|
|
def sitemap(monkeypatch) -> str:
|
|
"""The sitemap index."""
|
|
from backend import app as app_module
|
|
|
|
monkeypatch.setattr(app_module, "load_school_data", _schools_df)
|
|
return app_module.build_sitemap()
|
|
|
|
|
|
@pytest.fixture()
|
|
def schools_child(monkeypatch) -> str:
|
|
"""The first school child sitemap, where school URLs actually live."""
|
|
from backend import app as app_module
|
|
|
|
monkeypatch.setattr(app_module, "load_school_data", _schools_df)
|
|
return app_module.build_sitemaps()["schools-1.xml"]
|
|
|
|
|
|
@pytest.fixture()
|
|
def static_child(monkeypatch) -> str:
|
|
from backend import app as app_module
|
|
|
|
monkeypatch.setattr(app_module, "load_school_data", _schools_df)
|
|
return app_module.build_sitemaps()["static.xml"]
|
|
|
|
|
|
def test_every_loc_uses_the_www_host(sitemaps):
|
|
# The apex 301s to www. A <loc> that redirects burns a crawl per URL.
|
|
# Checked across every file, index included, not just one.
|
|
#
|
|
# A child can legitimately be empty — this fixture holds two schools and no
|
|
# town clearing the threshold — so the presence check applies only to files
|
|
# that carry URLs. The absence check applies to all of them.
|
|
for name, xml in sitemaps.items():
|
|
assert "https://schoolcompare.co.uk" not in xml, name
|
|
if "<loc>" in xml:
|
|
assert "https://www.schoolcompare.co.uk" in xml, name
|
|
|
|
|
|
def test_school_with_results_is_listed(schools_child):
|
|
assert "/school/100001-alpha-primary" in schools_child
|
|
|
|
|
|
def test_school_with_no_results_and_no_ofsted_is_omitted(schools_child):
|
|
# Nothing for a search result to say about it. Submitting it spends crawl
|
|
# budget and drags the corpus-wide quality signal down.
|
|
#
|
|
# Asserted against the child, not the index: the index carries no school
|
|
# URLs at all, so it would pass this trivially and prove nothing.
|
|
assert "/school/100002" not in schools_child
|
|
|
|
|
|
def test_no_invented_priority_or_changefreq(sitemaps):
|
|
# Google ignores both. They were noise dressed as signal.
|
|
for name, xml in sitemaps.items():
|
|
assert "<priority>" not in xml, name
|
|
assert "<changefreq>" not in xml, name
|
|
|
|
|
|
def test_ofsted_date_becomes_lastmod(monkeypatch):
|
|
from backend import app as app_module
|
|
import datetime
|
|
|
|
def _df():
|
|
base = _schools_df()
|
|
base.loc[base["urn"] == 100001, "ofsted_date"] = datetime.date(2024, 3, 14)
|
|
return base
|
|
|
|
monkeypatch.setattr(app_module, "load_school_data", _df)
|
|
xml = app_module.build_sitemaps()["schools-1.xml"]
|
|
assert "<lastmod>2024-03-14</lastmod>" in xml
|
|
|
|
|
|
def test_no_lastmod_invented_when_date_unknown(monkeypatch):
|
|
# An always-now lastmod is a claim Google learns to distrust. Absent
|
|
# honestly means unknown.
|
|
from backend import app as app_module
|
|
|
|
def _df():
|
|
df = _schools_df()
|
|
df["ofsted_date"] = None
|
|
return df
|
|
|
|
monkeypatch.setattr(app_module, "load_school_data", _df)
|
|
# The child only. The index legitimately carries a lastmod, because there
|
|
# it means "when this sitemap file changed", which we do know.
|
|
xml = app_module.build_sitemaps()["schools-1.xml"]
|
|
assert "<lastmod>" not in xml
|
|
|
|
|
|
def test_static_routes_are_listed(static_child):
|
|
for path in ("/", "/rankings", "/compare", "/admissions"):
|
|
assert f"<loc>https://www.schoolcompare.co.uk{path}</loc>" in static_child
|
|
|
|
|
|
@pytest.fixture()
|
|
def sitemaps(monkeypatch) -> dict:
|
|
from backend import app as app_module
|
|
|
|
monkeypatch.setattr(app_module, "load_school_data", _schools_df)
|
|
return app_module.build_sitemaps()
|
|
|
|
|
|
def test_index_lists_each_child(sitemaps):
|
|
index = sitemaps["sitemap.xml"]
|
|
assert "<sitemapindex" in index
|
|
assert "https://www.schoolcompare.co.uk/sitemaps/static.xml" in index
|
|
assert "https://www.schoolcompare.co.uk/sitemaps/schools-1.xml" in index
|
|
|
|
|
|
def test_index_carries_no_url_elements(sitemaps):
|
|
# A sitemap index holds <sitemap> entries only; mixing in <url> is invalid.
|
|
assert "<url>" not in sitemaps["sitemap.xml"]
|
|
|
|
|
|
def test_index_does_not_list_itself(sitemaps):
|
|
assert "<loc>https://www.schoolcompare.co.uk/sitemap.xml</loc>" not in sitemaps["sitemap.xml"]
|
|
|
|
|
|
def test_static_child_holds_the_static_routes(sitemaps):
|
|
static = sitemaps["static.xml"]
|
|
for path in ("/", "/rankings", "/compare", "/admissions"):
|
|
assert f"<loc>https://www.schoolcompare.co.uk{path}</loc>" in static
|
|
|
|
|
|
def test_school_child_holds_the_schools(sitemaps):
|
|
assert "/school/100001-alpha-primary" in sitemaps["schools-1.xml"]
|
|
|
|
|
|
def test_children_are_chunked_under_the_limit(monkeypatch):
|
|
# Sitemaps cap at 50,000 URLs per file. Chunk at 10,000 so a child stays
|
|
# small enough to eyeball in Search Console.
|
|
from backend import app as app_module
|
|
import pandas as _pd
|
|
|
|
rows = [
|
|
{"urn": 200000 + i, "school_name": f"School {i}", "year": 202425,
|
|
"rwm_expected_pct": 60.0, "attainment_8_score": None,
|
|
"ofsted_grade": 2.0, "ofsted_date": None}
|
|
for i in range(10_001)
|
|
]
|
|
monkeypatch.setattr(app_module, "load_school_data", lambda: _pd.DataFrame(rows))
|
|
|
|
maps = app_module.build_sitemaps()
|
|
assert maps["schools-1.xml"].count("<url>") == 10_000
|
|
assert maps["schools-2.xml"].count("<url>") == 1
|
|
|
|
|
|
def test_build_sitemap_still_returns_the_index(sitemap):
|
|
# lifespan and the admin endpoint call build_sitemap(); keep it working.
|
|
assert "<sitemapindex" in sitemap
|
|
|
|
|
|
def test_school_with_results_in_an_earlier_year_is_still_listed(monkeypatch):
|
|
"""Regression: The Mallard Academy (150367).
|
|
|
|
Real KS2 results 2015-16 to 2018-19, then null rows from 2022-23 onward
|
|
because the school stopped reporting. The first cut tested the latest
|
|
year's row alone and dropped it, along with ~220 others, even though its
|
|
detail page shows all four years of results.
|
|
"""
|
|
from backend import app as app_module
|
|
import pandas as _pd
|
|
|
|
base = {"local_authority": "Testshire", "school_type": "Academy",
|
|
"phase": "Primary", "ofsted_date": None, "ofsted_grade": np.nan,
|
|
"attainment_8_score": np.nan, "urn": 150367,
|
|
"school_name": "Mallard Academy"}
|
|
df = _pd.DataFrame([
|
|
{**base, "year": 201819, "rwm_expected_pct": 67.0},
|
|
{**base, "year": 202324, "rwm_expected_pct": np.nan},
|
|
{**base, "year": 202425, "rwm_expected_pct": np.nan},
|
|
])
|
|
monkeypatch.setattr(app_module, "load_school_data", lambda: df)
|
|
|
|
xml = app_module.build_sitemaps()["schools-1.xml"]
|
|
assert "/school/150367-mallard-academy" in xml
|
|
|
|
|
|
def test_school_with_no_results_in_any_year_is_still_omitted(monkeypatch):
|
|
"""The fix must not turn into "list everything"."""
|
|
from backend import app as app_module
|
|
import pandas as _pd
|
|
|
|
base = {"local_authority": "Testshire", "school_type": "Academy",
|
|
"phase": "Primary", "ofsted_date": None, "ofsted_grade": np.nan,
|
|
"attainment_8_score": np.nan, "rwm_expected_pct": np.nan,
|
|
"urn": 100002, "school_name": "Ghost Primary"}
|
|
df = _pd.DataFrame([{**base, "year": y} for y in (202324, 202425)])
|
|
monkeypatch.setattr(app_module, "load_school_data", lambda: df)
|
|
|
|
assert "/school/100002" not in app_module.build_sitemaps()["schools-1.xml"]
|
|
|
|
|
|
def _places_df() -> pd.DataFrame:
|
|
base = {
|
|
"local_authority": "Essex", "school_type": "Academy",
|
|
"phase": "Primary", "year": 202425, "ofsted_grade": 2.0,
|
|
"ofsted_date": None, "attainment_8_score": np.nan,
|
|
"town": "Brentwood", "postcode": "CM13 1AA",
|
|
}
|
|
return pd.DataFrame([
|
|
{**base, "urn": 100000 + i, "school_name": f"Brentwood School {i}",
|
|
"rwm_expected_pct": 60.0}
|
|
for i in range(6)
|
|
])
|
|
|
|
|
|
@pytest.fixture()
|
|
def place_sitemaps(monkeypatch) -> dict:
|
|
from backend import app as app_module
|
|
|
|
monkeypatch.setattr(app_module, "load_school_data", _places_df)
|
|
monkeypatch.setattr(app_module, "_place_registry", None)
|
|
return app_module.build_sitemaps()
|
|
|
|
|
|
def test_place_children_are_listed_in_the_index(place_sitemaps):
|
|
index = place_sitemaps["sitemap.xml"]
|
|
assert "/sitemaps/places-1.xml" in index
|
|
assert "/sitemaps/outcodes-1.xml" in index
|
|
|
|
|
|
def test_town_and_authority_urls_use_their_own_namespaces(place_sitemaps):
|
|
xml = place_sitemaps["places-1.xml"]
|
|
assert "<loc>https://www.schoolcompare.co.uk/schools/brentwood</loc>" in xml
|
|
assert "<loc>https://www.schoolcompare.co.uk/schools/authority/essex</loc>" in xml
|
|
|
|
|
|
def test_outcode_urls_live_in_their_own_child(place_sitemaps):
|
|
assert "/schools/near/cm13" in place_sitemaps["outcodes-1.xml"]
|
|
assert "/schools/near/cm13" not in place_sitemaps["places-1.xml"]
|
|
|
|
|
|
def test_place_urls_carry_no_priority_or_changefreq(place_sitemaps):
|
|
for name in ("places-1.xml", "outcodes-1.xml"):
|
|
assert "<priority>" not in place_sitemaps[name]
|
|
assert "<changefreq>" not in place_sitemaps[name]
|
|
|
|
|
|
def test_phase_variants_are_submitted_where_the_phase_clears_the_threshold(place_sitemaps):
|
|
# "primary schools in beccles" is the query shape the baseline showed, so
|
|
# each variant is its own page and has to be submitted. Emitting only the
|
|
# bare place URL left ~950 of them reachable by nothing.
|
|
xml = place_sitemaps["places-1.xml"]
|
|
assert "<loc>https://www.schoolcompare.co.uk/schools/brentwood/primary</loc>" in xml
|
|
|
|
|
|
def test_a_phase_below_its_own_threshold_is_not_submitted(place_sitemaps):
|
|
# The fixture is six primaries and no secondaries.
|
|
xml = place_sitemaps["places-1.xml"]
|
|
assert "/schools/brentwood/secondary" not in xml
|
|
|
|
|
|
def test_outcodes_get_no_phase_variants(place_sitemaps):
|
|
# Nobody searches "primary schools in CM13"; the routes do not exist.
|
|
xml = place_sitemaps["outcodes-1.xml"]
|
|
assert "/primary" not in xml and "/secondary" not in xml
|
|
|
|
|
|
def test_authority_phase_variants_are_submitted_in_their_own_namespace(place_sitemaps):
|
|
"""302 of these were already in the sitemap, and every one 404'd.
|
|
|
|
The spec gives authorities a phase route; the plan built the bare
|
|
authority route and dropped it. Nothing noticed because the sitemap was
|
|
written from the registry, which was right, while the routes were written
|
|
by hand. This test fails if the URL ever leaves the sitemap; the e2e
|
|
journey fails if the route ever leaves the app.
|
|
"""
|
|
xml = place_sitemaps["places-1.xml"]
|
|
assert ("<loc>https://www.schoolcompare.co.uk"
|
|
"/schools/authority/essex/primary</loc>") in xml
|
|
# And never in the town namespace, which is a different set of schools.
|
|
assert "/schools/essex/primary" not in xml
|