#125: the rest of the global/MVT acceptance tests in conf/common.yaml #140
No reviewers
Labels
No labels
blocked
claude-wip
map-report
map:is
map:se
needs-review
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
art/ismap!140
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "issue-125-global-acceptance-tests"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #125
#125 — Global/MVT acceptance tests: the rest of the config-level checks
Config-only change (
conf/common.yaml+ the R30/R121Check:lines indocs/country-requirements.md): the rest of the global/MVT candidates from#115's plan. No import was needed — every test reads data already live in the
DB, and they are the first tests to exercise the MVT functions themselves.
What ships (5 tests, all 10 s bucket; measured 1–1245 ms live)
The #119 Stockholm z5 test is untouched.
Design notes for the reviewer
IF/ELSIF chain over z (importer/layer.go
mvtFunc): below bottomZ no branchmatches,
resultstays NULL, the function returns'\x'. z=2 is below theglobal min_zoom (3) and therefore below every layer's floor; the stop_z−1
rows additionally exercise the five layers whose floor is a deliberate
choice above the global one. zxy_is is included because it is the other
source function tiles go through (Iceland's own layers, floors 8–12).
f(maxz+1, x, y)re-encodes the maxz table relative to the finer envelope, so it can never
byte-equal
f(maxz, x, y); asserting "serves the maxz table" without adecoder is the asymmetry R30 itself records — below the floor: empty; above
maxz: served. Every row was chosen by measuring the live maxz data (the
comment records each probe tile); a layer clamping to empty above maxz goes
red.
The shared-layer unification left no zxy_se/zxy_pl/zxy_fi (se/pl/fi declare
sources: []) — zxy_se exists only as a legacy stub whose tables are gone.The modern per-country check is the capital tile through the shared zxy_map,
at z5 + z8. zxy_is is not budgeted separately: it measured 101–295 bytes
over Reykjavík (point/line layers) and the shared Reykjavík tile covers it.
ismap_* is exactly what the zxy functions read (verified: 176 tables, 0
unindexed). se_/pl_/fi_* are staging since the unification, and two
staging scratch tables (pl_oikm_probe, pl_ptlz_ingest) are deliberately
unindexed — a blanket prefix sweep would fail on them, which is the reason
#12's cleanup is a prerequisite for it.
Not shipped — R121's decoded-tile half
The issue's MVT attribute-set candidate (R121: a decoded tile's keys match the
contract's
mvt_exclude-adjustedattr_names) is not a SQL test: MVT isprotobuf, Postgres has no decoder, and a SQL reimplementation of the wire
format is thousands of lines for a check that should be cheap. The
stop_zhalf of R121 is now covered by the R30 floor test. The decoded half is
filed as #139 (mapsrv-side Go test; mapsrv's glyph test already decodes
protobuf in Go, and scripts/mvt-decode.py is the hand decoder). The R121
Check: line in docs/country-requirements.md says exactly this.
Verification (live DB, no writes)
(semap's single not-run is the pre-existing R35 at 100 s — needs
-acceptance all, unrelated to this change.)New-test highlights from the run:
Nothing left undone that I know of: #139 carries the decoded-tile half, and
#127 (tile weight) is the standing performance work the budget tests guard.
Closes #125to #125: the rest of the global/MVT acceptance tests in conf/common.yamlReview
Verified:
go test ./...green, and the five new tests pass against the live database on all five configs (conf/ismap.yaml30/30,conf/semap.yaml16 passed + 1 not-run,conf/fimap.yaml16/16,conf/plmap.yaml17/17,conf/world.yaml13/13). The R30 floor test's 19 checks are the right 19 — z2 for every shared layer plus stop_z−1 for exactly the five floored ones (places 3, roads 4, water_points 7, transit 8, stations 8), pluszxy_mapandzxy_is.The all-ocean tile test cannot catch the failure its comment claims
The comment says this check is "what catches a layer serving its floor table to every request at an IN-RANGE zoom (the same #28 failure mode the floor test catches below the floor)". It cannot. In the generated function (
importer/layer.gomvtFunc) the zoom picks the table and the spatial filter is independent:WHERE geom && marginruns whichever branch was taken, so a mid-Atlantic tile returns zero rows no matter which zoom table is read. Reading the wrong zoom table is invisible to this probe.The test is still worth keeping — it is a real smoke check on clipping and projection, and it would catch a function that ignored its bounds or data landing at null island. The recorded rationale just isn't what it discriminates, and since these comments are the durable record of why a check exists, that matters more here than usual.
The floor half (z2, and stop_z−1) genuinely does catch the #28 mode, because there the branch chain is the only thing under test.
Three of the twelve R30 ceiling probes rest on a single feature
The ceiling tiles were measured against current data, and two of them are one feature wide:
The dense Stockholm probes are robust; these are not. Any re-import that re-merges, re-simplifies or shifts that one ferry chain or that one rapids point out of the tile turns the test red for a reason that has nothing to do with R30's property (that maxz+1 serves the maxz table). Picking tiles with a comfortable feature count for those two — or deriving the probe tile from the data at test time — would make the check fail only for the reason it names.
Note
These five tests live in
conf/common.yaml, so they run once per config;make acceptancetherefore renders the same tile-budget probes five times over. Trivial at ~1.2 s each, just worth knowing that the cost scales with the number of countries rather than with the checks.Review addressed (
845f7af)Both points fixed on
issue-125-global-acceptance-tests; the note is acknowledged.1. All-ocean test — comment rewritten to the true rationale
You're right: the zoom branch picks the table,
WHERE geom && marginruns whicheverbranch was taken, so a mid-Atlantic tile returns zero rows no matter which table is
read — the wrong-floor #28 mode is invisible to this probe. The comment now says so
explicitly and states what the check actually is: a smoke test on clipping and
projection that would catch a function ignoring its bounds or geometry landing at
null island. Retitled to match ("a clipping and projection smoke check"). The floor
half's z2 / stop_z−1 rows are still described as the real #28 guards, since there the
branch chain is the only thing under test.
2. R30 ceiling probes — both re-picked with a comfortable feature count
Measured live against the current data, the two one-feature-wide probes are gone:
Both new tiles sit in dense, stable national data (Digiroad ferry links in the
Archipelago Sea; Lantmäteriet Topo 50 rapids at the Dalälven mouth — the z14 table
holds the full source count, no z14 filtering), and the ceiling comment records the
counts so a future re-pick has a reference. A re-import shifting one feature can no
longer turn these rows red.
3. Note — acknowledged
Added a two-line note at the top of common.yaml's config-level block: the whole block
runs once per config, so
make acceptancerenders the MVT probes five times over(~1.2 s each), and the cost scales with the country count rather than the number of
checks.
Verification
make testgreen (importer + mapsrv).conf/ismap.yaml30/30,conf/semap.yaml16 passed + 1 not-run (the pre-existingR35 at 100 s, needs
-acceptance all),conf/plmap.yaml17/17,conf/fimap.yaml16/16,
conf/world.yaml13/13.checks=12, offending=with both re-picked probes non-empty.Re-review
Both findings are fixed, and the fixes are right for the right reasons.
The all-ocean test's rationale. The rewritten comment now states what the check cannot catch (the wrong-floor #28 mode is invisible, because
WHERE geom && marginruns whichever zoom branch was taken) and what it actually is (a clipping and projection smoke check — a function ignoring its bounds, or geometry at null island, goes red). That matchesmvtFuncexactly. The title carries the correction too, and the floor test's z2 / stop_z−1 rows are correctly left as the real #28 guards.The one-feature ceiling probes. Both re-picked tiles are comfortably wide now:
The commit's "13 rapids, 6 ferry links" counts match only when measured through the margin envelope (
ST_TileEnvelope(z,x,y, margin => 256.0/4096)) rather than the plain tile envelope — which is the right way to count, since that is what the generated function actually reads. Nice detail to have got right.Also picked up the note about the block running once per config, which is the useful half of that observation.
Verification
This branch is 7 commits behind
mainand merges cleanly, so I ran everything on the merged result — the state that will actually exist after merge, not the branch in isolation:go test ./...green, R30 ceiling reportschecks=12with both re-picked probes non-empty, and thedocs/country-requirements.mdaddition auto-merges into clean prose alongside what #122–#124 added to the same file.Nothing outstanding from my side.