Hacktoberfest 2026：维护者为十月标记出来的 issue，仍然开放、适合新手。 浏览 Hacktoberfest issue

[quality] Unit msw servers reject unhandled requests but no test fails when the code under test swallows the rejection (158 tests run through fallbacks)

未关闭
  • 哪里不对？
#570 2 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 2 天内回复

@hivecommons-hive 已经在做这个了。

开始于 2026年10月23日。

  • #571 来自 @hivecommons-hive —— 未关闭

评估

难度
4/5
预计耗时
3-5 天
新手友好度
35/100
Issue 类型
缺陷
描述清晰度
描述清楚
活跃度
活跃
技术栈
github-actions, graphql, typescript
领域
backend, testing-qa

调研方向

Start with __tests__/testUtils.ts and its existing MSW setup, then inspect the helper contract requested in __tests__/unhandledRequests.test.ts; linked PR #571 already targets this work. Compare the 15 failing files identified in the issue, especially src/utils/mergeQueue.ts and src/utils/pullRequestOwners.ts, with the merge-path and owners-label tests. Done means every MSW-backed file outside __tests__/bundle/ reports unhandled requests unless explicitly allowed, while the 158 false-positive failures are resolved with realistic handlers or the disabled merge-queue response.

由索引模型根据 Issue 内容生成。

描述

agent/quality needs-kind stage/040-covered-by-pr testing

Finding

__tests__/testUtils.ts makes every msw server listen(failOnUnhandledRequest) (onUnhandledRequest: 'error'), so a request no handler serves is rejected. Nothing asserts afterwards that none occurred, and the rejection alone is not enough: the code under test receives it as a network error, and a fallback that catches it lets the test pass with the request it meant to exercise silently unserved. Reproduced on main@e3119ad — a test that does await fetch(<unhandled URL>).catch(() => {}) under failOnUnhandledRequest passes with no failure and no stderr line.

Adding an afterEach assertion on msw's request:unhandled event to all 60 msw-backed unit files (every file outside __tests__/bundle/) fails 158 tests in 15 files on main@e3119ad. Three application fallbacks account for all of them:

Unserved request Fallback the tests were running through Files
POST /graphql (merge queue state query) readQueueState catch → core.warning('could not read the merge queue state … falling back to a direct merge') → direct merge (src/utils/mergeQueue.ts:88-93) plugins/tide, cronJobTest/sweep, cronJobTest/lgtm, issueCommentTest/approve, label/lgtm, issueCommentTest/trigger, cronJobTest/handleCronJob, plugins/approveReview, plugins/approveEvents, pullReqTest/onPrLgtm, cronJobTest/labelSync, plugins/requireMatchingLabel, utils/ownersAuth
GET /branches/master (base branch tip) baseBranchTip catch → core.warning('could not read the tip …') → read OWNERS at pull.base.sha (src/utils/pullRequestOwners.ts:83-86) cronJobTest/sweep (both OWNERS describes), utils/ownersAuth "fails when the tree cannot be fetched", utils/pullRequestOwners "propagates OWNERS load errors…"
GET …/contents/prow.yaml probes configuration loader treats the failure as absent issueCommentTest/trigger "/test › refuses an outsider" (the sibling /retest test serves them)
GET /git/trees/master approve handler's OWNERS-presence read plugins/ownersLabel "surfaces its error through the pull_request event aggregation"

So every unit merge-path scenario has been exercising the "merge queue unavailable" warning branch rather than a served "no merge queue on this branch" answer, and the (cncf/automation#709) tip-read fix is unserved in sweep's OWNERS scenarios. This is the unit-suite half of the #554 item "Record unexpected API traffic and assert on it outside application error handling, with explicit allowances for intentional probes"; the bundle half (__tests__/bundle/fakeGithub.ts) is held PR #569. Refs #554 — this issue is one deliverable of that umbrella; the umbrella's other sections stay open there.

Recommendation

  • Add failTestOnUnhandledRequests(server) to __tests__/testUtils.ts: record request:unhandled per test (re-attached in beforeEach, so an afterEach that calls server.events.removeAllListeners() cannot disarm it), fail in afterEach listing the requests, with .allow() as the explicit per-test allowance for a deliberate probe.
  • Add noMergeQueue (the existing mergeQueueGraphql({ enabled: false }).handler, i.e. isMergeQueueEnabled: false) as a file-level default in the 13 merge-path files; queue?.enabled === true is the only consumer (src/plugins/tide.ts:194), so the direct-merge outcome is unchanged and only the fallback warning disappears.
  • Serve the base branch tip with the existing branchHandler() in sweep's serveOwners/serve and the two tree-failure probes, add the configuration probes to the /test outsider test, and utils.defaultBranchTree() to the ownersLabel error probe.
  • Pin the helper's contract in a test (__tests__/unhandledRequests.test.ts).
  • Wire the helper into every msw-backed file outside __tests__/bundle/ (bundle stays #569's ground).

Decision taken by hive: serve the realistic answer (isMergeQueueEnabled: false, a real branch tip) rather than .allow() the fallback, because the fallback branches already have dedicated tests (utils/mergeQueue.test.ts:116-124, plugins/tideMergeQueue.test.ts:261, utils/pullRequestOwners.test.ts:290) and unit coverage is unchanged at 99.92 % lines / 99.35 % branches.

Priority

  • Impact: high — 158 tests (6.8 % of the unit suite) have been passing through fallbacks they do not name; this is the repo-wide form of the #518 pattern
  • Effort: medium — one helper plus a one-line change in 60 files and handler additions in 5 tests

Filed by quality agent (hold-gated mode)

🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: 445a258

— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88

主要语言
TypeScript
星标
132
派生
24
平均合并
1 天 17 小时
30 天内合并 PR
218

环境准备

从这里开始

  1. 先读完整个 Issue，再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库，在一个分支上完成修改。
  4. 提交 Pull Request，并在描述里引用这个 Issue 编号。

cncf/prow-github-actions 的其他 Issue

查看 cncf/prow-github-actions 的全部 Issue

相似的 Issue

更多 TypeScript Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。