mirror of
https://github.com/kuhyx/utils.git
synced 2026-11-10 20:07:18 +01:00
Don't judge allowlist rot from a file-scoped run; cut two library versions
Two things, because the first is what blocked the second from committing. **The rot check was scope-blind.** pre-commit passes only the staged manifests, so a run that sees no package.json cannot learn anything about whether the fleet-wide typescript hold is still needed -- yet it concluded the entry was dead and exited 2. Staging two pubspec.yaml files in utils was enough to make the repo uncommittable. The dead-entry check now runs only under --all, which is CI and the manual whole-repo run; a subset scan has seen a subset by construction. **crdt_sync_dart 0.11.1 and design_system 0.2.1.** Both had dependency upgrades land after their newest tag, so a consumer moving to "latest tag" would pick up the pre-upgrade constraints (crdt_sync_dart-v0.11.0 still carries `crypto: ^3.0.7`, `http: ^1.6.0`, `lints: ^5.0.0`) while carrying its own new exact pins -- the unsatisfiable-resolution case exact pins create. Version bump only; the dependency work is already in922fb36and704c67e. The allowlist end-to-end tests moved to their own file for the 250-line cap, and `run`/`canned` moved to conftest so both files share one definition. `canned` is no longer autouse: the resolver tests patch `_fetch` themselves. 293 tests, 100% branch coverage on every production module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRykabMwPgDzFHcARoqpaK
This commit is contained in:
parent
1439c0f5c0
commit
978d32af3a
@ -2,7 +2,7 @@ name: crdt_sync
|
||||
description: >-
|
||||
Shared CRDT merge scheme + GitHub-Contents-API sync transport for
|
||||
cross-device apps. Pure Dart, no Flutter dependency.
|
||||
version: 0.11.0
|
||||
version: 0.11.1
|
||||
publish_to: 'none'
|
||||
|
||||
environment:
|
||||
|
||||
@ -44,7 +44,8 @@ def repo_root(start: Path) -> Path:
|
||||
|
||||
|
||||
def _excuse(
|
||||
findings: list[Finding], entries, shared: Path | None = None
|
||||
findings: list[Finding], entries, shared: Path | None = None,
|
||||
whole_repo: bool = True,
|
||||
) -> tuple[list[Finding], list]:
|
||||
"""Split findings into (still failing, excused), and flag dead entries.
|
||||
|
||||
@ -56,6 +57,14 @@ def _excuse(
|
||||
repo that owns the shared file. A fleet-wide hold on `typescript`
|
||||
legitimately excuses nothing in a Flutter app, and failing there would
|
||||
make every repo without that dependency uncommittable.
|
||||
|
||||
`whole_repo` is False for a file-scoped run -- what pre-commit does, since
|
||||
it passes only the staged manifests. Such a run has seen a SUBSET of the
|
||||
repo by construction, so "no finding mentioned this package" carries no
|
||||
information about whether the entry is still needed. Judging rot there
|
||||
fails the commit over a manifest the commit never touched: staging two
|
||||
pubspec.yaml files in utils reported the fleet-wide typescript hold as
|
||||
dead, because no package.json was in scope to be found stale.
|
||||
"""
|
||||
by_label = {f"{e.ecosystem}:{e.package}": e for e in entries}
|
||||
used: set[str] = set()
|
||||
@ -73,7 +82,8 @@ def _excuse(
|
||||
))
|
||||
dead = [
|
||||
e for e in entries
|
||||
if f"{e.ecosystem}:{e.package}" not in used
|
||||
if whole_repo
|
||||
and f"{e.ecosystem}:{e.package}" not in used
|
||||
and (shared is None or e.source != shared)
|
||||
]
|
||||
return failing, dead
|
||||
@ -162,7 +172,9 @@ def main(argv: list[str] | None = None) -> int:
|
||||
findings = collect(_targets(args, root), resolver)
|
||||
shared = shared_path()
|
||||
failing, dead = _excuse(
|
||||
findings, entries, shared=None if root == shared.parent else shared
|
||||
findings, entries,
|
||||
shared=None if root == shared.parent else shared,
|
||||
whole_repo=args.all,
|
||||
)
|
||||
|
||||
still_blocking = {f.label: True for f in findings}
|
||||
|
||||
@ -85,3 +85,49 @@ def write(root: Path, relative: str, body: str) -> Path:
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text(body, encoding="utf-8")
|
||||
return path
|
||||
|
||||
|
||||
STALE_PUBSPEC = """\
|
||||
name: demo
|
||||
environment:
|
||||
sdk: ^3.12.2
|
||||
dependencies:
|
||||
http: 1.5.0
|
||||
"""
|
||||
CURRENT_PUBSPEC = STALE_PUBSPEC.replace("1.5.0", "1.6.0")
|
||||
ALLOWLIST = "dependency-freshness.allowlist.yaml"
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def canned(monkeypatch, cache_dir):
|
||||
"""One registry answer for every package the end-to-end fixtures declare.
|
||||
|
||||
Deliberately NOT autouse: the resolver tests monkeypatch `_fetch`
|
||||
themselves, and a fixture that quietly re-patched it for the whole suite
|
||||
would make those tests pass against this table instead of their own.
|
||||
`run` depends on it, so every end-to-end test still gets it.
|
||||
"""
|
||||
answers = {("pub", "http"): "1.6.0", ("toolchain", "dart"): "3.13.2"}
|
||||
monkeypatch.setattr(
|
||||
"dep_freshness.resolve._fetch",
|
||||
lambda eco, name: answers.get((eco, name)),
|
||||
)
|
||||
return answers
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def run(repo, monkeypatch, canned):
|
||||
"""Invoke the CLI inside the throwaway repo and return its exit code.
|
||||
|
||||
`check` is imported here rather than at module scope for the same reason
|
||||
as in `no_network`: the sys.path insert above is what makes the package
|
||||
importable, and a top-level import would run before it.
|
||||
"""
|
||||
from dep_freshness import check
|
||||
|
||||
assert canned, "the registry table must be patched before the CLI runs"
|
||||
|
||||
def invoke(*argv):
|
||||
monkeypatch.chdir(repo)
|
||||
return check.main(list(argv))
|
||||
return invoke
|
||||
|
||||
@ -7,7 +7,6 @@ this repo has shipped exactly that twice before (two fake file-length gates).
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from datetime import date, timedelta
|
||||
import os
|
||||
|
||||
import pytest
|
||||
@ -15,36 +14,12 @@ import pytest
|
||||
from dep_freshness import check
|
||||
from dep_freshness.cache import Cache
|
||||
from dep_freshness.registries import http
|
||||
from dep_freshness.tests.conftest import write
|
||||
|
||||
STALE_PUBSPEC = """\
|
||||
name: demo
|
||||
environment:
|
||||
sdk: ^3.12.2
|
||||
dependencies:
|
||||
http: 1.5.0
|
||||
"""
|
||||
CURRENT_PUBSPEC = STALE_PUBSPEC.replace("1.5.0", "1.6.0")
|
||||
ALLOWLIST = "dependency-freshness.allowlist.yaml"
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def canned(monkeypatch, cache_dir):
|
||||
"""One registry answer for every package the fixtures declare."""
|
||||
answers = {("pub", "http"): "1.6.0", ("toolchain", "dart"): "3.13.2"}
|
||||
monkeypatch.setattr(
|
||||
"dep_freshness.resolve._fetch",
|
||||
lambda eco, name: answers.get((eco, name)),
|
||||
)
|
||||
return answers
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def run(repo, monkeypatch):
|
||||
def invoke(*argv):
|
||||
monkeypatch.chdir(repo)
|
||||
return check.main(list(argv))
|
||||
return invoke
|
||||
from dep_freshness.tests.conftest import (
|
||||
ALLOWLIST,
|
||||
CURRENT_PUBSPEC,
|
||||
STALE_PUBSPEC,
|
||||
write,
|
||||
)
|
||||
|
||||
|
||||
def test_a_stale_pin_fails_and_names_the_package(repo, run, capsys):
|
||||
@ -71,56 +46,6 @@ def test_a_non_manifest_path_is_ignored(repo, run):
|
||||
assert run("lib/main.dart") == 0
|
||||
|
||||
|
||||
def test_a_transitive_exception_excuses_the_finding(repo, run):
|
||||
write(repo, "pubspec.yaml", STALE_PUBSPEC)
|
||||
write(repo, ALLOWLIST, """\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: http
|
||||
pinned: "1.5.0"
|
||||
reason: "something upstream holds it"
|
||||
blocked_by: "transitive:some_pkg@1.0.0"
|
||||
""")
|
||||
assert run("--all") == 0
|
||||
|
||||
|
||||
def test_an_exception_prints_loudly_even_on_success(repo, run, capsys):
|
||||
test_a_transitive_exception_excuses_the_finding(repo, run)
|
||||
err = capsys.readouterr().err
|
||||
assert "DEPENDENCY EXCEPTION IN USE" in err
|
||||
assert "still blocking" in err
|
||||
|
||||
|
||||
def test_an_exception_with_nothing_left_to_excuse_is_an_error(repo, run, capsys):
|
||||
write(repo, "pubspec.yaml", CURRENT_PUBSPEC)
|
||||
write(repo, ALLOWLIST, """\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: http
|
||||
pinned: "1.5.0"
|
||||
reason: "stale entry nobody removed"
|
||||
blocked_by: "transitive:some_pkg@1.0.0"
|
||||
""")
|
||||
assert run("--all") == 2
|
||||
assert "no longer stale" in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_an_expired_allowlist_exits_two(repo, run, capsys):
|
||||
write(repo, "pubspec.yaml", STALE_PUBSPEC)
|
||||
yesterday = (date.today() - timedelta(days=1)).isoformat()
|
||||
write(repo, ALLOWLIST, f"""\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: http
|
||||
pinned: "1.5.0"
|
||||
reason: "held"
|
||||
blocked_by: "discretionary"
|
||||
expires: "{yesterday}"
|
||||
""")
|
||||
assert run("--all") == 2
|
||||
assert "Allowlist ERROR" in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_offline_with_a_cold_cache_passes_with_a_degraded_banner(
|
||||
repo, run, capsys, monkeypatch
|
||||
):
|
||||
@ -186,43 +111,3 @@ def _offline(_ecosystem, _name):
|
||||
|
||||
def test_the_cache_directory_is_honoured(cache_dir):
|
||||
assert os.environ["DEP_FRESHNESS_CACHE"] == str(cache_dir)
|
||||
|
||||
|
||||
SHARED = """\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: some_other_package
|
||||
pinned: "1.0.0"
|
||||
reason: "a fleet-wide hold on something this repo does not use"
|
||||
blocked_by: "transitive:whatever@1.0.0"
|
||||
"""
|
||||
|
||||
|
||||
def test_an_inherited_entry_this_repo_does_not_need_is_not_rot(
|
||||
repo, run, no_shared_allowlist
|
||||
):
|
||||
"""A fleet-wide hold on typescript excuses nothing in a Flutter app.
|
||||
|
||||
Treating that as a dead entry would make every repo without the dependency
|
||||
uncommittable -- the shared allowlist would be unusable for the exact case
|
||||
it exists for.
|
||||
"""
|
||||
no_shared_allowlist.write_text(SHARED, encoding="utf-8")
|
||||
write(repo, "pubspec.yaml", CURRENT_PUBSPEC)
|
||||
assert run("--all") == 0
|
||||
|
||||
|
||||
def test_a_repo_local_entry_with_nothing_to_excuse_is_still_rot(
|
||||
repo, run, no_shared_allowlist
|
||||
):
|
||||
no_shared_allowlist.write_text(SHARED, encoding="utf-8")
|
||||
write(repo, "pubspec.yaml", CURRENT_PUBSPEC)
|
||||
write(repo, ALLOWLIST, """\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: http
|
||||
pinned: "1.5.0"
|
||||
reason: "this repo's own stale entry"
|
||||
blocked_by: "transitive:x@1.0.0"
|
||||
""")
|
||||
assert run("--all") == 2
|
||||
|
||||
131
dep_freshness/tests/test_check_allowlist.py
Normal file
131
dep_freshness/tests/test_check_allowlist.py
Normal file
@ -0,0 +1,131 @@
|
||||
"""Allowlist behaviour, end to end: what an exception excuses, and when it rots.
|
||||
|
||||
Split out of test_check.py for the 250-line cap. The fixtures (`run`,
|
||||
`canned`, `repo`, and the `no_shared_allowlist` redirect) live in conftest.py
|
||||
so both files see one definition.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from datetime import date, timedelta
|
||||
|
||||
from dep_freshness.tests.conftest import (
|
||||
ALLOWLIST,
|
||||
CURRENT_PUBSPEC,
|
||||
STALE_PUBSPEC,
|
||||
write,
|
||||
)
|
||||
|
||||
SHARED = """\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: some_other_package
|
||||
pinned: "1.0.0"
|
||||
reason: "a fleet-wide hold on something this repo does not use"
|
||||
blocked_by: "transitive:whatever@1.0.0"
|
||||
"""
|
||||
|
||||
|
||||
def test_a_transitive_exception_excuses_the_finding(repo, run):
|
||||
write(repo, "pubspec.yaml", STALE_PUBSPEC)
|
||||
write(repo, ALLOWLIST, """\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: http
|
||||
pinned: "1.5.0"
|
||||
reason: "something upstream holds it"
|
||||
blocked_by: "transitive:some_pkg@1.0.0"
|
||||
""")
|
||||
assert run("--all") == 0
|
||||
|
||||
|
||||
def test_an_exception_prints_loudly_even_on_success(repo, run, capsys):
|
||||
test_a_transitive_exception_excuses_the_finding(repo, run)
|
||||
err = capsys.readouterr().err
|
||||
assert "DEPENDENCY EXCEPTION IN USE" in err
|
||||
assert "still blocking" in err
|
||||
|
||||
|
||||
def test_an_exception_with_nothing_left_to_excuse_is_an_error(repo, run, capsys):
|
||||
write(repo, "pubspec.yaml", CURRENT_PUBSPEC)
|
||||
write(repo, ALLOWLIST, """\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: http
|
||||
pinned: "1.5.0"
|
||||
reason: "stale entry nobody removed"
|
||||
blocked_by: "transitive:some_pkg@1.0.0"
|
||||
""")
|
||||
assert run("--all") == 2
|
||||
assert "no longer stale" in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_a_file_scoped_run_never_judges_an_entry_dead(repo, run, capsys):
|
||||
"""pre-commit passes staged files, so it only ever sees a subset.
|
||||
|
||||
The entry below excuses a dependency that lives in a manifest outside the
|
||||
scoped path. `--all` would find it stale and mark the entry used; a scoped
|
||||
run cannot, and must not conclude the entry is rot -- that failed a commit
|
||||
in utils over the fleet-wide typescript hold when the staged files were
|
||||
two pubspec.yaml.
|
||||
"""
|
||||
write(repo, "pubspec.yaml", STALE_PUBSPEC)
|
||||
write(repo, "sub/pubspec.yaml", CURRENT_PUBSPEC)
|
||||
write(repo, ALLOWLIST, """\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: http
|
||||
pinned: "1.5.0"
|
||||
reason: "held by something upstream"
|
||||
blocked_by: "transitive:some_pkg@1.0.0"
|
||||
""")
|
||||
assert run("sub/pubspec.yaml") == 0
|
||||
assert "no longer stale" not in capsys.readouterr().err
|
||||
# The same allowlist over the whole repo still excuses, not rots.
|
||||
assert run("--all") == 0
|
||||
|
||||
|
||||
def test_an_expired_allowlist_exits_two(repo, run, capsys):
|
||||
write(repo, "pubspec.yaml", STALE_PUBSPEC)
|
||||
yesterday = (date.today() - timedelta(days=1)).isoformat()
|
||||
write(repo, ALLOWLIST, f"""\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: http
|
||||
pinned: "1.5.0"
|
||||
reason: "held"
|
||||
blocked_by: "discretionary"
|
||||
expires: "{yesterday}"
|
||||
""")
|
||||
assert run("--all") == 2
|
||||
assert "Allowlist ERROR" in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_an_inherited_entry_this_repo_does_not_need_is_not_rot(
|
||||
repo, run, no_shared_allowlist
|
||||
):
|
||||
"""A fleet-wide hold on typescript excuses nothing in a Flutter app.
|
||||
|
||||
Treating that as a dead entry would make every repo without the dependency
|
||||
uncommittable -- the shared allowlist would be unusable for the exact case
|
||||
it exists for.
|
||||
"""
|
||||
no_shared_allowlist.write_text(SHARED, encoding="utf-8")
|
||||
write(repo, "pubspec.yaml", CURRENT_PUBSPEC)
|
||||
assert run("--all") == 0
|
||||
|
||||
|
||||
def test_a_repo_local_entry_with_nothing_to_excuse_is_still_rot(
|
||||
repo, run, no_shared_allowlist
|
||||
):
|
||||
no_shared_allowlist.write_text(SHARED, encoding="utf-8")
|
||||
write(repo, "pubspec.yaml", CURRENT_PUBSPEC)
|
||||
write(repo, ALLOWLIST, """\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: http
|
||||
pinned: "1.5.0"
|
||||
reason: "this repo's own stale entry"
|
||||
blocked_by: "transitive:x@1.0.0"
|
||||
""")
|
||||
assert run("--all") == 2
|
||||
@ -5,7 +5,7 @@ description: >-
|
||||
helpers that were each reimplemented in three or more repos. Owns the tokens
|
||||
-- consumers import them rather than transcribing the token table by hand.
|
||||
publish_to: 'none'
|
||||
version: 0.2.0
|
||||
version: 0.2.1
|
||||
|
||||
environment:
|
||||
sdk: ^3.12.2
|
||||
|
||||
Loading…
Reference in New Issue
Block a user