diff --git a/AGENTS.md b/AGENTS.md index b5bfb68..009b47e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -184,7 +184,9 @@ line so plain grep works without a parser. - **A successful push deletes the local file** (`.md` and `.comments.md`), and prints the number and URL the issue now lives at. `--update` too: one rule, no exception. What is in the store is what has not - left. Get it back with `pull.py `. + left. Get it back with `pull.py ` — which brings its blockers back with it: + a pull returns the unit of work, not one row of it. `--no-deps` narrows it to + the one issue, and the cost of the default is in `pull.py`'s docstring. - Deletion happens only after a confirmed tracker response and only after `.remote.json` has been written. Network down, non-2xx, an answer that does not carry the right number: the file stays and the run stops. A never-pushed diff --git a/skills/sync/SKILL.md b/skills/sync/SKILL.md index 84219c5..84e3e51 100644 --- a/skills/sync/SKILL.md +++ b/skills/sync/SKILL.md @@ -42,8 +42,8 @@ there is nothing to pin a second time. No pin anywhere → exit with a pointer t | Script | What it does | |---|---| -| `remote.py [--state] [--label] [--milestone] [-q TEXT]` | discovery: one line per Gitea issue to stdout, writes nothing | -| `pull.py ` or `pull.py --milestone M \| --label L \| -q TEXT` | Gitea → `tmp/issues/.md`, plus `.comments.md` when the thread is not empty | +| `remote.py [--state] [--label] [--milestone] [-q TEXT] [--limit N]` | discovery: one line per Gitea issue to stdout, writes nothing; `--limit` caps the **listing** (default 30) | +| `pull.py ` or `pull.py --milestone M \| --label L \| -q TEXT [--limit N]` | Gitea → `tmp/issues/.md`, plus `.comments.md` when the thread is not empty; follows dependencies by default (`--no-deps` to stop); `--limit` caps what is **stored** (default 100) | | `push.py [id…] [--update] [--dry-run]` | local → Gitea; validates first, **deletes the local file on success** and prints where it lives now | | `comment.py --file F \| --body TEXT [--edit N]` | post or edit a comment, then refetch the thread | | `labels.py [--dry-run] [--fix]` | bootstrap the canonical `type/*` + `severity/*` set in a repo; exact names left alone, lookalikes reported, drift fixed only with `--fix` | @@ -93,7 +93,7 @@ python3 /scripts/pull.py 42 python3 /scripts/pull.py --milestone 6 # id or title python3 /scripts/pull.py --label type/bug --state all python3 /scripts/pull.py -q sqlc --limit 20 -python3 /scripts/pull.py 40 --deps # follow dependencies +python3 /scripts/pull.py 40 --no-deps # this issue only ``` Do not loop over numbers to pull a group — pass the filter. The list endpoint @@ -119,13 +119,55 @@ already on disk is refreshed either way — the local copy learns it was closed instead of staying open forever. Key mode is exempt: `pull.py 1` fetches a closed issue as always, because an address is not a bulk read. +**`--limit N` bounds the write, not the selection.** N is how many issues this +run leaves in the store — written, or left in place by `--cached`. Closed ones +that were enumerated and thrown away do not spend it, so `--limit 20` over a +milestone whose first 30 issues are closed still writes 20, as long as 20 open +ones are there to write. Pagination follows the budget rather than the other way +round: + +| | | +|---|---| +| budget full | the next page is never requested | +| pages run out | fewer than N, and that is the honest answer | +| filter matches almost only closed issues | at most 4× the pages N would need if nothing were dropped, then a warning on stderr and a short answer — raising `--limit` raises that ceiling too | +| dependencies | outside the count: a blocker is followed because a stored issue named it, not because the filter selected it — so `--limit 20` can leave more than 20 files behind | + +`remote.py --limit` means something else, deliberately: it caps the **listing**, +closed issues included. It writes nothing, so there is no write for a limit to +bound — enumeration is its whole job. + **Comments come with every pull** — there is no flag. An issue that has a thread gets `tmp/issues/.comments.md` beside it, in key mode and in filter mode alike, and the issue's output line says how many. An issue with none costs nothing: the count arrives in the list payload, so no request is made and no file is written — and a file left over from a thread that has since been emptied is deleted. `--cached` skips the thread along with the body, so a -skipped issue makes no request at all. +skipped issue makes one request for its links and no other. + +**Dependencies come with every pull too, and this one costs.** A pull answers +with the unit of work — the issue and what blocks it — so `depends:` is filled +from Gitea's native graph and every blocker is pulled as well, recursively, down +to `--depth` (default 3). It has to come from the native graph: the body's +`## Depends on` section holds slugs, never `#N`, so there is no edge to recover +from the text. `--no-deps` turns off both halves. `--deps` is still accepted and +does nothing — it names the default. + +| | requests | +|---|---| +| every issue that lands in the store | **+1** — `GET …/issues/{n}/dependencies`, fetched once and used twice (fills `depends:`, steers the walk) | +| every blocker the selection did not already carry | **+1** to fetch it, then its own links, until `--depth` | +| a closed issue filter mode drops | 0 — nothing was stored, so there is no unit of work to complete | +| `--milestone X` over 50 open issues | 1 list request + 50, plus a pair per outside blocker — it used to be 1 | +| the same with `--no-deps` | 1 | + +**A blocker the filter did not select still lands in the store, deliberately.** +`--milestone X` can leave an issue from milestone Y on disk; `--label` can leave +an unlabelled one. It is there because a stored issue names it, not because it +matched. The exception is a closed blocker: closed is not a unit of work, filter +mode drops it like any other closed issue, and the `depends:` edge to it goes +with it — nothing is left pointing at a file that is not there. Key mode +(`pull.py 42`) has no such rule and stores it. Two traps this handles for you: @@ -238,7 +280,7 @@ accumulate them however many round trips it makes, and why a body that somehow gained two is cleaned on the next pull. `depends:` survives the same round trip through Gitea's native links (below): -push writes them, `pull.py --deps` reads them back, and the ledger turns the +push writes them, every `pull.py` reads them back, and the ledger turns the numbers into the slugs they had here. Before anything is sent, `/tea:issue`'s validator runs (exactly one `type/*`, @@ -258,7 +300,7 @@ The two directions are symmetric, and they use the same endpoint: | | direction | endpoint | |---|---|---| | `push.py` | `depends:` → native links | `POST …/issues/{n}/dependencies` | -| `pull.py --deps` | native links → `depends:` | `GET …/issues/{n}/dependencies` | +| `pull.py` (default; `--no-deps` off) | native links → `depends:` | `GET …/issues/{n}/dependencies` | The POST body is Gitea's `IssueMeta` — `{"index", "owner", "repo"}` naming the **blocker**, posted to the **blocked** issue's endpoint ("make the issue in the @@ -334,7 +376,7 @@ is the one thing `branch:` exists to record. | `labels` | `labels[]` | names both ways; ids only on write | | `assignees` | `assignees[]` | logins | | `milestone` | `milestone.title` | resolved to an id on write | -| `depends` | native links | slugs here, `IssueMeta` there; push writes them, `pull --deps` reads them | +| `depends` | native links | slugs here, `IssueMeta` there; push writes them, every pull reads them (`--no-deps` opts out) | | — | `ref` | lands in `branch:`; sent only when non-empty | | — | `number`, `html_url` | lands in `gitea:` / `url:` | diff --git a/skills/sync/scripts/_gitea.py b/skills/sync/scripts/_gitea.py index 2cbbf3f..a3b22c1 100644 --- a/skills/sync/scripts/_gitea.py +++ b/skills/sync/scripts/_gitea.py @@ -38,6 +38,13 @@ import urllib.parse REMOTE_MAP = ".remote.json" +# How far past the ideal page count a `keep`-bounded listing may scan before it +# gives up (see list_issues). The ideal is what `limit` would need if every +# payload counted; the slack pays for the ones that do not. It is a bound on +# requests, deliberately small: "fetch until N are kept" without one is "fetch +# the whole tracker" on any repo whose filter matches mostly closed issues. +PAGE_SLACK = 4 + # -------------------------------------------------------------------------- # where request bodies land # -------------------------------------------------------------------------- @@ -174,17 +181,28 @@ def api(login, endpoint, method="GET", payload=None, payload_name=None, die("`tea api %s` returned non-JSON:\n%s" % (endpoint, body[:500])) -def paginate(login, endpoint, limit=50, max_pages=40, **kw): - """GET a list endpoint page by page; return the concatenated list.""" +def pages(login, endpoint, limit=50, max_pages=40, **kw): + """GET a list endpoint page by page, yielding each page as it arrives. + + A generator, because a caller whose budget is spent on what it *keeps* + cannot be served by a function that fetches everything first: the page after + the one that completed the budget must never be requested. Stop consuming + and no further request is made.""" sep = "&" if "?" in endpoint else "?" - out = [] for page in range(1, max_pages + 1): batch = api(login, "%s%spage=%d&limit=%d" % (endpoint, sep, page, limit), **kw) if not isinstance(batch, list) or not batch: - break - out.extend(batch) + return + yield batch if len(batch) < limit: - break + return # a short page is the last one + + +def paginate(login, endpoint, limit=50, max_pages=40, **kw): + """GET a list endpoint page by page; return the concatenated list.""" + out = [] + for batch in pages(login, endpoint, limit=limit, max_pages=max_pages, **kw): + out.extend(batch) return out @@ -247,11 +265,31 @@ def matches(payload, milestone_id=None, labels=()): def list_issues(login, base, state="open", labels=(), query=None, - milestone=None, limit=100): + milestone=None, limit=100, keep=None): """Filtered issue payloads. Returns (payloads, milestone_title). One request per page, and the payload already carries the issue bodies — a - whole milestone costs one call per 50 issues, not one per issue.""" + whole milestone costs one call per 50 issues, not one per issue. + + `limit` counts the payloads the CALLER cares about, not the ones the server + returned. Without `keep` those are the same thing and this behaves as it + always did. With it, `keep(payload)` says whether a payload counts, pages + keep coming until `limit` of them have, and the returned list carries the + ones that did not count too — they were enumerated, and a caller that has + something to say about them (pull.py: "N closed, not stored") still can. + + What `keep` means is the caller's business; this module only counts. Two + boundaries hold whatever it decides: + + - **Stop at the limit.** The page after the one that completed the budget + is not requested — `pages` is a generator and this loop returns out of it. + - **Stop at the page budget.** A predicate that rejects everything must not + turn a bounded read into a walk of the whole tracker, so a filtered read + may scan at most `PAGE_SLACK` times the pages `limit` would need if every + payload counted. Hitting that with an unfilled budget is a warning, not a + silent short answer: the caller asked for N and is told it got fewer.""" + if limit < 1: + die("--limit must be 1 or more, got %d" % limit) ms_id, ms_title = (None, None) if milestone is not None: ms_id, ms_title = resolve_milestone(login, base, milestone) @@ -266,10 +304,25 @@ def list_issues(login, base, state="open", labels=(), query=None, endpoint = "%s/issues?%s" % (base, urllib.parse.urlencode(params)) per_page = min(limit, 50) - got = paginate(login, endpoint, limit=per_page, - max_pages=max(1, -(-limit // per_page))) - got = [p for p in got if matches(p, ms_id, labels)] - return got[:limit], ms_title + ideal = max(1, -(-limit // per_page)) + budget = ideal if keep is None else ideal * PAGE_SLACK + + got, kept, seen_pages, last_full = [], 0, 0, False + for batch in pages(login, endpoint, limit=per_page, max_pages=budget): + seen_pages += 1 + last_full = len(batch) == per_page + for p in batch: + if not matches(p, ms_id, labels): + continue + got.append(p) + if keep is None or keep(p): + kept += 1 + if kept >= limit: + return got, ms_title + if keep is not None and seen_pages >= budget and last_full: + warn("scanned %d page(s) and stopped %d short of --limit %d — there may" + " be more; narrow the filter or raise --limit" % (budget, limit - kept, limit)) + return got, ms_title def get_issue(login, base, number): diff --git a/skills/sync/scripts/pull.py b/skills/sync/scripts/pull.py index 6b522a4..58ec80d 100644 --- a/skills/sync/scripts/pull.py +++ b/skills/sync/scripts/pull.py @@ -28,11 +28,31 @@ not exposed (404 on Gitea 1.26) — use milestones or labels, or the web UI. A closed issue is not a unit of work, so filter mode enumerates it but leaves it out of the store: `--state all` still shows the whole picture, and only -`--state closed` writes one. The limit is on the write, not on the selection — -an issue already on disk is refreshed either way, so the local copy learns it -was closed instead of staying open forever, and the count of the ones left out -goes to stderr. Key mode is exempt: an address is not a bulk read, and -`pull.py 1` fetches a closed issue as it always did. +`--state closed` writes one. An issue already on disk is refreshed either way, +so the local copy learns it was closed instead of staying open forever, and the +count of the ones left out goes to stderr. Key mode is exempt: an address is not +a bulk read, and `pull.py 1` fetches a closed issue as it always did. + +**`--limit` is on the write, not on the selection.** It counts the issues this +run puts in the store — written, or left in place by `--cached` — and never the +closed ones it enumerated and threw away. `--limit 20` over a milestone whose +first 30 issues are closed still writes 20, if 20 open ones are there to write: +pages keep coming until the budget is full. Two boundaries keep that honest: + +- Pages stop the moment the budget is full. Never one page more. +- A filtered read may scan at most `_gitea.PAGE_SLACK` times the pages the limit + would need if nothing were dropped. A filter that matches almost only closed + issues therefore ends in a warning and a short answer, not in a walk of the + whole tracker. Narrow the filter, or raise `--limit`, which raises the budget + with it. +- Dependencies are outside the count: a blocker is followed because a stored + issue named it, not because the filter selected it. `--limit 20` can + therefore leave more than 20 files behind — the budget counts the selection's + writes, and the graph is not part of the selection. + +`remote.py` is the deliberate exception, and it is not the same flag twice: it +writes nothing at all, so there is no write to bound and its `--limit` means +what it says — how many lines to print. Comments ride along by default, in both modes and for every issue written: the thread lands in tmp/issues/.comments.md, beside the issue. It costs @@ -42,8 +62,39 @@ from an earlier pull is deleted. An absent file therefore means "no comments", never "not asked for". The thread is pull-only: editing it changes nothing in Gitea (post with comment.py). +**Dependencies come with every pull.** A pull answers with the whole unit of +work — the issue and what blocks it — so `depends:` is filled from Gitea's +native dependency graph and every blocker is pulled too, recursively, down to +`--depth` (default 3). That graph is the only source there is: `map.from_api` +writes slugs into the `## Depends on` prose and never `#N`, so an edge cannot be +recovered from the body. `--no-deps` turns off both halves — no `depends:`, no +recursion, and no request spent on either. `--deps` is still accepted and now +does nothing; it names what already happens. + +What it costs, stated rather than hidden: + +- **One request per issue that lands in the store** — `GET …/issues/{n}/dependencies`, + fetched once and used twice, since the same links both fill `depends:` and + tell the walk where to go next. A closed issue that filter mode drops costs + nothing: nothing was stored, so there is no unit of work to complete. +- **One request per blocker the selection did not already carry** — a `GET` for + the issue itself, then its own links, and so on until `--depth`. +- So `--milestone X` over 50 open issues is one list request + 50 link requests + + one pair for every blocker outside the milestone, where it used to be one + request flat. `--no-deps` is the way back to one. + +**In filter mode a blocker the filter did not select still lands in the store, +and that is deliberate.** `--milestone X` can leave an issue from milestone Y on +disk and `--label` an unlabelled one: a blocker is followed because a stored +issue names it, not because it matched. The one blocker that does not land is a +closed one — closed is not a unit of work, filter mode drops it the way it drops +any other closed issue, and the `depends:` edge to it goes with it, so nothing +points at a file that is not there. Key mode has no such rule and stores it. + Other flags: - --deps [--depth N] follow dependencies and pull them too + --no-deps do not fill depends:, do not follow blockers + --deps accepted, does nothing: it is the default now + --depth N how deep to follow blockers (default 3) --cached skip issues already on disk (body AND comments) --repo owner/repo default: auto-detect from the CWD git remote @@ -52,7 +103,9 @@ have not pushed are lost — with exactly one exception, checkbox state. A `[x]` on either side wins for any item whose text matches, because a tick is monotone and unioning the two sides is not conflict resolution (gmap.merge_checkbox_state has the rule and its price). `--cached` skips an issue before any of that: it is -not read and not merged. Draw the graph afterwards with the domain's own +not read and not merged — it still costs its one link request, because a cached +issue's blockers can be missing from disk even when it is not (`--cached +--no-deps` is the free one). Draw the graph afterwards with the domain's own issue_tree.py — it needs no network. Login: the operator's pin from .claude/settings.local.json (see /tea:auth). @@ -98,6 +151,23 @@ def id_for(payload, store_ids, remote_map, repo, root): taken=store_ids) +def lands_in_store(payload, drop_closed, store_ids, remote_map, repo, root): + """Would this payload leave a file in the store? The `--limit` predicate. + + It has to be the same test the walk below applies, or the budget is spent on + issues that never land — which is the bug this exists to prevent. So: a + closed issue counts only when the store already has it (it is refreshed, and + that is a write); anything else counts, including one `--cached` will skip, + because a skipped issue is still an issue the store holds when the run ends. + + Cheap in the common case: only a closed payload costs an `id_for`, and that + is a lookup plus, at worst, a stat.""" + if not (drop_closed and payload.get("state") == "closed"): + return True + id = id_for(payload, store_ids, remote_map, repo, root) + return os.path.isfile(issue.path_of(root, id)) + + def comments_path(root, id): """Where an issue's comment thread lives — beside it, under the same slug. Named in `_gitea` because push.py has to delete the same file.""" @@ -131,8 +201,18 @@ def main(): ap.add_argument("-q", "--query", help="search text in title/body") ap.add_argument("--state", default="open", choices=["open", "closed", "all"], help="filter mode only (default: open)") - ap.add_argument("--limit", type=int, default=100, help="filter mode cap (default: 100)") - ap.add_argument("--deps", action="store_true", help="follow dependencies and pull them") + ap.add_argument("--limit", type=int, default=100, + help="filter mode: how many issues to STORE, not to enumerate" + " (default: 100)") + # Dependencies are the default: a pull answers with the unit of work, not + # one row of it. `--deps` stays accepted so the calls and command tables + # written against the old default keep working — it now sets what is + # already set. + ap.add_argument("--no-deps", dest="deps", action="store_false", + help="do not fill depends: and do not follow blockers") + ap.add_argument("--deps", dest="deps", action="store_true", + help="accepted, does nothing: dependencies are followed by default") + ap.set_defaults(deps=True) ap.add_argument("--depth", type=int, default=3, help="max dependency depth (default: 3)") ap.add_argument("--cached", action="store_true", help="skip issues already on disk instead of refetching") @@ -180,9 +260,14 @@ def main(): # ---- seeds ----------------------------------------------------------- if filtered: + # The limit bounds the write, so the transport is told what a write is + # and counts those; the closed ones it enumerated on the way come back + # in the list anyway, to be reported and dropped below. payloads, ms_title = _gitea.list_issues( login, base, state=args.state, labels=args.label, query=args.query, - milestone=args.milestone, limit=args.limit) + milestone=args.milestone, limit=args.limit, + keep=lambda p: lands_in_store(p, drop_closed, store_ids, remote_map, + repo, root)) if not payloads: _gitea.die("no issues match that filter") what = [] @@ -208,35 +293,42 @@ def main(): stored = os.path.isfile(issue.path_of(root, id)) # Closed and not already ours: nothing is written and nothing is asked - # of the server for it, not even its comments. The slug stays unclaimed - # too, so no other issue ends up pointing `depends:` at a missing file. + # of the server for it — not its comments, not its links, and its own + # blockers are not followed. The slug stays unclaimed too, so no other + # issue ends up pointing `depends:` at a missing file. if drop_closed and payload.get("state") == "closed" and not stored: dropped.append(number) + continue # not stored: no unit of work here, so no links are fetched + + store_ids.add(id) + number_of_id[number] = id + + # The native links, fetched ONCE for the two things they are for: + # filling this issue's `depends:` and telling the walk where to go next. + # One request per issue that lands in the store, and only one — the cost + # the docstring quotes is this line. + deps = _gitea.native_deps(login, base, number) if args.deps else [] + + if args.cached and stored: + skipped.append(id) # body and thread unread; only the links cost else: - store_ids.add(id) - number_of_id[number] = id - if args.cached and stored: - skipped.append(id) # untouched, unread, and not one request spent - else: - extra = _gitea.native_deps(login, base, number) if args.deps else [] - # The copy already on disk, as it was when this run started. It - # contributes its ticked checkboxes and nothing else; None when - # the store has never seen this issue. - prev = issues.get(id) - iss, unresolved = gmap.from_api(payload, id, repo, - id_for_number=number_of_id, - extra_numbers=extra, - synced=_gitea.now_iso(), - local_body=prev.body if prev else None) - issue.save(root, iss) - sync_comments(login, base, root, id, number, payload.get("comments") or 0) - remote_map[gmap.remote_key(repo, number)] = id - written.append(id) - pending.append((id, unresolved)) + # The copy already on disk, as it was when this run started. It + # contributes its ticked checkboxes and nothing else; None when + # the store has never seen this issue. + prev = issues.get(id) + iss, unresolved = gmap.from_api(payload, id, repo, + id_for_number=number_of_id, + extra_numbers=deps, + synced=_gitea.now_iso(), + local_body=prev.body if prev else None) + issue.save(root, iss) + sync_comments(login, base, root, id, number, payload.get("comments") or 0) + remote_map[gmap.remote_key(repo, number)] = id + written.append(id) + pending.append((id, unresolved)) if args.deps and depth < args.depth: - child_numbers = (gmap.numbers_in_body(payload.get("body") or "") - + _gitea.native_deps(login, base, number)) + child_numbers = gmap.numbers_in_body(payload.get("body") or "") + deps for n in child_numbers: if n in seen_numbers: continue @@ -265,8 +357,10 @@ def main(): # Compact output — the only thing that lands in the model's context. The # thread rides on the issue's own line; no file means no comments. + graph = False for id in sorted(set(written) | set(skipped)): iss = issue.load(root, id) + graph = graph or bool(iss.depends) note = " (cached)" if id in skipped else "" cpath = comments_path(root, id) if os.path.isfile(cpath): @@ -275,7 +369,9 @@ def main(): id, ", ".join(iss.labels) or "no labels", iss.title, iss.state, issue.path_of(root, id), note)) print("index: %s" % index_path) - if args.deps: + # Now that dependencies are the default, the hint is worth printing when + # there is something to draw, not on every run that could have drawn it. + if graph: print("graph: run issue_tree.py (offline) to draw it") diff --git a/skills/sync/scripts/push.py b/skills/sync/scripts/push.py index 32288a7..563f146 100644 --- a/skills/sync/scripts/push.py +++ b/skills/sync/scripts/push.py @@ -44,7 +44,7 @@ way, so nothing is lost, but the tracker shows no edge for it. The graph goes up with them. Once an issue has its number, every `depends:` entry that also has one becomes a **native Gitea link** — the same -`/dependencies` that `pull.py --deps` reads back, so the tracker shows the +`/dependencies` that every `pull.py` reads back, so the tracker shows the blocking panel and refuses to close a blocked issue first. Topological order means the blocker already has its number by then; no second pass is needed. `--update` links whatever appeared in `depends:` since the last push. A link diff --git a/skills/sync/scripts/remote.py b/skills/sync/scripts/remote.py index 5c19aa1..a27b22a 100644 --- a/skills/sync/scripts/remote.py +++ b/skills/sync/scripts/remote.py @@ -16,6 +16,11 @@ Usage: remote.py [--state open|closed|all] [--label L]… [-q TEXT] [--milestone M] [--limit N] [--repo owner/repo] +`--limit` here caps the LISTING: N lines out, closed ones among them. That is +not what the same flag means to `pull.py`, and the difference is not an +oversight — pull.py bounds what it writes, and this command writes nothing, so +there is nothing else for a limit to bound. Enumeration is the whole job. + Login: the operator's pin from .claude/settings.local.json (see /tea:auth). """ import argparse diff --git a/tests/test_checkbox_merge.py b/tests/test_checkbox_merge.py index 93cadd1..9bac6b6 100644 --- a/tests/test_checkbox_merge.py +++ b/tests/test_checkbox_merge.py @@ -255,6 +255,11 @@ class FakeGitea(object): path, _, query = endpoint.partition("?") params = dict(urllib.parse.parse_qsl(query)) + # Every pull asks for an issue's native links now (dependencies are the + # default). Nothing here has any; the answer just has to exist. + if path.endswith("/dependencies"): + return [] + m = re.match(r"^%s/issues/(\d+)$" % re.escape(BASE), path) if m and method == "GET": return self.issues.get(int(m.group(1))) diff --git a/tests/test_pull_deps_default.py b/tests/test_pull_deps_default.py new file mode 100644 index 0000000..6cea16d --- /dev/null +++ b/tests/test_pull_deps_default.py @@ -0,0 +1,354 @@ +#!/usr/bin/env python3 +""" +A pull returns the unit of work: the issue AND what blocks it. + +`--deps` used to be opt-in, so `pull.py 42` wrote a file with an empty +`depends:` and `issue_tree.py` drew it as a root with no blockers. The edge was +not lost — it lives in Gitea's native dependency graph — but it was not asked +for, and the body cannot supply it: `map.from_api` writes slugs into the +`## Depends on` prose and never `#N`. Following the graph is now the default. + +What is asserted here: + +1. **The default fills the graph.** A bare `pull.py ` fills `depends:` and + pulls the blocker too, down to `--depth`. +2. **`--no-deps` is the way out, and it is free.** No `depends:`, no recursion, + and not one request beyond the issue itself. +3. **`--deps` still works and means nothing.** Calls written against the old + default keep running and get what they always got. +4. **The cost is one request per stored issue.** The native links are fetched + once and used twice — for `depends:` and for the walk. Never twice. +5. **Filter mode follows blockers out of the selection, deliberately.** A + blocker no filter selected still lands in the store and does not spend + `--limit`; a closed one is dropped like any other closed issue, and so is + the edge to it. An issue the filter dropped costs no link request at all. + +The transport is stubbed at `_gitea.api`, as the other suites do it, and the +stub records every call so "how many requests" is an observation. No network, +and no test touches the developer's store: each builds its own in a +`tempfile.TemporaryDirectory()`. +""" +import contextlib +import io +import os +import sys +import tempfile +import unittest +import urllib.parse +from unittest import mock + +_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +for _p in (os.path.join(_ROOT, "skills", "sync", "scripts"), + os.path.join(_ROOT, "skills", "issue", "scripts")): + if _p not in sys.path: + sys.path.insert(0, _p) + +import _gitea # noqa: E402 +import issue # noqa: E402 +import pull # noqa: E402 + +REPO = "claude-skills/tea" +BASE = "repos/%s" % REPO + +BODY = """## Summary +Прозаическое описание задачи. + +## Spec +skills/issue/references/format.md + +## Acceptance criteria +- [ ] что-нибудь работает +""" + + +def payload(number, title, state="open"): + return {"number": number, "title": title, "body": BODY, "state": state, + "comments": 0, "labels": [{"name": "type/task"}], "assignees": [], + "milestone": None, "ref": "main", "updated_at": "2026-08-10T00:00:00Z", + "html_url": "https://git.example/%s/issues/%d" % (REPO, number), + "repository": {"full_name": REPO}} + + +class FakeTracker(object): + """`tea api` answered from memory, with a native dependency graph. + + `listed` is what the list endpoint serves — the filter's selection. `extra` + exists and is fetchable by number but is in no selection, which is how a + blocker outside the filter is modelled. `deps` maps a blocked issue's number + to the numbers that block it, the direction `GET …/dependencies` reads. + """ + + def __init__(self, listed=(), extra=(), deps=None): + self.listed = list(listed) + self.issues = {p["number"]: p for p in list(listed) + list(extra)} + self.deps = {int(k): list(v) for k, v in (deps or {}).items()} + self.calls = [] # (method, path), in request order + + # -- what the tests read off it ---------------------------------------- + + def paths(self, suffix): + return [p for m, p in self.calls if p.endswith(suffix)] + + def issue_gets(self): + """`GET …/issues/` — one issue fetched by number.""" + return [p for m, p in self.calls + if m == "GET" and p.startswith("%s/issues/" % BASE) + and p.rsplit("/", 1)[1].isdigit()] + + # -- the seam ---------------------------------------------------------- + + def api(self, login, endpoint, method="GET", payload=None, + payload_name=None, out_root=None, allow_fail=False): + path, _, qs = endpoint.partition("?") + q = urllib.parse.parse_qs(qs) + self.calls.append((method, path)) + + if path == "%s/issues" % BASE and method == "GET": + page, per = int(q["page"][0]), int(q["limit"][0]) + return self.listed[(page - 1) * per:(page - 1) * per + per] + + if path.endswith("/comments"): + return [] + + if path.endswith("/dependencies") and method == "GET": + n = int(path.split("/issues/")[1].split("/")[0]) + return [self.issues[b] for b in self.deps.get(n, []) if b in self.issues] + + if path.startswith("%s/issues/" % BASE) and method == "GET": + return self.issues.get(int(path.rsplit("/", 1)[1])) + + raise AssertionError("unstubbed call: %s %s" % (method, endpoint)) + + +class PullDepsTestCase(unittest.TestCase): + """A temp store, a fake tracker, no git and no network.""" + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory(prefix="tea-deps-") + self.addCleanup(self.tmp.cleanup) + self.root = os.path.join(self.tmp.name, "tmp", "issues") + os.makedirs(self.root) + p = mock.patch.object(_gitea, "require_login", lambda: "test-login") + p.start() + self.addCleanup(p.stop) + + def serve(self, listed=(), extra=(), deps=None): + self.fake = FakeTracker(listed, extra, deps) + p = mock.patch.object(_gitea, "api", self.fake.api) + p.start() + self.addCleanup(p.stop) + return self.fake + + def blocked_pair(self): + """#10 "Second thing" is blocked by #7 "First thing".""" + return self.serve(listed=[payload(10, "Second thing"), + payload(7, "First thing")], + deps={10: [7]}) + + def run_pull(self, *argv): + out, err = io.StringIO(), io.StringIO() + args = ["pull.py", "--repo", REPO, "--out", self.root] + list(argv) + with mock.patch.object(sys, "argv", args), \ + contextlib.redirect_stdout(out), \ + contextlib.redirect_stderr(err): + pull.main() + return out.getvalue(), err.getvalue() + + def stored(self): + return sorted(issue.all_ids(self.root)) + + def depends_of(self, id): + return issue.load(self.root, id).depends + + +# -------------------------------------------------------------------------- +# 1. the default fills the graph +# -------------------------------------------------------------------------- + +class DepsAreTheDefaultTest(PullDepsTestCase): + + def test_a_bare_pull_fills_depends(self): + """The acceptance criterion, and the whole point: no flag, and the file + knows what blocks it.""" + self.blocked_pair() + self.run_pull("10") + self.assertEqual(self.depends_of("second-thing"), ["first-thing"]) + + def test_a_bare_pull_stores_the_blocker(self): + """`depends:` pointing at a file that is not there would be worse than + an empty one — the blocker comes with it.""" + self.blocked_pair() + self.run_pull("10") + self.assertIn("first-thing", self.stored()) + + def test_the_walk_is_recursive(self): + """A blocker's blocker is context too, down to --depth (default 3).""" + self.serve(listed=[payload(n, "Thing %d" % n) for n in range(1, 6)], + deps={1: [2], 2: [3], 3: [4], 4: [5]}) + self.run_pull("1") + self.assertEqual(self.stored(), ["thing-1", "thing-2", "thing-3", "thing-4"], + "the default depth of 3 was not what was walked") + + def test_depth_bounds_the_walk(self): + self.serve(listed=[payload(n, "Thing %d" % n) for n in range(1, 6)], + deps={1: [2], 2: [3], 3: [4], 4: [5]}) + self.run_pull("1", "--depth", "1") + self.assertEqual(self.stored(), ["thing-1", "thing-2"]) + + def test_the_graph_hint_is_printed_when_there_is_a_graph(self): + self.blocked_pair() + out, _ = self.run_pull("10") + self.assertIn("issue_tree.py", out) + + +# -------------------------------------------------------------------------- +# 2. --no-deps is the way out, and it is free +# -------------------------------------------------------------------------- + +class NoDepsOptsOutTest(PullDepsTestCase): + + def test_no_deps_leaves_depends_empty(self): + self.blocked_pair() + self.run_pull("10", "--no-deps") + self.assertEqual(self.depends_of("second-thing"), []) + + def test_no_deps_does_not_pull_the_blocker(self): + self.blocked_pair() + self.run_pull("10", "--no-deps") + self.assertEqual(self.stored(), ["second-thing"]) + + def test_no_deps_spends_no_extra_request(self): + """The other half of the criterion: not the links, not the blocker. + One issue asked for, one request made.""" + self.blocked_pair() + self.run_pull("10", "--no-deps") + self.assertEqual(self.fake.paths("/dependencies"), []) + self.assertEqual(self.fake.issue_gets(), ["%s/issues/10" % BASE]) + + def test_no_deps_prints_no_graph_hint(self): + self.blocked_pair() + out, _ = self.run_pull("10", "--no-deps") + self.assertNotIn("issue_tree.py", out) + + +# -------------------------------------------------------------------------- +# 3. --deps is still accepted, and means nothing +# -------------------------------------------------------------------------- + +class DepsFlagIsANoOpTest(PullDepsTestCase): + + def test_the_flag_is_still_accepted(self): + """Existing calls and the /tea:sync command tables must not break.""" + self.blocked_pair() + self.run_pull("10", "--deps") + self.assertEqual(self.depends_of("second-thing"), ["first-thing"]) + + def test_it_changes_nothing_about_the_run(self): + self.blocked_pair() + self.run_pull("10", "--deps") + with_flag = (self.stored(), self.depends_of("second-thing"), + list(self.fake.calls)) + + self.setUp() + self.blocked_pair() + self.run_pull("10") + self.assertEqual((self.stored(), self.depends_of("second-thing"), + list(self.fake.calls)), with_flag) + + +# -------------------------------------------------------------------------- +# 4. one request per stored issue +# -------------------------------------------------------------------------- + +class TheCostIsOneRequestPerIssueTest(PullDepsTestCase): + + def test_the_links_are_fetched_once_per_issue(self): + """They fill `depends:` AND steer the walk; fetching them twice is + double the price the docstring quotes.""" + self.blocked_pair() + self.run_pull("10") + self.assertEqual(self.fake.paths("/dependencies"), + ["%s/issues/10/dependencies" % BASE, + "%s/issues/7/dependencies" % BASE]) + + def test_a_bulk_pull_costs_one_per_issue(self): + """The number the docstring quotes: one list request, then one link + request per issue that lands in the store.""" + self.serve(listed=[payload(n, "Thing %d" % n) for n in range(1, 21)]) + self.run_pull("-q", "x") + self.assertEqual(len(self.fake.paths("/dependencies")), 20) + self.assertEqual(len(self.fake.paths("/issues")), 1) + + def test_a_cached_issue_costs_its_links_and_nothing_else(self): + """--cached stops the body and the thread, not the graph: a cached + issue's blockers can be missing from disk even when it is not.""" + self.blocked_pair() + self.run_pull("10", "--no-deps") # only #10 on disk + self.fake.calls = [] + self.run_pull("10", "--cached") + self.assertEqual(self.fake.paths("/dependencies"), + ["%s/issues/10/dependencies" % BASE, + "%s/issues/7/dependencies" % BASE]) + self.assertIn("first-thing", self.stored()) + + +# -------------------------------------------------------------------------- +# 5. filter mode follows blockers out of the selection +# -------------------------------------------------------------------------- + +class FilterModeFollowsOutwardTest(PullDepsTestCase): + + def test_a_blocker_outside_the_filter_lands_in_the_store(self): + """Documented as deliberate: a blocker is followed because a stored + issue named it, not because the filter selected it.""" + self.serve(listed=[payload(1, "Selected thing")], + extra=[payload(99, "Outside thing")], + deps={1: [99]}) + self.run_pull("-q", "x") + self.assertEqual(self.stored(), ["outside-thing", "selected-thing"]) + self.assertEqual(self.depends_of("selected-thing"), ["outside-thing"]) + + def test_a_blocker_does_not_spend_the_limit(self): + """--limit counts the selection's writes; the graph is not part of the + selection, so the store can legitimately hold more than N.""" + self.serve(listed=[payload(n, "Thing %d" % n) for n in range(1, 5)], + extra=[payload(100 + n, "Blocker %d" % n) for n in range(1, 5)], + deps={n: [100 + n] for n in range(1, 5)}) + self.run_pull("-q", "x", "--limit", "2") + self.assertEqual(self.stored(), + ["blocker-1", "blocker-2", "thing-1", "thing-2"]) + + def test_a_closed_blocker_is_dropped_with_the_edge_to_it(self): + """The documented exception. Closed is not a unit of work, so filter + mode drops it like any other closed issue — and `depends:` must not be + left pointing at a file that is not there.""" + self.serve(listed=[payload(1, "Selected thing")], + extra=[payload(99, "Closed blocker", state="closed")], + deps={1: [99]}) + self.run_pull("-q", "x") + self.assertEqual(self.stored(), ["selected-thing"]) + self.assertEqual(self.depends_of("selected-thing"), []) + + def test_a_closed_blocker_is_stored_in_key_mode(self): + """An address is not a bulk read: `pull.py 1` has no closed rule.""" + self.serve(listed=[payload(1, "Selected thing")], + extra=[payload(99, "Closed blocker", state="closed")], + deps={1: [99]}) + self.run_pull("1") + self.assertEqual(self.stored(), ["closed-blocker", "selected-thing"]) + + def test_a_dropped_closed_issue_costs_no_link_request(self): + """Nothing was stored for it, so there is no unit of work to complete + — and its own blockers are not dragged in behind it.""" + self.serve(listed=[payload(1, "Closed thing", state="closed"), + payload(2, "Open thing")], + extra=[payload(50, "Blocker of the closed one")], + deps={1: [50]}) + self.run_pull("-q", "x", "--state", "all") + self.assertEqual(self.fake.paths("/dependencies"), + ["%s/issues/2/dependencies" % BASE]) + self.assertEqual(self.stored(), ["open-thing"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_pull_limit.py b/tests/test_pull_limit.py new file mode 100644 index 0000000..4e733d5 --- /dev/null +++ b/tests/test_pull_limit.py @@ -0,0 +1,349 @@ +#!/usr/bin/env python3 +""" +`pull.py --limit N` bounds the WRITE, not the selection. + +The bug this file exists to keep dead: the limit used to cut the list of +payloads before pull.py dropped the closed ones, so a milestone whose first +issues are closed spent the budget on issues that never reached disk — +`--limit 20` wrote twelve, and the docstring promised twenty. + +What is asserted, in the order the fix has to hold it: + +1. **The count is of files.** N issues under the filter that would be stored → + exactly N files, however many closed ones were enumerated on the way. +2. **Pagination serves the budget.** More pages are requested while the budget + is unfilled, and the page after the one that fills it is never requested. +3. **The scan is bounded.** A filter that matches almost only closed issues + stops after `_gitea.PAGE_SLACK` times the ideal page count, says so, and + returns short — it does not walk the tracker. +4. **`remote.py` is unchanged.** Its `--limit` still caps the listing, closed + issues included, because it writes nothing there is a limit for. + +The transport is stubbed at `_gitea.api`, the way the other suites do it, and +the stub serves `page=` / `limit=` itself so the request pattern is a real +observation and not an assumption. No network, and no test writes to the +developer's store: each one builds its own in a `tempfile.TemporaryDirectory()`. +""" +import contextlib +import io +import os +import sys +import tempfile +import unittest +import urllib.parse +from unittest import mock + +_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +for _p in (os.path.join(_ROOT, "skills", "sync", "scripts"), + os.path.join(_ROOT, "skills", "issue", "scripts")): + if _p not in sys.path: + sys.path.insert(0, _p) + +import _gitea # noqa: E402 +import issue # noqa: E402 +import map as gmap # noqa: E402 +import pull # noqa: E402 +import remote # noqa: E402 + +REPO = "claude-skills/tea" +BASE = "repos/%s" % REPO + +BODY = """## Summary +Прозаическое описание задачи. + +## Spec +skills/issue/references/format.md + +## Acceptance criteria +- [ ] что-нибудь работает +""" + + +def payload(number, state="open", title=None, comments=0): + return {"number": number, "title": title or "Issue number %d" % number, + "body": BODY, "state": state, "comments": comments, + "labels": [{"name": "type/task"}], "assignees": [], "milestone": None, + "ref": "main", "updated_at": "2026-08-10T00:00:00Z", + "html_url": "https://git.example/%s/issues/%d" % (REPO, number), + "repository": {"full_name": REPO}} + + +def alternating(count, first="closed"): + """`count` issues, every other one closed. The shape of the bug report: + closed issues sitting in front of the open ones, in page order.""" + other = "open" if first == "closed" else "closed" + return [payload(n, first if n % 2 else other) for n in range(1, count + 1)] + + +class FakeTracker(object): + """`tea api` answered from a list, with real pagination. + + It slices on the `page=` and `limit=` it was given rather than ignoring + them, so "which pages were requested" is something the test can read off + `self.list_pages` instead of inferring.""" + + def __init__(self, payloads): + self.payloads = list(payloads) + self.list_pages = [] # (page, per_page), in request order + + def api(self, login, endpoint, method="GET", payload=None, + payload_name=None, out_root=None, allow_fail=False): + path, _, qs = endpoint.partition("?") + q = urllib.parse.parse_qs(qs) + + if path == "%s/issues" % BASE and method == "GET": + page, per = int(q["page"][0]), int(q["limit"][0]) + self.list_pages.append((page, per)) + return self.payloads[(page - 1) * per:(page - 1) * per + per] + + if path.endswith("/comments"): + return [] + + if path.endswith("/dependencies"): + return [] + + if "/issues/" in path and method == "GET": + n = int(path.rsplit("/", 1)[1]) + for p in self.payloads: + if p["number"] == n: + return p + return None + + raise AssertionError("unstubbed call: %s %s" % (method, endpoint)) + + +class PullLimitTestCase(unittest.TestCase): + """A temp store, a fake tracker, no git and no network.""" + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory(prefix="tea-limit-") + self.addCleanup(self.tmp.cleanup) + self.root = os.path.join(self.tmp.name, "tmp", "issues") + os.makedirs(self.root) + p = mock.patch.object(_gitea, "require_login", lambda: "test-login") + p.start() + self.addCleanup(p.stop) + + # -- runners ----------------------------------------------------------- + + def serve(self, payloads): + self.fake = FakeTracker(payloads) + p = mock.patch.object(_gitea, "api", self.fake.api) + p.start() + self.addCleanup(p.stop) + return self.fake + + def run_pull(self, *argv): + return self._run(pull, "pull.py", argv) + + def run_remote(self, *argv): + return self._run(remote, "remote.py", argv) + + def _run(self, mod, name, argv): + out, err = io.StringIO(), io.StringIO() + args = [name, "--repo", REPO, "--out", self.root] + list(argv) + with mock.patch.object(sys, "argv", args), \ + contextlib.redirect_stdout(out), \ + contextlib.redirect_stderr(err): + mod.main() + return out.getvalue(), err.getvalue() + + # -- assertions -------------------------------------------------------- + + def stored(self): + return sorted(issue.all_ids(self.root)) + + def assertStoredCount(self, n, why=""): + got = self.stored() + self.assertEqual(len(got), n, "%d issue(s) in the store, wanted %d%s: %s" + % (len(got), n, why and " — " + why, got)) + + +# -------------------------------------------------------------------------- +# 1. the count is of files +# -------------------------------------------------------------------------- + +class LimitCountsWritesTest(PullLimitTestCase): + + def test_closed_issues_do_not_spend_the_budget(self): + """The regression. Half the selection is closed and stands in front of + the open ones; the limit still buys ten files.""" + self.serve(alternating(40)) + self.run_pull("-q", "x", "--state", "all", "--limit", "10") + self.assertStoredCount(10) + + def test_only_open_issues_landed(self): + self.serve(alternating(40)) + self.run_pull("-q", "x", "--state", "all", "--limit", "10") + for id in self.stored(): + self.assertEqual(issue.load(self.root, id).state, "open") + + def test_the_dropped_ones_are_still_reported(self): + """Enumerated-and-dropped is not silence: the closed ones seen on the + pages that were fetched are counted on stderr.""" + self.serve(alternating(40)) + _, err = self.run_pull("-q", "x", "--state", "all", "--limit", "10") + self.assertIn("closed issue(s) enumerated, not stored", err) + + def test_a_closed_issue_already_in_the_store_spends_it(self): + """It is refreshed rather than dropped — that is a write, so it counts. + The limit is on what the store holds when the run ends, and this issue + is in it.""" + kept = issue.Issue(id="already-here", title="Already here", body=BODY, + labels=["type/task"], origin="gitea", + extra={"gitea": gmap.remote_key(REPO, 1)}) + issue.save(self.root, kept) + _gitea.save_map(self.root, {gmap.remote_key(REPO, 1): "already-here"}) + + self.serve(alternating(40)) # #1 is closed, and is on disk + self.run_pull("-q", "x", "--state", "all", "--limit", "10") + self.assertStoredCount(10) + self.assertEqual(issue.load(self.root, "already-here").state, "closed", + "a stored issue must learn it was closed") + + def test_state_closed_writes_closed_ones(self): + """Nothing above may leak into the mode where closed IS the selection.""" + self.serve([payload(n, "closed") for n in range(1, 21)]) + self.run_pull("-q", "x", "--state", "closed", "--limit", "6") + self.assertStoredCount(6) + + +# -------------------------------------------------------------------------- +# 2. pagination serves the budget +# -------------------------------------------------------------------------- + +class PaginationFollowsTheBudgetTest(PullLimitTestCase): + + def test_more_pages_are_fetched_until_the_budget_is_full(self): + """One page of ten holds five open issues, so ten files cost two.""" + self.serve(alternating(40)) + self.run_pull("-q", "x", "--state", "all", "--limit", "10") + self.assertStoredCount(10) + self.assertEqual([p for p, _ in self.fake.list_pages], [1, 2]) + + def test_the_page_after_the_last_needed_one_is_never_requested(self): + """The budget fills inside page 2; page 3 exists and must not be asked + for. Bounding the write must not become fetching the whole repo.""" + self.serve(alternating(200)) + self.run_pull("-q", "x", "--state", "all", "--limit", "10") + self.assertEqual(len(self.fake.list_pages), 2, + "extra pages requested: %r" % (self.fake.list_pages,)) + + def test_an_unfiltered_selection_still_costs_one_page(self): + """Nothing is dropped, so nothing changes: the old arithmetic holds.""" + self.serve([payload(n) for n in range(1, 60)]) + self.run_pull("-q", "x", "--limit", "10") + self.assertStoredCount(10) + self.assertEqual(len(self.fake.list_pages), 1) + + def test_running_out_of_pages_gives_a_short_answer(self): + """Six issues, three of them open, `--limit 10`: three files, no crash, + and no page beyond the last.""" + self.serve(alternating(6)) + self.run_pull("-q", "x", "--state", "all", "--limit", "10") + self.assertStoredCount(3) + self.assertEqual(len(self.fake.list_pages), 1) + + +# -------------------------------------------------------------------------- +# 3. the scan is bounded +# -------------------------------------------------------------------------- + +class ScanIsBoundedTest(PullLimitTestCase): + + def test_a_selection_of_only_closed_issues_stops_at_the_page_budget(self): + self.serve([payload(n, "closed") for n in range(1, 501)]) + _, err = self.run_pull("-q", "x", "--state", "all", "--limit", "10") + self.assertStoredCount(0) + self.assertEqual(len(self.fake.list_pages), _gitea.PAGE_SLACK, + "the scan walked past its budget: %r" % (self.fake.list_pages,)) + self.assertIn("short of --limit", err) + + def test_a_full_budget_does_not_warn(self): + """The warning means "there may be more"; it must not fire on a run + that got everything it asked for.""" + self.serve(alternating(40)) + _, err = self.run_pull("-q", "x", "--state", "all", "--limit", "10") + self.assertNotIn("short of --limit", err) + + def test_a_selection_that_ran_out_does_not_warn(self): + """Six issues in the repo and the server said so — that is an answer, + not a truncation.""" + self.serve(alternating(6)) + _, err = self.run_pull("-q", "x", "--state", "all", "--limit", "10") + self.assertNotIn("short of --limit", err) + + +# -------------------------------------------------------------------------- +# 4. remote.py is the deliberate exception +# -------------------------------------------------------------------------- + +class RemoteListingIsUnchangedTest(PullLimitTestCase): + + def test_the_listing_limit_still_counts_lines_not_writes(self): + """remote.py writes nothing, so there is no write to bound: ten lines + out, closed ones among them, one request.""" + self.serve(alternating(40)) + out, _ = self.run_remote("-q", "x", "--state", "all", "--limit", "10") + numbered = [l for l in out.splitlines() if l.startswith("#")] + self.assertEqual(len(numbered), 10) + self.assertTrue(any("closed" in l for l in numbered), + "a listing that hides closed issues is not a listing") + self.assertEqual(len(self.fake.list_pages), 1) + + def test_it_leaves_the_store_alone(self): + self.serve(alternating(40)) + self.run_remote("-q", "x", "--state", "all", "--limit", "10") + self.assertStoredCount(0, "discovery wrote to the store") + + +# -------------------------------------------------------------------------- +# the transport on its own +# -------------------------------------------------------------------------- + +class ListIssuesKeepTest(PullLimitTestCase): + """`_gitea.list_issues` without a caller in front of it — the counting rule + is the transport's, and it is testable without a store.""" + + def list(self, payloads, **kw): + self.serve(payloads) + return _gitea.list_issues("test-login", BASE, state="all", **kw) + + def test_without_keep_the_limit_caps_the_selection(self): + got, _ = self.list(alternating(40), limit=10) + self.assertEqual(len(got), 10) + + def test_with_keep_the_limit_caps_the_kept(self): + got, _ = self.list(alternating(40), limit=10, + keep=lambda p: p["state"] == "open") + self.assertEqual(len([p for p in got if p["state"] == "open"]), 10) + + def test_the_rejected_ones_come_back_too(self): + """They were enumerated. The caller reports them; the transport does + not get to throw away what it did not count.""" + got, _ = self.list(alternating(40), limit=10, + keep=lambda p: p["state"] == "open") + self.assertTrue([p for p in got if p["state"] == "closed"]) + + def test_a_limit_below_one_is_refused(self): + """The page arithmetic divides by the page size, and a limit of zero + used to make that a traceback. It is a usage error, so it reads like + one.""" + with self.assertRaises(SystemExit): + self.list(alternating(4), limit=0) + + def test_pull_requests_never_count(self): + """`matches` drops them, so they cannot spend the budget either.""" + mixed = [] + for n in range(1, 41): + p = payload(n) + if n % 2: + p["pull_request"] = {"merged": False} + mixed.append(p) + got, _ = self.list(mixed, limit=10, keep=lambda p: True) + self.assertEqual(len(got), 10) + self.assertFalse([p for p in got if p.get("pull_request")]) + + +if __name__ == "__main__": + unittest.main()