mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-12 19:59:35 +00:00
ci: cover gap-fill manifest key scans
Signed-off-by: byrongamatos <xasiklas@gmail.com>
This commit is contained in:
@@ -121,12 +121,11 @@ Known, and worth fixing in follow-ups rather than blocking on:
|
|||||||
instead of inferring.
|
instead of inferring.
|
||||||
- **Layer 1 covers top-level keys only.** Nested structure (`arrangements[].file`, `.id`, `.notation`) isn't
|
- **Layer 1 covers top-level keys only.** Nested structure (`arrangements[].file`, `.id`, `.notation`) isn't
|
||||||
checked. Extending to it means walking the schema's `$ref` subschemas.
|
checked. Extending to it means walking the schema's `$ref` subschemas.
|
||||||
- **Layer 1 recognises `get`, `setdefault`, and subscripts** as key access. `update()` and `pop()` aren't
|
- **Layer 1 recognises `get`, `setdefault`, subscripts, and the known gap-fill helper** as key access.
|
||||||
used against a feedpak manifest anywhere in the tree, so they're deliberately not special-cased rather than
|
`update()` and `pop()` aren't used against a feedpak manifest anywhere in the tree, so they're deliberately
|
||||||
speculatively handled. `readers-complete` reuses the same scanner (`keys_touched()`), so this blind spot is
|
not special-cased rather than speculatively handled. `readers-complete` reuses the same scanner
|
||||||
shared, not doubled: a module using only unrecognised access forms would evade both. Keys passed as literal
|
(`keys_touched()`), so this blind spot is shared, not doubled: a module using only unrecognised access forms
|
||||||
*call arguments* to helpers (`_gap_fill_manifest_absent(manifest, "album")` in `lib/routers/song.py`) are
|
would evade both.
|
||||||
likewise unseen.
|
|
||||||
- **Layer 4 can't catch unknown keys**, because `manifest.schema.json` sets `additionalProperties: true` and
|
- **Layer 4 can't catch unknown keys**, because `manifest.schema.json` sets `additionalProperties: true` and
|
||||||
the reference validator deliberately "treats unknown keys/files as forward-compatible". Fixing this
|
the reference validator deliberately "treats unknown keys/files as forward-compatible". Fixing this
|
||||||
properly belongs in the spec (tighten the schema, or give the validator a `--strict` mode). Until then,
|
properly belongs in the spec (tighten the schema, or give the validator a `--strict` mode). Until then,
|
||||||
|
|||||||
@@ -90,6 +90,18 @@ def test_non_literal_keys_are_ignored(tmp_path):
|
|||||||
assert reads == set() and writes == set()
|
assert reads == set() and writes == set()
|
||||||
|
|
||||||
|
|
||||||
|
def test_manifest_key_read_helper_is_seen(tmp_path):
|
||||||
|
# lib/routers/song.py uses this helper for gap-fill proposals. If helpers
|
||||||
|
# are invisible, adding a new literal key through that path bypasses both
|
||||||
|
# key-coverage and readers-complete.
|
||||||
|
reads, writes = _touch(
|
||||||
|
tmp_path,
|
||||||
|
'_gap_fill_manifest_absent(manifest, "album")\n'
|
||||||
|
'_gap_fill_manifest_absent(manifest, dynamic_key)\n',
|
||||||
|
)
|
||||||
|
assert reads == {"album"} and writes == set()
|
||||||
|
|
||||||
|
|
||||||
# ------------------------------------------------------------ exceptions file
|
# ------------------------------------------------------------ exceptions file
|
||||||
|
|
||||||
def test_duplicate_exception_key_is_rejected():
|
def test_duplicate_exception_key_is_rejected():
|
||||||
|
|||||||
@@ -78,6 +78,12 @@ FEEDPAK_SIGNALS = re.compile(r"import sloppak|from sloppak|load_manifest|manifes
|
|||||||
# name list alone silently misses real readers.
|
# name list alone silently misses real readers.
|
||||||
MANIFEST_VARS = {"manifest", "mf"}
|
MANIFEST_VARS = {"manifest", "mf"}
|
||||||
|
|
||||||
|
# Helper functions that take `(manifest, "literal_key", ...)` and read the
|
||||||
|
# manifest for that key. Keep this narrow: only helpers whose first argument is
|
||||||
|
# the manifest dict and whose second argument is a top-level manifest key belong
|
||||||
|
# here.
|
||||||
|
MANIFEST_KEY_READ_HELPERS = {"_gap_fill_manifest_absent"}
|
||||||
|
|
||||||
# Packs committed to this repo, checked against the spec's reference validator.
|
# Packs committed to this repo, checked against the spec's reference validator.
|
||||||
PACK_GLOBS = ["content/starter/*.feedpak", "docs/**/*.sloppak", "docs/**/*.feedpak"]
|
PACK_GLOBS = ["content/starter/*.feedpak", "docs/**/*.sloppak", "docs/**/*.feedpak"]
|
||||||
|
|
||||||
@@ -171,6 +177,16 @@ def keys_touched(path: Path) -> tuple[set[str], set[str]]:
|
|||||||
):
|
):
|
||||||
bucket = writes if node.func.attr == "setdefault" else reads
|
bucket = writes if node.func.attr == "setdefault" else reads
|
||||||
bucket.add(node.args[0].value)
|
bucket.add(node.args[0].value)
|
||||||
|
elif (
|
||||||
|
isinstance(node, ast.Call)
|
||||||
|
and isinstance(node.func, ast.Name)
|
||||||
|
and node.func.id in MANIFEST_KEY_READ_HELPERS
|
||||||
|
and len(node.args) >= 2
|
||||||
|
and _is_manifest_receiver(node.args[0], receivers)
|
||||||
|
and isinstance(node.args[1], ast.Constant)
|
||||||
|
and isinstance(node.args[1].value, str)
|
||||||
|
):
|
||||||
|
reads.add(node.args[1].value)
|
||||||
elif (
|
elif (
|
||||||
isinstance(node, ast.Subscript)
|
isinstance(node, ast.Subscript)
|
||||||
and _is_manifest_receiver(node.value, receivers)
|
and _is_manifest_receiver(node.value, receivers)
|
||||||
|
|||||||
Reference in New Issue
Block a user