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

util/config: assertOptionalStringArray '// istanbul ignore else' hides untested 'array contains non-strings' branch

未关闭 适合新手
#68 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 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 内容生成。

描述

bug S4

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:

  1. !Array.isArray(array) - covered by tests that pass a plain string for an array field (e.g. load-cli-config.test.ts:503, :720 pass components: 'DOM: Workers' and labels: 'bug').
  2. 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 by assertOptionalStringArray. Searching for tests using bugStatus, keywords, components, bugFields, bugSeverity, labels, and excludePatterns only 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 else directive and add a test that calls assertOptionalStringArray (or one of its callers, like normalizeBugzillaTaskConfig with bugStatus: ['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,通用步骤见我们的新手贡献指南。

从这里开始

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

joewalker/loop-the-loop 的其他 Issue

查看 joewalker/loop-the-loop 的全部 Issue

相似的 Issue

更多 TypeScript Issue

把新 issue 发到你的邮箱

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