hive-forge: fetch issue comments in one request; the endpoint has no paging
The forge's issue-comments endpoint takes no page or limit and always returns the whole thread. #5055 wrapped view's fetch in all_pages, which never saw a short page on a thread of 50+ comments and fetched the same list 50 times: `issue view 3782` printed 3000 comments instead of 60. `comments --limit K` sent K as a page size the forge ignored, so it returned the whole thread. view and pr status's last_comment now send one request. comments fetches the thread once and cuts the --limit / --tail window client-side; the X-Total-Count probe and the computed-page tail walk are gone, and the count of comments outside the window is the length of the list. --since is unchanged. tests/comments_no_paging.rs runs the binary against a loopback mock forge whose comments route ignores page and limit, as Forgejo's does.
This commit is contained in:
atlas
parent
6f8a6ab8e0
commit
1d56a74aad
6 changed files with 215 additions and 262 deletions
|
|
@ -42,7 +42,7 @@ hive-forge issue create --title "..." --body "..."
|
|||
# --- flat aliases below remain valid (hidden) ---
|
||||
hive-forge view 42 # title + body + comments (marks thread read)
|
||||
hive-forge comments 42 # list all comments (human-readable; marks thread read); for PRs, review bodies merged in
|
||||
hive-forge comments 42 --tail 10 # last 10 comments (count-then-page; efficient on long threads)
|
||||
hive-forge comments 42 --tail 10 # last 10 comments (fetches the whole thread, shows the tail)
|
||||
hive-forge --json comments 42 # same as above, JSON array (global flag)
|
||||
hive-forge comment 42 --body "..." # post comment (inline body)
|
||||
hive-forge comment 42 --body-file - <<EOF # ...or pipe a HEREDOC
|
||||
|
|
|
|||
|
|
@ -7,13 +7,12 @@
|
|||
//! default moved to `--tail`'s window instead (see below).
|
||||
//! - `--tail N` returns the *last* N comments, chronological — and with
|
||||
//! neither flag given, THIS is the default (N = 10): most-recent
|
||||
//! activity. Reads this endpoint's own real total first (see
|
||||
//! `fetch_total`) to compute which page holds the tail, then fetches
|
||||
//! only `ceil(N/50) + 1` pages — bounded by `N`, not thread length.
|
||||
//! activity. Both windows are cut from one whole-thread request (see
|
||||
//! `fetch_thread`).
|
||||
//! - **The count of comments outside whatever window is shown is
|
||||
//! always reported** — never a silent truncation. That total is this
|
||||
//! endpoint's own header, never `issue_get_issue`'s `comments` field
|
||||
//! (see `fetch_total`'s doc comment for why).
|
||||
//! always reported** — never a silent truncation. That total is the
|
||||
//! length of this endpoint's list, never `issue_get_issue`'s `comments`
|
||||
//! field, which also counts non-prose events this endpoint never returns.
|
||||
//! - `--since <RFC3339>` filters to comments at or after that
|
||||
//! timestamp instead of a head/tail window — a cursor: feed the
|
||||
//! last-seen row's own `created_at` back in next time. Mutually
|
||||
|
|
@ -38,7 +37,7 @@ use time::OffsetDateTime;
|
|||
use crate::client::{Client, index};
|
||||
use crate::notify;
|
||||
use crate::verbs::{
|
||||
MAX_LIMIT, PAGE_SIZE, all_pages, clamp_limit, comment_reactions, parse_rfc3339, print_json,
|
||||
MAX_LIMIT, all_pages, clamp_limit, comment_reactions, parse_rfc3339, print_json,
|
||||
reaction_summary, rfc3339,
|
||||
};
|
||||
|
||||
|
|
@ -76,10 +75,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
|||
// thread's current end); a `--limit` window has nothing before it (it
|
||||
// starts at the thread's beginning); a `--since` window is a boolean
|
||||
// "there's more" (no total exists for a since-filtered query) rather
|
||||
// than an exact count. `more_before`/`more_after` are derived from
|
||||
// `thread.len()` rather than the requested `n`/`limit` so a
|
||||
// shrunk-under-us thread (comments deleted mid-fetch) still reports
|
||||
// accurately instead of the number we merely asked for.
|
||||
// than an exact count.
|
||||
let (thread, more_before, more_after, since_more, since_clamped) =
|
||||
if let Some(since_str) = &args.since {
|
||||
let since = parse_rfc3339(since_str)?;
|
||||
|
|
@ -87,8 +83,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
|||
let (thread, more) = fetch_since(client, args.number, since, limit)?;
|
||||
(thread, 0, 0, more, clamped)
|
||||
} else if let Some(limit) = args.limit {
|
||||
let (thread, total) = fetch_head(client, args.number, limit)?;
|
||||
let more_after = total.saturating_sub(thread.len());
|
||||
let (thread, more_after) = head(fetch_thread(client, args.number)?, limit);
|
||||
(thread, 0, more_after, false, false)
|
||||
} else {
|
||||
// Default (no `--limit`/`--tail`/`--since`): same window as
|
||||
|
|
@ -96,9 +91,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
|||
// oldest. `args.tail` still wins if the agent passed a
|
||||
// different count explicitly.
|
||||
let n = args.tail.unwrap_or(10);
|
||||
let total = fetch_total(client, args.number)?;
|
||||
let thread = fetch_tail(client, args.number, n, total)?;
|
||||
let more_before = total.saturating_sub(thread.len());
|
||||
let (thread, more_before) = tail(fetch_thread(client, args.number)?, n);
|
||||
(thread, more_before, 0, false, false)
|
||||
};
|
||||
// Merge in PR review bodies (empty for issues — degrades to a
|
||||
|
|
@ -276,50 +269,6 @@ fn attachment_json_from_value(c: &Value) -> Vec<Value> {
|
|||
.collect()
|
||||
}
|
||||
|
||||
/// Interpret a comments-endpoint response's `X-Total-Count` header as
|
||||
/// the real prose-comment total, falling back to `fallback` (the
|
||||
/// window just fetched, or `0` for a bare probe) when the header is
|
||||
/// absent or won't fit a `usize` (defensive — Forgejo always sends a
|
||||
/// small non-negative value in practice). Never invents a number: a
|
||||
/// missing header only ever *under*-reports via the fallback, it can't
|
||||
/// manufacture a `more_before`/`more_after` count that then turns out
|
||||
/// to be a non-prose event the comments endpoint would never surface.
|
||||
fn total_from_header(x_total_count: Option<i64>, fallback: usize) -> usize {
|
||||
x_total_count
|
||||
.and_then(|t| usize::try_from(t).ok())
|
||||
.unwrap_or(fallback)
|
||||
}
|
||||
|
||||
/// Real *prose*-comment count on an issue/PR, read off this same
|
||||
/// endpoint's own `X-Total-Count` header via a cheap `page_size=1`
|
||||
/// probe — used to plan `--tail`'s pagination and to report how many
|
||||
/// comments sit outside whatever window ends up shown.
|
||||
///
|
||||
/// This used to read `issue_get_issue`'s `comments` field instead —
|
||||
/// Forgejo's running counter on the Issue object, which also increments for
|
||||
/// several non-prose event kinds (`review_request`, `issue_ref`,
|
||||
/// `comment_ref`, `pull_push`, …) that `issue_get_comments` never
|
||||
/// returns. Subtracting that inflated count against this endpoint's own
|
||||
/// results reported drift that was never real: a comment that would
|
||||
/// never be shown at any window size. Mirrors `timeline.rs`'s own
|
||||
/// `fetch_total`, which moved to the same trick for its own endpoint
|
||||
/// first — reading the count straight off the endpoint that also
|
||||
/// produces the window means the two can never drift apart again.
|
||||
fn fetch_total(client: &Client, number: u64) -> Result<usize> {
|
||||
let (owner, name) = client.owner_repo()?;
|
||||
let (headers, _) = client
|
||||
.api()
|
||||
.issue_get_comments(
|
||||
owner,
|
||||
name,
|
||||
index(number)?,
|
||||
IssueGetCommentsQuery::default(),
|
||||
)
|
||||
.page_size(1)
|
||||
.send()?;
|
||||
Ok(total_from_header(headers.x_total_count, 0))
|
||||
}
|
||||
|
||||
/// Serialize a typed comment page back to the JSON `Value` shape the
|
||||
/// merge + render pipeline works on (the structs serialize to the API
|
||||
/// wire shape, so downstream field access is unchanged).
|
||||
|
|
@ -407,13 +356,12 @@ fn merge_chronological(mut items: Vec<Value>, reviews: Vec<Value>) -> Vec<Value>
|
|||
items
|
||||
}
|
||||
|
||||
/// Fetch the first page's worth of comments (existing behaviour) plus
|
||||
/// the thread's real total, read straight off this same request's
|
||||
/// `X-Total-Count` header — one request, no separate `fetch_total`
|
||||
/// call needed for this branch.
|
||||
fn fetch_head(client: &Client, number: u64, limit: u64) -> Result<(Vec<Value>, usize)> {
|
||||
/// Every comment on an issue/PR, oldest-first, in one request. The
|
||||
/// endpoint takes no page or limit, so the response is always the whole
|
||||
/// thread and a window has to be cut client-side.
|
||||
fn fetch_thread(client: &Client, number: u64) -> Result<Vec<Value>> {
|
||||
let (owner, name) = client.owner_repo()?;
|
||||
let (headers, comments) = client
|
||||
let (_, comments) = client
|
||||
.api()
|
||||
.issue_get_comments(
|
||||
owner,
|
||||
|
|
@ -421,59 +369,23 @@ fn fetch_head(client: &Client, number: u64, limit: u64) -> Result<(Vec<Value>, u
|
|||
index(number)?,
|
||||
IssueGetCommentsQuery::default(),
|
||||
)
|
||||
.page_size(u32::try_from(limit).unwrap_or(u32::MAX))
|
||||
.send()?;
|
||||
let total = total_from_header(headers.x_total_count, comments.len());
|
||||
Ok((to_values(comments)?, total))
|
||||
to_values(comments)
|
||||
}
|
||||
|
||||
/// Fetch the last `n` comments on an issue/PR in chronological order.
|
||||
/// `total` (the thread's comment count, from [`fetch_total`]) drives the
|
||||
/// pagination plan and is the caller's, not fetched again here.
|
||||
///
|
||||
/// Forgejo orders `/issues/<n>/comments` oldest-first and has no
|
||||
/// `direction=desc` knob, so naive "page everything and slice" pages
|
||||
/// from the WRONG end on long threads — the first 1000 comments
|
||||
/// instead of the last `n`. Fix: use `total` to know how many exist,
|
||||
/// then start paginating from the page that contains item
|
||||
/// `total - n`. Work is bounded by `ceil(n/50) + 1` page fetches,
|
||||
/// regardless of thread length.
|
||||
fn fetch_tail(client: &Client, number: u64, n: usize, total: usize) -> Result<Vec<Value>> {
|
||||
if n == 0 || total == 0 {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
let (owner, name) = client.owner_repo()?;
|
||||
let idx = index(number)?;
|
||||
// Cap `n` at the actual total so the math below stays in range
|
||||
// when the caller asks for more comments than exist.
|
||||
let n = n.min(total);
|
||||
let page_size = usize::try_from(PAGE_SIZE).unwrap_or(usize::MAX);
|
||||
// 0-based index of the first comment we want; integer-divide to
|
||||
// get the 1-based page that contains it.
|
||||
let start_idx = total - n;
|
||||
let start_page = (start_idx / page_size) + 1;
|
||||
let last_page = (total - 1) / page_size + 1;
|
||||
let mut merged: Vec<Value> = Vec::with_capacity(n + page_size);
|
||||
for page in start_page..=last_page {
|
||||
let (_, arr) = client
|
||||
.api()
|
||||
.issue_get_comments(owner, name, idx, IssueGetCommentsQuery::default())
|
||||
.page(u32::try_from(page).unwrap_or(u32::MAX))
|
||||
.page_size(u32::try_from(PAGE_SIZE).unwrap_or(u32::MAX))
|
||||
.send()?;
|
||||
if arr.is_empty() {
|
||||
// Page came back empty — either we miscounted (comments
|
||||
// deleted between the issue GET and now) or upstream's
|
||||
// playing tricks. Stop rather than spin.
|
||||
break;
|
||||
}
|
||||
merged.extend(to_values(arr)?);
|
||||
}
|
||||
// The first fetched page contains items from `start_page` × 50
|
||||
// back; we overshoot by `start_idx % 50` items. Slice the tail
|
||||
// to exactly `n` (or fewer if the count shrank under us).
|
||||
let overshoot = merged.len().saturating_sub(n);
|
||||
Ok(merged.into_iter().skip(overshoot).collect())
|
||||
/// The first `limit` comments of `thread`, and how many follow them.
|
||||
fn head(mut thread: Vec<Value>, limit: u64) -> (Vec<Value>, usize) {
|
||||
let total = thread.len();
|
||||
thread.truncate(usize::try_from(limit).unwrap_or(usize::MAX));
|
||||
let more_after = total - thread.len();
|
||||
(thread, more_after)
|
||||
}
|
||||
|
||||
/// The last `n` comments of `thread`, and how many precede them.
|
||||
fn tail(mut thread: Vec<Value>, n: usize) -> (Vec<Value>, usize) {
|
||||
let more_before = thread.len().saturating_sub(n);
|
||||
thread.drain(..more_before);
|
||||
(thread, more_before)
|
||||
}
|
||||
|
||||
/// Fetch comments at or after `since`, capped at `limit` (already
|
||||
|
|
@ -510,121 +422,40 @@ fn fetch_since(
|
|||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// Pure helper mirroring the page-arithmetic in `fetch_tail`:
|
||||
/// given a total comment count + requested tail size, return
|
||||
/// the (`start_page`, `last_page`) pair the network loop would
|
||||
/// walk. Lets us pin the pagination plan — the part that's
|
||||
/// easy to off-by-one — without touching the network.
|
||||
fn tail_plan(total: usize, n: usize) -> (usize, usize) {
|
||||
let n = n.min(total);
|
||||
let page_size = usize::try_from(PAGE_SIZE).unwrap_or(usize::MAX);
|
||||
let start_idx = total - n;
|
||||
let start_page = (start_idx / page_size) + 1;
|
||||
let last_page = (total - 1) / page_size + 1;
|
||||
(start_page, last_page)
|
||||
fn thread(len: usize) -> Vec<Value> {
|
||||
(0..len).map(|i| json!({ "id": i })).collect()
|
||||
}
|
||||
|
||||
fn ids(thread: &[Value]) -> Vec<u64> {
|
||||
thread.iter().filter_map(|c| c["id"].as_u64()).collect()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tail_plan_small_thread() {
|
||||
// 5 total, tail 2 → page 1 covers everything; trim happens
|
||||
// via the merged.len() - n overshoot calculation, not pages.
|
||||
assert_eq!(tail_plan(5, 2), (1, 1));
|
||||
fn head_keeps_the_first_limit_and_counts_the_rest() {
|
||||
let (shown, more_after) = head(thread(60), 5);
|
||||
assert_eq!(ids(&shown), vec![0, 1, 2, 3, 4]);
|
||||
assert_eq!(more_after, 55);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tail_plan_exact_page_boundary() {
|
||||
// 50 total, tail 3 → all on page 1 (items 1..50).
|
||||
assert_eq!(tail_plan(50, 3), (1, 1));
|
||||
fn head_past_the_end_shows_everything() {
|
||||
let (shown, more_after) = head(thread(3), 10);
|
||||
assert_eq!(shown.len(), 3);
|
||||
assert_eq!(more_after, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tail_plan_crosses_page_boundary() {
|
||||
// 51 total, tail 3 → start_idx=48 lives on page 1, item 51
|
||||
// lives on page 2; fetch both.
|
||||
assert_eq!(tail_plan(51, 3), (1, 2));
|
||||
fn tail_keeps_the_last_n_and_counts_the_rest() {
|
||||
let (shown, more_before) = tail(thread(60), 3);
|
||||
assert_eq!(ids(&shown), vec![57, 58, 59]);
|
||||
assert_eq!(more_before, 57);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tail_plan_large_thread_bounded_pages() {
|
||||
// 5000 total, tail 3 → start_idx=4997 lives on page 100,
|
||||
// last_page=100. ONE page fetch on a 5000-comment thread —
|
||||
// the whole point of swapping to count-then-page (vs the
|
||||
// old "page everything, then slice the wrong end").
|
||||
assert_eq!(tail_plan(5000, 3), (100, 100));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tail_plan_large_thread_spans_two_pages() {
|
||||
// 5000 total, tail 60 → start_idx=4940 on page 99, item 5000
|
||||
// on page 100. Two fetches even for n > PAGE_SIZE.
|
||||
assert_eq!(tail_plan(5000, 60), (99, 100));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tail_plan_n_exceeds_total() {
|
||||
// 5 total, tail 100 → cap n at total; same plan as the
|
||||
// small-thread case above.
|
||||
assert_eq!(tail_plan(5, 100), (1, 1));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tail_plan_with_inflated_total_overshoots_the_real_last_page() {
|
||||
// The offset half of the mismatched-total mechanism: 48 real prose comments
|
||||
// (all fit on page 1, page_size 50) plus 5 non-prose events the
|
||||
// old `issue.comments` counter double-counted gives an inflated
|
||||
// total of 53. The correct plan never leaves page 1 — everything
|
||||
// real lives there.
|
||||
assert_eq!(tail_plan(48, 3), (1, 1));
|
||||
// Fed the inflated total instead, the plan reaches for page 2,
|
||||
// which doesn't exist on the real thread — `fetch_tail`'s
|
||||
// `arr.is_empty()` guard then breaks immediately and returns
|
||||
// zero comments instead of the real last 3. Not just a wrong
|
||||
// trailer number: the wrong rows (none) get shown.
|
||||
assert_eq!(tail_plan(53, 3), (2, 2));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn total_from_header_prefers_the_header_over_the_fallback() {
|
||||
assert_eq!(total_from_header(Some(53), 8), 53);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn total_from_header_falls_back_when_header_absent() {
|
||||
// Never invents a number it doesn't have — under-reports to the
|
||||
// fallback rather than guessing.
|
||||
assert_eq!(total_from_header(None, 8), 8);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn total_from_header_falls_back_on_a_malformed_value() {
|
||||
// A negative count can't be a real `usize`; fall back rather
|
||||
// than silently wrapping.
|
||||
assert_eq!(total_from_header(Some(-1), 8), 8);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn drift_scenario_reports_nothing_withheld_when_header_matches_window() {
|
||||
// The core mismatched-total scenario: the real total, read off the comments
|
||||
// endpoint's own header, matches the fetched window exactly, even
|
||||
// though a differently-scoped counter (the old `issue.comments`
|
||||
// field, no longer read) would have come back higher because of
|
||||
// unrelated non-prose events (`review_request`, `issue_ref`,
|
||||
// `comment_ref`, …) that endpoint never returns. Trailer must
|
||||
// report nothing withheld.
|
||||
let window_len = 8;
|
||||
let total = total_from_header(Some(8), window_len);
|
||||
assert_eq!(truncation_note(total.saturating_sub(window_len), 0), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn drift_scenario_still_reports_a_genuine_truncation() {
|
||||
// A real truncation must still surface — silencing the trailer
|
||||
// outright would swap a false positive for a false negative,
|
||||
// which is strictly worse.
|
||||
let window_len = 10;
|
||||
let total = total_from_header(Some(12), window_len);
|
||||
let note = truncation_note(total.saturating_sub(window_len), 0).unwrap();
|
||||
assert!(note.contains('2'), "{note}");
|
||||
fn tail_past_the_start_shows_everything() {
|
||||
let (shown, more_before) = tail(thread(3), 10);
|
||||
assert_eq!(shown.len(), 3);
|
||||
assert_eq!(more_before, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -213,35 +213,24 @@ fn fetch_combined(client: &Client, sha: &str) -> Result<(String, Vec<Value>)> {
|
|||
}
|
||||
|
||||
/// The most recent issue comment on the PR, as `(login, created_at)`.
|
||||
/// Comments page oldest-first; we drain (bounded) and take the max
|
||||
/// timestamp so a long thread still reports the genuinely-latest one.
|
||||
/// One request: the comments endpoint takes no page or limit and returns
|
||||
/// the whole thread. Takes the max timestamp rather than the last row.
|
||||
fn last_comment(client: &Client, pr: u64) -> Result<Option<(String, String)>> {
|
||||
const PAGE_SIZE: u32 = 50;
|
||||
const MAX_PAGES: u32 = 20;
|
||||
let (owner, name) = client.owner_repo()?;
|
||||
let idx = index(pr)?;
|
||||
let (_, comments) = client
|
||||
.api()
|
||||
.issue_get_comments(owner, name, index(pr)?, IssueGetCommentsQuery::default())
|
||||
.send()?;
|
||||
let mut last: Option<(String, String)> = None;
|
||||
for page in 1..=MAX_PAGES {
|
||||
let (_, comments) = client
|
||||
.api()
|
||||
.issue_get_comments(owner, name, idx, IssueGetCommentsQuery::default())
|
||||
.page(page)
|
||||
.page_size(PAGE_SIZE)
|
||||
.send()?;
|
||||
let short = comments.len() < PAGE_SIZE as usize;
|
||||
for c in &comments {
|
||||
let Some(login) = c.user.as_ref().and_then(|u| u.login.clone()) else {
|
||||
continue;
|
||||
};
|
||||
let Some(created) = rfc3339(c.created_at) else {
|
||||
continue;
|
||||
};
|
||||
if last.as_ref().is_none_or(|(_, t)| created > *t) {
|
||||
last = Some((login, created));
|
||||
}
|
||||
}
|
||||
if short {
|
||||
break;
|
||||
for c in &comments {
|
||||
let Some(login) = c.user.as_ref().and_then(|u| u.login.clone()) else {
|
||||
continue;
|
||||
};
|
||||
let Some(created) = rfc3339(c.created_at) else {
|
||||
continue;
|
||||
};
|
||||
if last.as_ref().is_none_or(|(_, t)| created > *t) {
|
||||
last = Some((login, created));
|
||||
}
|
||||
}
|
||||
Ok(last)
|
||||
|
|
|
|||
|
|
@ -138,10 +138,9 @@ fn truncation_note(more_before: usize, more_after: usize) -> Option<String> {
|
|||
}
|
||||
|
||||
/// Total timeline-event count for an issue/PR, read off this endpoint's
|
||||
/// own `X-Total-Count` header via a cheap `page_size=1` request. Unlike
|
||||
/// `comments.rs`'s `fetch_total` (which reads a field straight off the
|
||||
/// issue object) there's no cheaper alternate endpoint carrying this
|
||||
/// count — this request-then-discard-the-one-item IS the cheap path.
|
||||
/// own `X-Total-Count` header via a cheap `page_size=1` request. No
|
||||
/// cheaper alternate endpoint carries this count — this
|
||||
/// request-then-discard-the-one-item IS the cheap path.
|
||||
fn fetch_total(client: &Client, number: u64) -> Result<usize> {
|
||||
let (owner, name) = client.owner_repo()?;
|
||||
let (headers, _) = client
|
||||
|
|
@ -193,8 +192,7 @@ fn fetch_head(client: &Client, number: u64, limit: u64) -> Result<(Vec<TimelineC
|
|||
|
||||
/// Fetch the last `n` timeline events (chronological). `total` (from
|
||||
/// [`fetch_total`]) drives the pagination plan and is the caller's, not
|
||||
/// fetched again here — same shape as `comments.rs`'s `fetch_tail`:
|
||||
/// Forgejo paginates oldest-first with no `direction=desc` knob, so this
|
||||
/// fetched again here. Forgejo paginates oldest-first with no `direction=desc` knob, so this
|
||||
/// starts fetching from the page that contains item `total - n` rather
|
||||
/// than paging the whole thread.
|
||||
fn fetch_tail(
|
||||
|
|
@ -472,8 +470,7 @@ mod tests {
|
|||
|
||||
/// Pure helper mirroring the page-arithmetic in `fetch_tail`: given a
|
||||
/// total event count + requested tail size, return the (`start_page`,
|
||||
/// `last_page`) pair the network loop would walk. Same shape as
|
||||
/// `comments.rs`'s `tail_plan` test helper — lets us pin the
|
||||
/// `last_page`) pair the network loop would walk — lets us pin the
|
||||
/// pagination plan without touching the network.
|
||||
fn tail_plan(total: usize, n: usize) -> (usize, usize) {
|
||||
let n = n.min(total);
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ use forgejo_api::structs::{IssueGetCommentsQuery, StateType};
|
|||
|
||||
use crate::client::{Client, index};
|
||||
use crate::notify;
|
||||
use crate::verbs::{all_pages, attachment_line, issue_reactions, reaction_summary, refuse_json};
|
||||
use crate::verbs::{attachment_line, issue_reactions, reaction_summary, refuse_json};
|
||||
|
||||
#[derive(ClapArgs)]
|
||||
pub struct Args {
|
||||
|
|
@ -63,15 +63,12 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
|||
println!("[reactions: {summary}]");
|
||||
}
|
||||
let idx = index(args.number)?;
|
||||
let comments = all_pages(|page, size| {
|
||||
let (_, batch) = client
|
||||
.api()
|
||||
.issue_get_comments(owner, name, idx, IssueGetCommentsQuery::default())
|
||||
.page(page)
|
||||
.page_size(size)
|
||||
.send()?;
|
||||
Ok(batch)
|
||||
})?;
|
||||
// One request: this endpoint takes no page or limit and returns the
|
||||
// whole thread, so a paging loop would fetch it again on every page.
|
||||
let (_, comments) = client
|
||||
.api()
|
||||
.issue_get_comments(owner, name, idx, IssueGetCommentsQuery::default())
|
||||
.send()?;
|
||||
if !comments.is_empty() {
|
||||
println!();
|
||||
println!("---");
|
||||
|
|
|
|||
139
hive-forge/tests/comments_no_paging.rs
Normal file
139
hive-forge/tests/comments_no_paging.rs
Normal file
|
|
@ -0,0 +1,139 @@
|
|||
//! `view` and `comments` against a mock forge whose issue-comments
|
||||
//! endpoint behaves like Forgejo's: it takes no page or limit and returns
|
||||
//! the whole thread on every request. Drives the compiled binary, since
|
||||
//! the request code is inline in each verb's `run()`.
|
||||
|
||||
use std::io::{BufRead as _, BufReader, Write as _};
|
||||
use std::net::{TcpListener, TcpStream};
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
|
||||
/// More than one 50-row page, so a loop that pages until a short page
|
||||
/// never sees one.
|
||||
const THREAD_LEN: usize = 60;
|
||||
|
||||
/// The issue the mock forge serves; any other route gets an empty list.
|
||||
const NUMBER: &str = "1";
|
||||
|
||||
/// A unique scratch `HYPERHIVE_STATE_DIR` holding a `forge-token` file,
|
||||
/// same manual-tmpdir pattern as `credential_helper.rs`.
|
||||
fn scratch_state_dir(tag: &str) -> PathBuf {
|
||||
let ts = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map_or(0, |d| d.as_nanos());
|
||||
let dir = std::env::temp_dir().join(format!("hive-forge-comments-test-{tag}-{ts}"));
|
||||
std::fs::create_dir_all(&dir).expect("create scratch state dir");
|
||||
std::fs::write(dir.join("forge-token"), "test-token\n").expect("seed forge-token");
|
||||
dir
|
||||
}
|
||||
|
||||
/// Answer one request. The comments route ignores the query string, as
|
||||
/// the real endpoint does; any route the verbs only read best-effort
|
||||
/// (notifications, reactions, PR reviews) gets an empty list.
|
||||
fn serve(mut stream: TcpStream) {
|
||||
let mut reader = BufReader::new(stream.try_clone().expect("clone stream"));
|
||||
let mut request_line = String::new();
|
||||
reader
|
||||
.read_line(&mut request_line)
|
||||
.expect("read request line");
|
||||
loop {
|
||||
let mut line = String::new();
|
||||
if reader.read_line(&mut line).expect("read header") == 0 || line == "\r\n" {
|
||||
break;
|
||||
}
|
||||
}
|
||||
let target = request_line.split_whitespace().nth(1).unwrap_or("");
|
||||
let path = target.split('?').next().unwrap_or("");
|
||||
let issue = format!("/api/v1/repos/o/r/issues/{NUMBER}");
|
||||
let body = match path {
|
||||
p if p == format!("{issue}/comments") => serde_json::Value::Array(
|
||||
(0..THREAD_LEN)
|
||||
.map(|i| {
|
||||
serde_json::json!({
|
||||
"id": i + 1,
|
||||
"body": format!("comment {i}"),
|
||||
"user": {"login": "someone"},
|
||||
"created_at": format!("2026-10-09T00:{:02}:00Z", i % 60),
|
||||
})
|
||||
})
|
||||
.collect(),
|
||||
),
|
||||
p if p == issue => serde_json::json!({
|
||||
"number": NUMBER.parse::<u64>().expect("numeric NUMBER"),
|
||||
"title": "a long thread",
|
||||
"body": "",
|
||||
"state": "open",
|
||||
"user": {"login": "someone"},
|
||||
}),
|
||||
_ => serde_json::json!([]),
|
||||
}
|
||||
.to_string();
|
||||
let response = format!(
|
||||
"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nX-Total-Count: {THREAD_LEN}\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}",
|
||||
body.len()
|
||||
);
|
||||
stream
|
||||
.write_all(response.as_bytes())
|
||||
.expect("write response");
|
||||
}
|
||||
|
||||
/// Start the mock forge on a loopback port; returns its base URL.
|
||||
fn mock_forge() -> String {
|
||||
let listener = TcpListener::bind("127.0.0.1:0").expect("bind mock forge");
|
||||
let addr = listener.local_addr().expect("mock forge addr");
|
||||
std::thread::spawn(move || {
|
||||
for stream in listener.incoming() {
|
||||
serve(stream.expect("accept"));
|
||||
}
|
||||
});
|
||||
format!("http://{addr}")
|
||||
}
|
||||
|
||||
/// Run `hive-forge -r o/r <args>` against a fresh mock forge; returns
|
||||
/// stdout, asserting the run succeeded.
|
||||
fn hive_forge(tag: &str, args: &[&str]) -> String {
|
||||
let out = Command::new(env!("CARGO_BIN_EXE_hive-forge"))
|
||||
.args(["-r", "o/r"])
|
||||
.args(args)
|
||||
.env("HIVE_FORGE_URL", mock_forge())
|
||||
.env("HYPERHIVE_STATE_DIR", scratch_state_dir(tag))
|
||||
.env_remove("HIVE_FORGE_TOKEN_FILE")
|
||||
.env_remove("HIVE_LABEL")
|
||||
.output()
|
||||
.expect("run hive-forge");
|
||||
let stdout = String::from_utf8_lossy(&out.stdout).into_owned();
|
||||
assert!(
|
||||
out.status.success(),
|
||||
"hive-forge {args:?} failed\nstdout:\n{stdout}\nstderr:\n{}",
|
||||
String::from_utf8_lossy(&out.stderr)
|
||||
);
|
||||
stdout
|
||||
}
|
||||
|
||||
/// Rendered comments: both verbs start each one with a bold `**` author.
|
||||
fn comment_count(stdout: &str) -> usize {
|
||||
stdout.lines().filter(|l| l.starts_with("**")).count()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn view_prints_each_comment_once() {
|
||||
let stdout = hive_forge("view", &["view", NUMBER]);
|
||||
assert_eq!(comment_count(&stdout), THREAD_LEN, "{stdout}");
|
||||
assert!(stdout.contains(&format!("## Comments ({THREAD_LEN})")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn comments_limit_returns_exactly_limit() {
|
||||
let stdout = hive_forge("limit", &["comments", NUMBER, "--limit", "5"]);
|
||||
assert_eq!(comment_count(&stdout), 5, "{stdout}");
|
||||
assert!(stdout.contains(&format!("({} more comment(s)", THREAD_LEN - 5)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn comments_tail_returns_the_last_n() {
|
||||
let stdout = hive_forge("tail", &["comments", NUMBER, "--tail", "3"]);
|
||||
assert_eq!(comment_count(&stdout), 3, "{stdout}");
|
||||
assert!(stdout.contains(&format!("comment {}", THREAD_LEN - 1)));
|
||||
assert!(!stdout.contains(&format!("comment {}", THREAD_LEN - 4)));
|
||||
assert!(stdout.contains(&format!("({} earlier comment(s)", THREAD_LEN - 3)));
|
||||
}
|
||||
Loading…
Reference in a new issue