Skip to content

test(miner-manage): cover ci-poller transient-failure edge cases - #4356

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
glorydavid03023:test/miner-ci-poller-failure-modes
Jul 9, 2026
Merged

test(miner-manage): cover ci-poller transient-failure edge cases#4356
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
glorydavid03023:test/miner-ci-poller-failure-modes

Conversation

@glorydavid03023

Copy link
Copy Markdown
Contributor

Summary

Closes #4281.

test/unit/miner-ci-poller.test.ts (#2323) already covers pollCheckRuns thoroughly (13 cases). This adds the three transient-failure modes it does not exercise, using the existing injected fetchFn/sleepFn style (no real network):

  1. Rate-limit / non-OK response. A 403/429 propagates as a github_403/github_429 error and aborts the poll on attempt 1 — pollCheckRuns' attempt loop has no try/catch around the fetches, so it does not consume a backoff attempt or retry through the remaining maxAttempts (asserted: sleepFn never called, only the head-SHA fetch issued).
  2. fetchFn promise rejection (network timeout/abort) during the PR head-SHA fetch and during the check-runs fetch — propagates a clear error rather than hanging or silently swallowing.
  3. Mid-pagination partial failure — page 1 of check-runs succeeds (with a rel="next" Link header) but page 2 fails; fetchCheckRuns has no per-page retry, so the error propagates deterministically out of the whole poll.

These pin the current behavior (a single transient failure kills the poll with no retry). Per the issue's guidance, I did not change that behavior under a test-prefixed PR: if the poller should instead retry specifically on a 429, that is worth a separate feat/fix — flagging it here rather than silently altering it.

Scope

Validation

  • git diff --check
  • npm run typecheck (green)
  • test/unit/miner-ci-poller-failure-modes.test.ts — 5 assertions (403 + 429 via it.each, head-SHA-fetch rejection, check-runs-fetch rejection, mid-pagination page-2 failure) all pass.

If any required check was skipped, explain why:

  • The full npm run test:ci was not run locally (several self-host suites require Linux/bash/Docker/Postgres and fail only on Windows). This PR is test-only — it adds one test/unit/*.test.ts file and changes no src//package source, so there is no runtime, generated-artifact, or Codecov-patch surface; typecheck and the new tests are green, and the remaining CI runs on this PR.

Safety

  • No secrets, wallets, hotkeys, coldkeys, PATs, private keys, trust scores, private rankings, or maintainer evidence are exposed.
  • Public GitHub text stays sanitized and low-noise.
  • No auth/cookie/CORS/GitHub App/Cloudflare/session changes.
  • No API/OpenAPI/MCP behavior changes.

UI Evidence

N/A - test-only change; no UI, frontend, docs, or extension change.

Extends test/unit/miner-ci-poller.test.ts (JSONbored#2323) with the three transient-failure modes it did not exercise, per JSONbored#4281: (1) a 403/429 response propagates as github_<status> and aborts the poll without consuming a backoff attempt or retrying through the remaining maxAttempts; (2) a fetchFn promise rejection (network timeout/abort) during the PR head-SHA fetch and during the check-runs fetch propagates a clear error rather than hanging or swallowing; (3) page 1 of check-runs succeeding but page 2 failing mid-pagination aborts deterministically (fetchCheckRuns has no per-page retry). Test-only, injected fetchFn/sleepFn, no real network. These pin the current no-retry/no-swallow behavior; if the poll should instead retry on 429, that is a separate feat/fix intentionally not made here. Closes JSONbored#4281.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.95%. Comparing base (4d0aef1) to head (69ed8b7).
⚠️ Report is 50 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4356   +/-   ##
=======================================
  Coverage   93.95%   93.95%           
=======================================
  Files         399      399           
  Lines       36782    36782           
  Branches    13436    13436           
=======================================
  Hits        34557    34557           
  Misses       1569     1569           
  Partials      656      656           
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 9, 2026
@loopover-orb

loopover-orb Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-09 17:14:57 UTC

1 file · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a narrow, test-only PR that adds three transient-failure-mode tests (403/429 rate-limit, fetchFn promise rejection during head-SHA and check-runs fetches, and mid-pagination failure) to `pollCheckRuns`, using the same `fetchFn`/`sleepFn` injection style as the existing `miner-ci-poller.test.ts`. It links an eligible open issue (#4281), makes no production code changes, and CI (including codecov/patch and codecov/project) is green on this exact commit. The mid-pagination test's `url.endsWith("page=1")` branch check would have caused the call-count assertion (`toHaveBeenCalledTimes(3)`) to fail if it didn't match the real URL shape, so the green CI result corroborates the test exercises the real pagination path rather than a fabricated one.

Nits — 5 non-blocking
  • test/unit/miner-ci-poller-failure-modes.test.ts: the `sleepFn`/`fetchFn` `vi.fn()` boilerplate is repeated near-identically across all four test cases — consider a small `makeFakes()` helper to reduce duplication.
  • test/unit/miner-ci-poller-failure-modes.test.ts: the multi-line block comment above `describe` cites specific line numbers (`ci-poller.js:200-225`) that will silently go stale if the poller is refactored — consider referencing the function/behavior instead of exact line numbers.
  • test/unit/miner-ci-poller-failure-modes.test.ts: the 403/429 case only asserts `.code` via `toMatchObject`; consider also asserting the error message/status is surfaced if `pollCheckRuns` attaches one, to make the pinned contract more complete.
  • Since the PR explicitly flags that a 429 arguably deserves a retry rather than immediate abort, consider filing (or confirming) a follow-up issue now so that intent isn't lost — the description says it's 'worth a separate feat/fix' but doesn't link one.
  • test/unit/miner-ci-poller-failure-modes.test.ts: extracting the shared `sleepFn`/assertion pattern (call counts + `sleepFn` not called) into a tiny shared assertion helper would tighten the four cases and make future additions (e.g. a 5xx case) cheaper to add.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4281
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 256 registered-repo PR(s), 155 merged, 18 issue(s).
Contributor context ✅ Confirmed Gittensor contributor glorydavid03023; Gittensor profile; 256 PR(s), 18 issue(s).
Gate result ✅ Passing No configured blocker found.
Linked issue satisfaction

Addressed
The added test file directly implements all three deliverables from the issue—403/429 non-retry propagation, fetchFn rejection during both head-SHA and check-runs fetches, and mid-pagination page-2 failure—using the existing injected fetchFn/sleepFn style without real network calls, and the PR description explicitly notes the current no-retry behavior is being pinned rather than silently changed.

Review context
  • Author: glorydavid03023
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, JavaScript, TypeScript, Rust, C++, Kotlin, MDX, Ruby
  • Official Gittensor activity: 256 PR(s), 18 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 730f52e into JSONbored:main Jul 9, 2026
9 checks passed
@loopover-orb loopover-orb Bot added gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. and removed gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. labels Jul 9, 2026
@JSONbored JSONbored added gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. and removed gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(miner-manage): regression coverage for the CI/gate polling backoff edge cases

2 participants