merge: follow dependencies on every pull by default
This commit was merged in pull request #36.
This commit is contained in:
@@ -138,7 +138,9 @@ line so plain grep works without a parser.
|
|||||||
- **A successful push deletes the local file** (`<id>.md` and
|
- **A successful push deletes the local file** (`<id>.md` and
|
||||||
`<id>.comments.md`), and prints the number and URL the issue now lives at.
|
`<id>.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
|
`--update` too: one rule, no exception. What is in the store is what has not
|
||||||
left. Get it back with `pull.py <n>`.
|
left. Get it back with `pull.py <n>` — 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
|
- Deletion happens only after a confirmed tracker response and only after
|
||||||
`.remote.json` has been written. Network down, non-2xx, an answer that does
|
`.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
|
not carry the right number: the file stays and the run stops. A never-pushed
|
||||||
|
|||||||
+31
-7
@@ -38,7 +38,7 @@ 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] [--limit N]` | discovery: one line per Gitea issue to stdout, writes nothing; `--limit` caps the **listing** (default 30) |
|
| `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 [--limit N]` | Gitea → `tmp/issues/<id>.md`, plus `<id>.comments.md` when the thread is not empty; `--limit` caps what is **stored** (default 100) |
|
| `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; 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 |
|
| `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` |
|
||||||
@@ -88,7 +88,7 @@ python3 <skill-base-dir>/scripts/pull.py 42
|
|||||||
python3 <skill-base-dir>/scripts/pull.py --milestone 6 # id or title
|
python3 <skill-base-dir>/scripts/pull.py --milestone 6 # id or title
|
||||||
python3 <skill-base-dir>/scripts/pull.py --label type/bug --state all
|
python3 <skill-base-dir>/scripts/pull.py --label type/bug --state all
|
||||||
python3 <skill-base-dir>/scripts/pull.py -q sqlc --limit 20
|
python3 <skill-base-dir>/scripts/pull.py -q sqlc --limit 20
|
||||||
python3 <skill-base-dir>/scripts/pull.py 40 --deps # follow dependencies
|
python3 <skill-base-dir>/scripts/pull.py 40 --no-deps # this issue only
|
||||||
```
|
```
|
||||||
|
|
||||||
Do not loop over numbers to pull a group — pass the filter. The list endpoint
|
Do not loop over numbers to pull a group — pass the filter. The list endpoint
|
||||||
@@ -126,7 +126,7 @@ round:
|
|||||||
| budget full | the next page is never requested |
|
| budget full | the next page is never requested |
|
||||||
| pages run out | fewer than N, and that is the honest answer |
|
| 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 |
|
| 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 |
|
| 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**,
|
`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
|
closed issues included. It writes nothing, so there is no write for a limit to
|
||||||
@@ -138,7 +138,31 @@ 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
|
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
|
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
|
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:
|
Two traps this handles for you:
|
||||||
|
|
||||||
@@ -251,7 +275,7 @@ accumulate them however many round trips it makes, and why a body that somehow
|
|||||||
gained two is cleaned on the next pull.
|
gained two is cleaned on the next pull.
|
||||||
|
|
||||||
`depends:` survives the same round trip through Gitea's native links (below):
|
`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.
|
numbers into the slugs they had here.
|
||||||
|
|
||||||
Before anything is sent, `/tea:issue`'s validator runs (exactly one `type/*`,
|
Before anything is sent, `/tea:issue`'s validator runs (exactly one `type/*`,
|
||||||
@@ -271,7 +295,7 @@ The two directions are symmetric, and they use the same endpoint:
|
|||||||
| | direction | endpoint |
|
| | direction | endpoint |
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
| `push.py` | `depends:` → native links | `POST …/issues/{n}/dependencies` |
|
| `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
|
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
|
**blocker**, posted to the **blocked** issue's endpoint ("make the issue in the
|
||||||
@@ -337,7 +361,7 @@ never check out, create, or write anything.
|
|||||||
| `labels` | `labels[]` | names both ways; ids only on write |
|
| `labels` | `labels[]` | names both ways; ids only on write |
|
||||||
| `assignees` | `assignees[]` | logins |
|
| `assignees` | `assignees[]` | logins |
|
||||||
| `milestone` | `milestone.title` | resolved to an id on write |
|
| `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 |
|
| — | `ref` | lands in `branch:`; sent only when non-empty |
|
||||||
| — | `number`, `html_url` | lands in `gitea:` / `url:` |
|
| — | `number`, `html_url` | lands in `gitea:` / `url:` |
|
||||||
|
|
||||||
|
|||||||
+84
-30
@@ -45,8 +45,10 @@ pages keep coming until the budget is full. Two boundaries keep that honest:
|
|||||||
issues therefore ends in a warning and a short answer, not in a walk of the
|
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
|
whole tracker. Narrow the filter, or raise `--limit`, which raises the budget
|
||||||
with it.
|
with it.
|
||||||
- `--deps` is outside the count: a dependency is followed because an issue
|
- Dependencies are outside the count: a blocker is followed because a stored
|
||||||
named it, not because the filter selected it.
|
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
|
`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
|
writes nothing at all, so there is no write to bound and its `--limit` means
|
||||||
@@ -60,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
|
never "not asked for". The thread is pull-only: editing it changes nothing in
|
||||||
Gitea (post with comment.py).
|
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:
|
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)
|
--cached skip issues already on disk (body AND comments)
|
||||||
--repo owner/repo default: auto-detect from the CWD git remote
|
--repo owner/repo default: auto-detect from the CWD git remote
|
||||||
|
|
||||||
@@ -70,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
|
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
|
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
|
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.
|
issue_tree.py — it needs no network.
|
||||||
|
|
||||||
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).
|
||||||
@@ -169,7 +204,15 @@ def main():
|
|||||||
ap.add_argument("--limit", type=int, default=100,
|
ap.add_argument("--limit", type=int, default=100,
|
||||||
help="filter mode: how many issues to STORE, not to enumerate"
|
help="filter mode: how many issues to STORE, not to enumerate"
|
||||||
" (default: 100)")
|
" (default: 100)")
|
||||||
ap.add_argument("--deps", action="store_true", help="follow dependencies and pull them")
|
# 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("--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",
|
||||||
help="skip issues already on disk instead of refetching")
|
help="skip issues already on disk instead of refetching")
|
||||||
@@ -250,35 +293,42 @@ def main():
|
|||||||
stored = os.path.isfile(issue.path_of(root, id))
|
stored = os.path.isfile(issue.path_of(root, id))
|
||||||
|
|
||||||
# Closed and not already ours: nothing is written and nothing is asked
|
# 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
|
# of the server for it — not its comments, not its links, and its own
|
||||||
# too, so no other issue ends up pointing `depends:` at a missing file.
|
# 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:
|
if drop_closed and payload.get("state") == "closed" and not stored:
|
||||||
dropped.append(number)
|
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:
|
else:
|
||||||
store_ids.add(id)
|
# The copy already on disk, as it was when this run started. It
|
||||||
number_of_id[number] = id
|
# contributes its ticked checkboxes and nothing else; None when
|
||||||
if args.cached and stored:
|
# the store has never seen this issue.
|
||||||
skipped.append(id) # untouched, unread, and not one request spent
|
prev = issues.get(id)
|
||||||
else:
|
iss, unresolved = gmap.from_api(payload, id, repo,
|
||||||
extra = _gitea.native_deps(login, base, number) if args.deps else []
|
id_for_number=number_of_id,
|
||||||
# The copy already on disk, as it was when this run started. It
|
extra_numbers=deps,
|
||||||
# contributes its ticked checkboxes and nothing else; None when
|
synced=_gitea.now_iso(),
|
||||||
# the store has never seen this issue.
|
local_body=prev.body if prev else None)
|
||||||
prev = issues.get(id)
|
issue.save(root, iss)
|
||||||
iss, unresolved = gmap.from_api(payload, id, repo,
|
sync_comments(login, base, root, id, number, payload.get("comments") or 0)
|
||||||
id_for_number=number_of_id,
|
remote_map[gmap.remote_key(repo, number)] = id
|
||||||
extra_numbers=extra,
|
written.append(id)
|
||||||
synced=_gitea.now_iso(),
|
pending.append((id, unresolved))
|
||||||
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:
|
if args.deps and depth < args.depth:
|
||||||
child_numbers = (gmap.numbers_in_body(payload.get("body") or "")
|
child_numbers = gmap.numbers_in_body(payload.get("body") or "") + deps
|
||||||
+ _gitea.native_deps(login, base, number))
|
|
||||||
for n in child_numbers:
|
for n in child_numbers:
|
||||||
if n in seen_numbers:
|
if n in seen_numbers:
|
||||||
continue
|
continue
|
||||||
@@ -307,8 +357,10 @@ def main():
|
|||||||
|
|
||||||
# Compact output — the only thing that lands in the model's context. The
|
# 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.
|
# thread rides on the issue's own line; no file means no comments.
|
||||||
|
graph = False
|
||||||
for id in sorted(set(written) | set(skipped)):
|
for id in sorted(set(written) | set(skipped)):
|
||||||
iss = issue.load(root, id)
|
iss = issue.load(root, id)
|
||||||
|
graph = graph or bool(iss.depends)
|
||||||
note = " (cached)" if id in skipped else ""
|
note = " (cached)" if id in skipped else ""
|
||||||
cpath = comments_path(root, id)
|
cpath = comments_path(root, id)
|
||||||
if os.path.isfile(cpath):
|
if os.path.isfile(cpath):
|
||||||
@@ -317,7 +369,9 @@ def main():
|
|||||||
id, ", ".join(iss.labels) or "no labels", iss.title, iss.state,
|
id, ", ".join(iss.labels) or "no labels", iss.title, iss.state,
|
||||||
issue.path_of(root, id), note))
|
issue.path_of(root, id), note))
|
||||||
print("index: %s" % index_path)
|
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")
|
print("graph: run issue_tree.py (offline) to draw it")
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -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:`
|
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
|
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
|
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.
|
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
|
`--update` links whatever appeared in `depends:` since the last push. A link
|
||||||
|
|||||||
@@ -255,6 +255,11 @@ class FakeGitea(object):
|
|||||||
path, _, query = endpoint.partition("?")
|
path, _, query = endpoint.partition("?")
|
||||||
params = dict(urllib.parse.parse_qsl(query))
|
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)
|
m = re.match(r"^%s/issues/(\d+)$" % re.escape(BASE), path)
|
||||||
if m and method == "GET":
|
if m and method == "GET":
|
||||||
return self.issues.get(int(m.group(1)))
|
return self.issues.get(int(m.group(1)))
|
||||||
|
|||||||
@@ -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 <n>` 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/<n>` — 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()
|
||||||
Reference in New Issue
Block a user