v1.1: harden recovery and control boundaries
WP-D and WP-E complete the planned v1.1 implementation packages. WP-D — recovery honesty: - backups verify the completed copy with PRAGMA integrity_check - corruption missed by quick_check is detected by the full check - existing good backups remain protected - oversized exports are still delivered but declare whether this version can import them, while the 20 MB import limit remains unchanged - backup was exercised through the real browser UI on both the normal campaign database and a campaign-shaped database over 100 MB WP-E — control-boundary contrast: - interactive control boundaries meet the WCAG 1.4.11 3:1 target - the contrast audit is now a failing gate rather than an advisory - rendered browser measurements pass for the composer, controls, tabs and nav - text contrast and focus visibility remain intact - owner reviewed and approved the before/after screenshots Reports: - planning/reports/v1.1/V1.1-WP-D-REPORT.md - planning/reports/v1.1/V1.1-WP-E-REPORT.md All planned v1.1 work packages A-E are now complete. Release validation has not yet begun.
This commit is contained in:
@@ -0,0 +1,294 @@
|
||||
"""v1.1 WP-D: a backup that was really checked, and an export that says what it is.
|
||||
|
||||
Two recovery-path claims, each of which was true only in the small before this
|
||||
package:
|
||||
|
||||
- **A backup is verified.** M9 ran `PRAGMA quick_check` on the finished copy.
|
||||
That reads every page and every record, and skips the cross-check between a
|
||||
table and its indexes — so a copy whose index disagrees with its table passed.
|
||||
`test_the_fixture_is_the_difference_between_the_two_checks` builds exactly that
|
||||
damage and shows the two pragmas disagreeing about it, before anything here
|
||||
uses it as evidence.
|
||||
- **An export is importable.** Nothing compared the bundle with
|
||||
`limits.MAX_IMPORT_BODY_BYTES`, so a campaign could be exported and then
|
||||
refused by its own importer, with the reader finding out at the moment they
|
||||
needed it. The export still succeeds — the file is complete, and a version
|
||||
that refused to write it would destroy the copy someone was trying to make —
|
||||
and now it says so.
|
||||
|
||||
python -m pytest tests/test_v11_d_recovery.py -v
|
||||
"""
|
||||
|
||||
import json
|
||||
import sqlite3
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from fastapi import Depends
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app import auth, backup, limits, models
|
||||
from app.database import Base, SessionLocal, engine, get_db
|
||||
from app.main import app
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def client():
|
||||
Base.metadata.create_all(bind=engine)
|
||||
setup = SessionLocal()
|
||||
user = models.User(is_guest=False, email="wp-d@example.com")
|
||||
setup.add(user)
|
||||
setup.flush()
|
||||
setup.add(models.Settings(user_id=user.id, model="test-model"))
|
||||
adventure = models.Adventure(user_id=user.id, title="Recovery")
|
||||
setup.add(adventure)
|
||||
setup.flush()
|
||||
setup.add(models.Action(adventure_id=adventure.id, type="start", text="The story opens."))
|
||||
setup.commit()
|
||||
adv_id, user_id = adventure.id, user.id
|
||||
setup.close()
|
||||
app.dependency_overrides[auth.get_current_user] = (
|
||||
lambda db=Depends(get_db): db.get(models.User, user_id))
|
||||
test_client = TestClient(app)
|
||||
test_client.adv_id = adv_id
|
||||
test_client.user_id = user_id
|
||||
try:
|
||||
yield test_client
|
||||
finally:
|
||||
app.dependency_overrides.clear()
|
||||
Base.metadata.drop_all(bind=engine)
|
||||
|
||||
|
||||
# ------------------------------------------------------------ the fixture
|
||||
|
||||
def build_corrupt_copy(path: Path) -> None:
|
||||
"""A database whose index disagrees with its table, and nothing else.
|
||||
|
||||
One digit inside one index leaf page is changed, so that entry names a key
|
||||
no row holds and one row's key is in no index entry. Every page is still
|
||||
structurally sound and every record still parses, which is the whole point:
|
||||
this is the damage `quick_check` is not looking for.
|
||||
"""
|
||||
path.unlink(missing_ok=True)
|
||||
connection = sqlite3.connect(path)
|
||||
connection.execute("PRAGMA page_size=4096")
|
||||
connection.execute("CREATE TABLE t (id INTEGER PRIMARY KEY, k TEXT NOT NULL, filler TEXT)")
|
||||
connection.execute("CREATE INDEX i_t_k ON t(k)")
|
||||
connection.executemany("INSERT INTO t (k, filler) VALUES (?, ?)",
|
||||
[(f"k{n:06d}", "x" * 40) for n in range(400)])
|
||||
connection.commit()
|
||||
page_size = connection.execute("PRAGMA page_size").fetchone()[0]
|
||||
leaves = [row[0] for row in connection.execute(
|
||||
"SELECT pageno FROM dbstat WHERE name='i_t_k' AND pagetype='leaf' ORDER BY pageno")]
|
||||
connection.close()
|
||||
assert leaves, "the index must have a leaf page to damage"
|
||||
|
||||
raw = bytearray(path.read_bytes())
|
||||
start = (leaves[0] - 1) * page_size
|
||||
page = raw[start:start + page_size]
|
||||
at = page.find(b"k000")
|
||||
assert at != -1, "expected an indexed key on the index's first leaf page"
|
||||
page[at + 4] = ord("9") # k000144 -> k000944: a key no row has
|
||||
raw[start:start + page_size] = page
|
||||
path.write_bytes(bytes(raw))
|
||||
|
||||
|
||||
def check(path: Path, pragma: str) -> str:
|
||||
connection = sqlite3.connect(f"file:{path}?mode=ro", uri=True)
|
||||
try:
|
||||
return ", ".join(str(row[0]) for row in connection.execute(f"PRAGMA {pragma}").fetchall())
|
||||
finally:
|
||||
connection.close()
|
||||
|
||||
|
||||
def test_the_fixture_is_the_difference_between_the_two_checks(tmp_path):
|
||||
"""Before using it as evidence: quick_check calls this database fine."""
|
||||
damaged = tmp_path / "damaged.db"
|
||||
build_corrupt_copy(damaged)
|
||||
assert check(damaged, "quick_check") == "ok"
|
||||
integrity = check(damaged, "integrity_check")
|
||||
assert integrity != "ok"
|
||||
assert "i_t_k" in integrity # it names the index that disagrees
|
||||
|
||||
|
||||
# ---------------------------------------------------------------- backups
|
||||
|
||||
def test_a_healthy_backup_passes_the_full_check_and_is_kept(client, tmp_path):
|
||||
source = tmp_path / "campaign.db"
|
||||
source.write_bytes(Path(str(engine.url.database)).read_bytes())
|
||||
result = backup.create(source)
|
||||
assert result.integrity == "ok"
|
||||
assert result.path.exists() and result.bytes > 0
|
||||
assert check(result.path, "integrity_check") == "ok"
|
||||
assert result.path.parent == backup.directory(source)
|
||||
|
||||
|
||||
def test_the_backup_runs_the_full_check_not_the_quick_one(client, tmp_path, monkeypatch):
|
||||
"""The pragma itself, named. SQLite traces every statement it executes, so
|
||||
this reads what the backup actually asked the copy rather than inferring it."""
|
||||
asked: list[str] = []
|
||||
real_connect = sqlite3.connect
|
||||
|
||||
def tracing(*args, **kwargs):
|
||||
connection = real_connect(*args, **kwargs)
|
||||
connection.set_trace_callback(asked.append)
|
||||
return connection
|
||||
|
||||
monkeypatch.setattr(backup.sqlite3, "connect", tracing)
|
||||
source = tmp_path / "campaign.db"
|
||||
source.write_bytes(Path(str(engine.url.database)).read_bytes())
|
||||
backup.create(source).path.unlink()
|
||||
assert any("integrity_check" in sql for sql in asked), asked
|
||||
assert not any("quick_check" in sql for sql in asked), asked
|
||||
|
||||
|
||||
def test_a_copy_the_full_check_rejects_is_not_kept(client, tmp_path, monkeypatch):
|
||||
"""The copy is damaged after it is written and before it is verified, which
|
||||
is where a real page-level fault would appear: between the copy and the
|
||||
rename. Nothing wearing a backup's name may be left behind."""
|
||||
source = tmp_path / "campaign.db"
|
||||
source.write_bytes(Path(str(engine.url.database)).read_bytes())
|
||||
real_copy = backup._copy
|
||||
|
||||
def damage(source_path, working):
|
||||
pages = real_copy(source_path, working)
|
||||
build_corrupt_copy(working)
|
||||
return pages
|
||||
|
||||
monkeypatch.setattr(backup, "_copy", damage)
|
||||
with pytest.raises(backup.BackupError) as refused:
|
||||
backup.create(source)
|
||||
assert "did not verify" in str(refused.value)
|
||||
assert "i_t_k" in str(refused.value) # it says what was wrong
|
||||
kept = list(backup.directory(source).glob("*"))
|
||||
assert kept == [], f"a rejected backup was left behind: {kept}"
|
||||
|
||||
|
||||
def test_a_rejected_backup_leaves_an_earlier_good_one_alone(client, tmp_path, monkeypatch):
|
||||
source = tmp_path / "campaign.db"
|
||||
source.write_bytes(Path(str(engine.url.database)).read_bytes())
|
||||
good = backup.create(source)
|
||||
before = good.path.read_bytes()
|
||||
|
||||
real_copy = backup._copy
|
||||
|
||||
def damage(source_path, working):
|
||||
pages = real_copy(source_path, working)
|
||||
build_corrupt_copy(working)
|
||||
return pages
|
||||
|
||||
monkeypatch.setattr(backup, "_copy", damage)
|
||||
with pytest.raises(backup.BackupError):
|
||||
backup.create(source)
|
||||
assert good.path.exists()
|
||||
assert good.path.read_bytes() == before
|
||||
assert check(good.path, "integrity_check") == "ok"
|
||||
assert [p.name for p in backup.directory(source).glob("*")] == [good.path.name]
|
||||
|
||||
|
||||
def test_the_backup_file_semantics_are_unchanged(client, tmp_path):
|
||||
"""Same directory, same stamped name, same reported fields: WP-D changed the
|
||||
check, not the file."""
|
||||
source = tmp_path / "campaign.db"
|
||||
source.write_bytes(Path(str(engine.url.database)).read_bytes())
|
||||
first = backup.create(source)
|
||||
second = backup.create(source)
|
||||
assert first.path.name.startswith(backup.PREFIX) and first.path.suffix == ".db"
|
||||
assert first.path != second.path, "an existing backup is never overwritten"
|
||||
assert set(first.as_dict()) == {"filename", "bytes", "pages", "seconds", "integrity"}
|
||||
listed = [row["filename"] for row in backup.existing(source)]
|
||||
assert sorted(listed) == sorted([first.path.name, second.path.name])
|
||||
|
||||
|
||||
# ----------------------------------------------------------------- exports
|
||||
|
||||
def export(client, adv_id):
|
||||
response = client.get(f"/api/adventures/{adv_id}/export")
|
||||
assert response.status_code == 200, response.text[:200]
|
||||
return response
|
||||
|
||||
|
||||
def test_a_normal_export_carries_no_warning(client):
|
||||
response = export(client, client.adv_id)
|
||||
assert "X-Export-Warning" not in response.headers
|
||||
assert response.headers["X-Importable-By-This-Version"] == "true"
|
||||
assert int(response.headers["X-Import-Limit-Bytes"]) == limits.MAX_IMPORT_BODY_BYTES
|
||||
assert int(response.headers["X-Export-Bytes"]) == len(response.content)
|
||||
assert response.json()["format"] == "ai-dnd-adventure-v3"
|
||||
|
||||
|
||||
def fill_past_the_limit(adv_id: int) -> int:
|
||||
"""Real rows, until the campaign's bundle is genuinely over the ceiling.
|
||||
|
||||
Not a mocked size: the export below serialises all of it.
|
||||
"""
|
||||
chunk = "The rain kept on over the harbour road, and nobody came. " * 900 # ~50 kB
|
||||
written = 0
|
||||
with SessionLocal() as db:
|
||||
while written < limits.MAX_IMPORT_BODY_BYTES + 2 * 1024 * 1024:
|
||||
db.add_all([models.Action(adventure_id=adv_id, type="ai", text=chunk)
|
||||
for _ in range(40)])
|
||||
db.commit()
|
||||
written += 40 * len(chunk)
|
||||
return written
|
||||
|
||||
|
||||
def test_an_oversized_export_is_still_delivered_and_says_it_cannot_come_back(client):
|
||||
fill_past_the_limit(client.adv_id)
|
||||
response = export(client, client.adv_id)
|
||||
|
||||
# Delivered, whole, and still the same format.
|
||||
body = response.content
|
||||
assert len(body) > limits.MAX_IMPORT_BODY_BYTES
|
||||
parsed = json.loads(body)
|
||||
assert parsed["format"] == "ai-dnd-adventure-v3"
|
||||
assert len(parsed["actions"]) > 40
|
||||
|
||||
# And honest about what this version can do with it.
|
||||
assert response.headers["X-Importable-By-This-Version"] == "false"
|
||||
warning = response.headers["X-Export-Warning"]
|
||||
assert limits.import_limit_label() in warning
|
||||
assert "exported successfully" in warning
|
||||
assert "cannot import" in warning
|
||||
assert int(response.headers["X-Export-Bytes"]) == len(body)
|
||||
|
||||
|
||||
def test_the_warning_follows_the_configured_limit(monkeypatch):
|
||||
"""The text is generated from the constant, so changing the constant changes
|
||||
the sentence rather than leaving a stale number in it."""
|
||||
assert "20 MB" in limits.oversized_export_warning(21_000_000)
|
||||
monkeypatch.setattr(limits, "MAX_IMPORT_BODY_BYTES", 50 * 1024 * 1024)
|
||||
assert limits.import_limit_label() == "50 MB"
|
||||
assert "50 MB" in limits.oversized_export_warning(60_000_000)
|
||||
assert "20 MB" not in limits.oversized_export_warning(60_000_000)
|
||||
|
||||
|
||||
def test_the_bundle_itself_never_carries_the_warning(client):
|
||||
"""The warning is about the export, not part of the portable story file."""
|
||||
fill_past_the_limit(client.adv_id)
|
||||
parsed = json.loads(export(client, client.adv_id).content)
|
||||
flat = json.dumps(parsed).lower()
|
||||
assert "import limit" not in flat
|
||||
assert "cannot import" not in flat
|
||||
for key in parsed:
|
||||
assert "warning" not in key.lower()
|
||||
|
||||
|
||||
def test_that_same_bundle_is_refused_by_import_naming_the_limit(client):
|
||||
fill_past_the_limit(client.adv_id)
|
||||
body = export(client, client.adv_id).content
|
||||
response = client.post("/api/adventures/import", content=body,
|
||||
headers={"Content-Type": "application/json"})
|
||||
assert response.status_code == 413
|
||||
detail = response.json()["detail"]
|
||||
assert "too large" in detail.lower()
|
||||
assert limits.import_limit_label() in detail
|
||||
|
||||
|
||||
def test_a_bundle_under_the_limit_still_imports(client):
|
||||
"""The refusal is about size alone: the ordinary path is untouched."""
|
||||
body = export(client, client.adv_id).content
|
||||
assert len(body) < limits.MAX_IMPORT_BODY_BYTES
|
||||
response = client.post("/api/adventures/import", content=body,
|
||||
headers={"Content-Type": "application/json"})
|
||||
assert response.status_code == 201, response.text[:200]
|
||||
@@ -0,0 +1,152 @@
|
||||
"""v1.1 WP-E: the contrast audit is a gate, not a report.
|
||||
|
||||
Before this package `tools/contrast_audit.py` measured control boundaries,
|
||||
printed that two of them were below 3:1, and exited 0 anyway — on the argument
|
||||
that a control is identified by its label rather than its edge. A check that
|
||||
cannot fail is not a check, and these tests are what make it one: the threshold
|
||||
is exercised from both sides, on a real tokens file, so a future palette change
|
||||
that dims a control's edge stops the run instead of adding a line to it.
|
||||
|
||||
python -m pytest tests/test_v11_e_contrast.py -v
|
||||
"""
|
||||
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "tools"))
|
||||
|
||||
import contrast_audit as audit # noqa: E402
|
||||
|
||||
|
||||
# --------------------------------------------------------------- the maths
|
||||
|
||||
def test_the_ratio_is_the_wcag_ratio():
|
||||
"""Anchored on values with known answers, so a broken formula is visible."""
|
||||
assert audit.ratio("#ffffff", "#000000") == pytest.approx(21.0, abs=0.01)
|
||||
assert audit.ratio("#ffffff", "#ffffff") == pytest.approx(1.0, abs=0.001)
|
||||
# Order does not matter: contrast is symmetric.
|
||||
assert audit.ratio("#131320", "#676792") == pytest.approx(
|
||||
audit.ratio("#676792", "#131320"), abs=1e-9)
|
||||
|
||||
|
||||
# ------------------------------------------------------- the gate itself
|
||||
|
||||
def tokens_file(tmp_path: Path, **overrides: str) -> Path:
|
||||
"""A real tokens file with named colours replaced."""
|
||||
source = audit.TOKENS.read_text()
|
||||
for name, value in overrides.items():
|
||||
token = "--" + name.replace("_", "-")
|
||||
start = source.index(f"{token}: ")
|
||||
end = source.index(";", start)
|
||||
source = source[:start] + f"{token}: {value}" + source[end:]
|
||||
written = tmp_path / "tokens.css"
|
||||
written.write_text(source)
|
||||
return written
|
||||
|
||||
|
||||
def run_against(path: Path, monkeypatch) -> int:
|
||||
monkeypatch.setattr(audit, "TOKENS", path)
|
||||
return audit.main()
|
||||
|
||||
|
||||
def test_a_boundary_below_three_to_one_fails_the_run(tmp_path, monkeypatch, capsys):
|
||||
"""2.99:1 against --bg-input — the wrong side of the line by one hundredth.
|
||||
|
||||
This value clears 3:1 against --bg-panel (3.21:1), so it would have passed
|
||||
the audit as M11 wrote it. It fails now because the floor is taken against
|
||||
the background the control is actually drawn on.
|
||||
"""
|
||||
below = tokens_file(tmp_path, border="#58639a")
|
||||
assert run_against(below, monkeypatch) == 1
|
||||
printed = capsys.readouterr().out
|
||||
assert "2.99:1" in printed
|
||||
assert "FAIL" in printed
|
||||
assert "below 3:1 (WCAG 1.4.11)" in printed
|
||||
|
||||
|
||||
def test_a_boundary_at_three_to_one_passes(tmp_path, monkeypatch, capsys):
|
||||
"""3.00:1 — the right side of the same line, one hundredth from the value
|
||||
above, and not failed for arithmetic the reader cannot see."""
|
||||
at = tokens_file(tmp_path, border="#58639b")
|
||||
assert run_against(at, monkeypatch) == 0
|
||||
printed = capsys.readouterr().out
|
||||
assert "3.00:1" in printed
|
||||
assert "every control boundary clears 3:1" in printed
|
||||
|
||||
|
||||
def test_the_v1_0_0_boundary_would_now_fail(tmp_path, monkeypatch, capsys):
|
||||
"""The value v1.0.0 shipped. This is the defect WP-E closes, and the gate
|
||||
has to be the thing that would have caught it."""
|
||||
shipped = tokens_file(tmp_path, border="#2b2b3d")
|
||||
assert run_against(shipped, monkeypatch) == 1
|
||||
assert "1.24:1" in capsys.readouterr().out # against --bg-input, the worst case
|
||||
|
||||
|
||||
def test_a_text_pair_below_four_point_five_still_fails(tmp_path, monkeypatch, capsys):
|
||||
"""WP-E raised the boundary floor and must not have lowered the text one."""
|
||||
dimmed = tokens_file(tmp_path, text_dim="#5a5750")
|
||||
assert run_against(dimmed, monkeypatch) == 1
|
||||
assert "below WCAG AA (1.4.3)" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_a_missing_token_is_a_failure_not_a_skip(tmp_path, monkeypatch, capsys):
|
||||
source = audit.TOKENS.read_text().replace("--border-bright:", "--border-was-renamed:")
|
||||
written = tmp_path / "tokens.css"
|
||||
written.write_text(source)
|
||||
assert run_against(written, monkeypatch) == 1
|
||||
assert "MISSING TOKEN" in capsys.readouterr().out
|
||||
|
||||
|
||||
# ------------------------------------------------- the shipped palette
|
||||
|
||||
def test_the_real_tokens_pass_both_criteria(capsys):
|
||||
"""The palette as it stands, through the same gate CI would run."""
|
||||
assert audit.main() == 0
|
||||
printed = capsys.readouterr().out
|
||||
assert "every text pair clears WCAG AA (1.4.3)" in printed
|
||||
assert "every control boundary clears 3:1 (1.4.11)" in printed
|
||||
assert "FAIL" not in printed
|
||||
|
||||
|
||||
def test_every_boundary_pair_is_measured_against_the_background_it_is_drawn_on():
|
||||
"""The audit used to check borders only against --bg-panel, which is not
|
||||
where the bordered controls are: inputs and buttons sit on --bg-input
|
||||
(styles/forms.css), which is lighter and therefore harder. Checking only the
|
||||
easier background would let a token pass while the real control failed."""
|
||||
boundary = [(fg, bg) for kind, fg, bg, _, _ in audit.PAIRS if kind == "boundary"]
|
||||
for background in ("--bg-input", "--bg-panel", "--bg"):
|
||||
assert ("--border", background) in boundary, background
|
||||
assert ("--border-bright", "--bg-input") in boundary
|
||||
|
||||
|
||||
def test_the_text_baselines_are_unchanged_by_wp_e():
|
||||
"""WP-E changed only boundary tokens. These are the M11 text measurements,
|
||||
and they have to still be exactly what the earlier reports recorded."""
|
||||
tokens = audit.read_tokens(audit.TOKENS)
|
||||
measured = {
|
||||
"body text on the page": audit.ratio(tokens["--text"], tokens["--bg"]),
|
||||
"body text in a panel": audit.ratio(tokens["--text"], tokens["--bg-panel"]),
|
||||
"secondary text in a panel": audit.ratio(tokens["--text-dim"], tokens["--bg-panel"]),
|
||||
"secondary text on the page": audit.ratio(tokens["--text-dim"], tokens["--bg"]),
|
||||
}
|
||||
assert measured["body text on the page"] == pytest.approx(14.57, abs=0.01)
|
||||
assert measured["body text in a panel"] == pytest.approx(13.57, abs=0.01)
|
||||
assert measured["secondary text in a panel"] == pytest.approx(5.48, abs=0.01)
|
||||
assert measured["secondary text on the page"] == pytest.approx(5.88, abs=0.01)
|
||||
|
||||
|
||||
def test_the_hover_edge_stays_brighter_than_the_resting_edge():
|
||||
"""Rest and hover have to remain distinguishable from each other, not merely
|
||||
each clear the floor against the background."""
|
||||
tokens = audit.read_tokens(audit.TOKENS)
|
||||
panel = tokens["--bg-panel"]
|
||||
assert audit.ratio(tokens["--border-bright"], panel) > audit.ratio(tokens["--border"], panel)
|
||||
|
||||
|
||||
def test_a_boundary_never_becomes_as_loud_as_body_text():
|
||||
"""A control's edge that outshines the words inside it is its own defect."""
|
||||
tokens = audit.read_tokens(audit.TOKENS)
|
||||
panel = tokens["--bg-panel"]
|
||||
assert audit.ratio(tokens["--border-bright"], panel) < audit.ratio(tokens["--text"], panel)
|
||||
Reference in New Issue
Block a user