util/config: assertOptionalStringArray '// istanbul ignore else' hides untested 'array contains non-strings' branch
还没有人认领这个 Issue。
评估
- 难度
- 2/5
- 预计耗时
- 1-3 小时
- 新手友好度
- 72/100
- Issue 类型
- 缺陷
- 描述清晰度
- 描述清楚
- 活跃度
- 冷清
- 技术栈
- typescript
- 领域
- testing-qa
调研方向
从 src/prompt-generators/util/config.ts 中的 assertOptionalStringArray 开始,然后检查 load-cli-config.test.ts 中相关的用例。运行针对性测试和覆盖率检查,添加一个包含非字符串项的数组用例,并验证 validator 和覆盖率报告会执行该分支,而不会使用无正当理由的 ignore 指令。
由索引模型根据 Issue 内容生成。
描述
Observed behavior
assertOptionalStringArray in src/prompt-generators/util/config.ts (lines 76-90) has an // istanbul ignore else directive immediately above its validation check:
export function assertOptionalStringArray(
value: Record<string, unknown>,
key: string,
field: string,
): void {
if (!(key in value)) {
return;
}
const array = value[key];
// istanbul ignore else
if (!Array.isArray(array) || array.some(item => typeof item !== 'string')) {
throw new Error(`${field} must be an array of strings`);
}
}
git blame traces the directive to commit 8f4d7d6 ("Push coverage to 100% using ignores"). The condition is a two-clause OR:
!Array.isArray(array)- covered by tests that pass a plain string for an array field (e.g.load-cli-config.test.ts:503,:720passcomponents: 'DOM: Workers'andlabels: 'bug').array.some(item => typeof item !== 'string')- I cannot find a single test in the repo that passes an array containing a non-string item to a field validated byassertOptionalStringArray. Searching for tests usingbugStatus,keywords,components,bugFields,bugSeverity,labels, andexcludePatternsonly turns up valid string arrays and "not even an array" failure cases.
The // istanbul ignore else is on the if statement, so strictly it suppresses the implicit "else" (no-throw) branch, not the OR's branch coverage. But it sits exactly where a branch is uncovered. Combined with the commit message, this strongly suggests the directive is silencing a real coverage gap: the array.some(...) arm of the OR has no test.
The runtime consequence is small but real: a regression that broke the per-item string check (for example, changing typeof item !== 'string' to typeof item === 'string', or accidentally removing the .some call) would not be caught by any existing test, and coverage would still report 100%.
Expected behavior
Either:
- Remove the
// istanbul ignore elsedirective and add a test that callsassertOptionalStringArray(or one of its callers, likenormalizeBugzillaTaskConfigwithbugStatus: ['OPEN', 42]) with an array containing a non-string item, so both arms of the OR are covered by an actual assertion, or - If the success path is the one that needs the suppression (because v8 is flagging it), explicitly justify that in a comment so future readers do not assume the directive is masking a different gap.
Minimal reproduction
import { assertOptionalStringArray } from './prompt-generators/util/config.js';
// Tests in the repo cover:
assertOptionalStringArray({ foo: 'bar' }, 'foo', 'test.foo'); // throws (first OR arm)
assertOptionalStringArray({ foo: ['a', 'b'] }, 'foo', 'test.foo'); // does not throw (else of if)
// Not covered anywhere:
assertOptionalStringArray({ foo: ['a', 42] }, 'foo', 'test.foo'); // throws (second OR arm)
Suggested fix
Remove the // istanbul ignore else line and add a unit test (or extend load-cli-config.test.ts) that exercises an array with at least one non-string element, e.g.:
it('should reject Bugzilla bugStatus arrays containing non-strings', async () => {
await expect(
normalizeCliConfig(
{
name: 'test',
agent: 'claude-sdk',
promptGenerator: [
'bugzilla',
{
search: { bugStatus: ['OPEN', 42] },
promptTemplate: 'Review {{id}}',
} as unknown as BugzillaTask,
],
},
join(configDir, 'config.json'),
),
).rejects.toThrow('bugzilla.search.bugStatus must be an array of strings');
});
Severity
S4 - cleanup with low behavioral risk. The validator works correctly today; the issue is that a future regression in the per-item type check would slip past both tests and the 100% coverage gate.
- 主要语言
- TypeScript
- 星标
- 2
- 派生
- 1
- PR 合并指标
- 30 天内没有已合并 PR
环境准备
这个项目没有提供开发容器、Dockerfile 或贡献指南,环境需要你自己搭建:先看它的 README,通用步骤见我们的新手贡献指南。
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
joewalker/loop-the-loop 的其他 Issue
-
bug S4
难度 2/5 1-3 小时 新手友好度 75/100
joewalker/loop-the-loop#88 ·
-
Git exec(): child killed by a signal rejects with new Error('') and loses the signal information未关闭bug S3
难度 2/5 1-3 小时 新手友好度 82/100
joewalker/loop-the-loop#84 ·
-
bug S3
难度 2/5 1-3 小时 新手友好度 78/100
joewalker/loop-the-loop#83 ·
-
bug S3
难度 2/5 1-3 小时 新手友好度 76/100
joewalker/loop-the-loop#82 ·
-
bug S4
难度 1/5 1 小时以内 新手友好度 90/100
joewalker/loop-the-loop#79 ·
查看 joewalker/loop-the-loop 的全部 Issue
相似的 Issue
-
bug ui-dashboard
难度 2/5 1-3 小时 新手友好度 78/100
维护者通常 1 天内回复
-
难度 1/5 1 小时以内 新手友好度 91/100
caelestia-dots/caelestia#534 · 1 条评论 ·
维护者通常 1 天内回复
-
area: scripts bug
难度 2/5 1-3 小时 新手友好度 76/100
tdk-landscape/tdk-cli-core#2706 ·
-
难度 2/5 1-3 小时 新手友好度 78/100
bobadilla-tech/biasmarket.com#402 ·
维护者通常 1 天内回复
-
bug good first issue
难度 2/5 1-3 小时 新手友好度 88/100
javvadivijayprasad/sel2pw#28 ·