merge: apply --limit to the write, not to the selection
This commit was merged in pull request #22.
This commit is contained in:
+20
-2
@@ -37,8 +37,8 @@ the `tea-guard` hook reads. No pin → exit with a pointer to `/tea:auth`.
|
|||||||
|
|
||||||
| Script | What it does |
|
| Script | What it does |
|
||||||
|---|---|
|
|---|---|
|
||||||
| `remote.py [--state] [--label] [--milestone] [-q TEXT]` | discovery: one line per Gitea issue to stdout, writes nothing |
|
| `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 <key…>` or `pull.py --milestone M \| --label L \| -q TEXT` | Gitea → `tmp/issues/<id>.md`, plus `<id>.comments.md` when the thread is not empty |
|
| `pull.py <key…>` or `pull.py --milestone M \| --label L \| -q TEXT [--limit N]` | Gitea → `tmp/issues/<id>.md`, plus `<id>.comments.md` when the thread is not empty; `--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 |
|
| `push.py [id…] [--update] [--dry-run]` | local → Gitea; validates first, **deletes the local file on success** and prints where it lives now |
|
||||||
| `comment.py <id> --file F \| --body TEXT [--edit N]` | post or edit a comment, then refetch the thread |
|
| `comment.py <id> --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` |
|
| `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` |
|
||||||
@@ -114,6 +114,24 @@ 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
|
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.
|
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 |
|
||||||
|
| `--deps` | outside the count: a dependency is followed because an issue named it, not because the filter selected it |
|
||||||
|
|
||||||
|
`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
|
**Comments come with every pull** — there is no flag. An issue that has a
|
||||||
thread gets `tmp/issues/<id>.comments.md` beside it, in key mode and in filter
|
thread gets `tmp/issues/<id>.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
|
mode alike, and the issue's output line says how many. An issue with none
|
||||||
|
|||||||
@@ -30,6 +30,13 @@ import urllib.parse
|
|||||||
PAYLOAD_DIR = ".payload"
|
PAYLOAD_DIR = ".payload"
|
||||||
REMOTE_MAP = ".remote.json"
|
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
|
||||||
|
|
||||||
|
|
||||||
def die(msg, code=1):
|
def die(msg, code=1):
|
||||||
sys.stderr.write("%s: %s\n" % (os.path.basename(sys.argv[0]), msg))
|
sys.stderr.write("%s: %s\n" % (os.path.basename(sys.argv[0]), msg))
|
||||||
@@ -114,17 +121,28 @@ def api(login, endpoint, method="GET", payload=None, payload_name=None,
|
|||||||
die("`tea api %s` returned non-JSON:\n%s" % (endpoint, body[:500]))
|
die("`tea api %s` returned non-JSON:\n%s" % (endpoint, body[:500]))
|
||||||
|
|
||||||
|
|
||||||
def paginate(login, endpoint, limit=50, max_pages=40, **kw):
|
def pages(login, endpoint, limit=50, max_pages=40, **kw):
|
||||||
"""GET a list endpoint page by page; return the concatenated list."""
|
"""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 "?"
|
sep = "&" if "?" in endpoint else "?"
|
||||||
out = []
|
|
||||||
for page in range(1, max_pages + 1):
|
for page in range(1, max_pages + 1):
|
||||||
batch = api(login, "%s%spage=%d&limit=%d" % (endpoint, sep, page, limit), **kw)
|
batch = api(login, "%s%spage=%d&limit=%d" % (endpoint, sep, page, limit), **kw)
|
||||||
if not isinstance(batch, list) or not batch:
|
if not isinstance(batch, list) or not batch:
|
||||||
break
|
return
|
||||||
out.extend(batch)
|
yield batch
|
||||||
if len(batch) < limit:
|
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
|
return out
|
||||||
|
|
||||||
|
|
||||||
@@ -187,11 +205,31 @@ def matches(payload, milestone_id=None, labels=()):
|
|||||||
|
|
||||||
|
|
||||||
def list_issues(login, base, state="open", labels=(), query=None,
|
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).
|
"""Filtered issue payloads. Returns (payloads, milestone_title).
|
||||||
|
|
||||||
One request per page, and the payload already carries the issue bodies — a
|
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)
|
ms_id, ms_title = (None, None)
|
||||||
if milestone is not None:
|
if milestone is not None:
|
||||||
ms_id, ms_title = resolve_milestone(login, base, milestone)
|
ms_id, ms_title = resolve_milestone(login, base, milestone)
|
||||||
@@ -206,10 +244,25 @@ def list_issues(login, base, state="open", labels=(), query=None,
|
|||||||
endpoint = "%s/issues?%s" % (base, urllib.parse.urlencode(params))
|
endpoint = "%s/issues?%s" % (base, urllib.parse.urlencode(params))
|
||||||
|
|
||||||
per_page = min(limit, 50)
|
per_page = min(limit, 50)
|
||||||
got = paginate(login, endpoint, limit=per_page,
|
ideal = max(1, -(-limit // per_page))
|
||||||
max_pages=max(1, -(-limit // per_page)))
|
budget = ideal if keep is None else ideal * PAGE_SLACK
|
||||||
got = [p for p in got if matches(p, ms_id, labels)]
|
|
||||||
return got[:limit], ms_title
|
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):
|
def get_issue(login, base, number):
|
||||||
|
|||||||
@@ -28,11 +28,29 @@ 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
|
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
|
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 —
|
`--state closed` writes one. An issue already on disk is refreshed either way,
|
||||||
an issue already on disk is refreshed either way, so the local copy learns it
|
so the local copy learns it was closed instead of staying open forever, and the
|
||||||
was closed instead of staying open forever, and the count of the ones left out
|
count of the ones left out goes to stderr. Key mode is exempt: an address is not
|
||||||
goes to stderr. Key mode is exempt: an address is not a bulk read, and
|
a bulk read, and `pull.py 1` fetches a closed issue as it always did.
|
||||||
`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.
|
||||||
|
- `--deps` is outside the count: a dependency is followed because an issue
|
||||||
|
named it, not because the filter selected it.
|
||||||
|
|
||||||
|
`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:
|
Comments ride along by default, in both modes and for every issue written:
|
||||||
the thread lands in tmp/issues/<id>.comments.md, beside the issue. It costs
|
the thread lands in tmp/issues/<id>.comments.md, beside the issue. It costs
|
||||||
@@ -98,6 +116,23 @@ def id_for(payload, store_ids, remote_map, repo, root):
|
|||||||
taken=store_ids)
|
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):
|
def comments_path(root, id):
|
||||||
"""Where an issue's comment thread lives — beside it, under the same slug.
|
"""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."""
|
Named in `_gitea` because push.py has to delete the same file."""
|
||||||
@@ -131,7 +166,9 @@ def main():
|
|||||||
ap.add_argument("-q", "--query", help="search text in title/body")
|
ap.add_argument("-q", "--query", help="search text in title/body")
|
||||||
ap.add_argument("--state", default="open", choices=["open", "closed", "all"],
|
ap.add_argument("--state", default="open", choices=["open", "closed", "all"],
|
||||||
help="filter mode only (default: open)")
|
help="filter mode only (default: open)")
|
||||||
ap.add_argument("--limit", type=int, default=100, help="filter mode cap (default: 100)")
|
ap.add_argument("--limit", type=int, default=100,
|
||||||
|
help="filter mode: how many issues to STORE, not to enumerate"
|
||||||
|
" (default: 100)")
|
||||||
ap.add_argument("--deps", action="store_true", help="follow dependencies and pull them")
|
ap.add_argument("--deps", action="store_true", help="follow dependencies and pull them")
|
||||||
ap.add_argument("--depth", type=int, default=3, help="max dependency depth (default: 3)")
|
ap.add_argument("--depth", type=int, default=3, help="max dependency depth (default: 3)")
|
||||||
ap.add_argument("--cached", action="store_true",
|
ap.add_argument("--cached", action="store_true",
|
||||||
@@ -180,9 +217,14 @@ def main():
|
|||||||
|
|
||||||
# ---- seeds -----------------------------------------------------------
|
# ---- seeds -----------------------------------------------------------
|
||||||
if filtered:
|
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(
|
payloads, ms_title = _gitea.list_issues(
|
||||||
login, base, state=args.state, labels=args.label, query=args.query,
|
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:
|
if not payloads:
|
||||||
_gitea.die("no issues match that filter")
|
_gitea.die("no issues match that filter")
|
||||||
what = []
|
what = []
|
||||||
|
|||||||
@@ -16,6 +16,11 @@ Usage:
|
|||||||
remote.py [--state open|closed|all] [--label L]… [-q TEXT]
|
remote.py [--state open|closed|all] [--label L]… [-q TEXT]
|
||||||
[--milestone M] [--limit N] [--repo owner/repo]
|
[--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).
|
Login: the operator's pin from .claude/settings.local.json (see /tea:auth).
|
||||||
"""
|
"""
|
||||||
import argparse
|
import argparse
|
||||||
|
|||||||
@@ -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()
|
||||||
Reference in New Issue
Block a user