Replace module-level _test* seams with dependency injection
メンテナーはふだん 1 日以内に返信
まだ誰も着手していません。
評価
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 初心者へのやさしさ
- 25/100
- issue の種類
- リファクタリング
- 明瞭さ
- おおむね明確
- 活発さ
- 活発
- 技術スタック
- bun, node.js, typescript
調査の方向性
Start with the _testSetPlatform and _testSetExecSync seams: packages/config-resolver/src/index.ts, src/config/writer.ts, src/config/secret-store.ts and src/extension/store.ts, where process.platform is copied into a module-level let _platform. Find how each module is called and replace the seam with an injected deps parameter. Done when those two setters are gone, the tests pass under both Node and Bun, and the rg -n 'export function _test' count drops. The issue also asks for a policy line in AGENTS.md.
索引モデルが issue の本文から書いたものです。
説明
Summary
Production modules expose module-level _test* setters that tests use to swap out globals (stdin reader, TTY, execSync, process.platform, fetch, spawn, client singletons). Each one is a mutable module global plus a setter, and tests must restore it in finally. They also ship in the published package, because package.json files includes dist.
Counts (taken at the current main working copy):
rg -n 'export function _test' src packages/*/src→ 23 exported seams in 12 files:src/factory.ts:64,82,101(_testSetStdinReader,_testSetIsTTY,_testSetConfirmReader)_testSetExecSync×3:packages/config-resolver/src/index.ts:341,src/config/writer.ts:46,src/config/secret-store.ts:130_testSetPlatform×4: the same three files (:347,:53,:144) plussrc/extension/store.ts:34_testReset*×7: Es/Kibana/Cloud clients, config store, detection, resolvers, loose inline secret warning- extension:
runner.ts:32,installer.ts:52,60,store.ts:34,78
- Module-level
_testSetFetchexists only once (src/status/register.ts:43). The ES/Kibana/Cloud clients have_testSetFetchas an instance method (es-client.ts:151,kibana-client.ts:226,cloud-client.ts:92), not as an export. rg -l '_test[A-Z]' --glob '*.test.*' .→ 28 test files use them.rg -l '_testSet' distlists built.jsand.d.tsfiles, e.g.dist/factory.js,dist/config/secret-store.d.ts,dist/lib/cloud-client.d.ts. Your counts depend on how fresh your localdistis.rg -l 'mock\.(method|module|fn)' --glob '*.test.*' .→ 1 hit, and that hit is a comment.test/es/handler.test.ts:50explains that a hand-rolledspy()exists because "t.mock.fn()… is not implemented in Bun".
The four _testSetPlatform seams are needed because each module copies process.platform into a let _platform at module scope (e.g. packages/config-resolver/src/index.ts:133, src/extension/store.ts:31). Stubbing process.platform after import would not reach that copy.
Motivation
Every seam adds global mutable state. If a test forgets to restore one, the stub leaks into later tests in the same file. The _test* names also become de facto public API of the published package. Because package.json declares "bun": ">=1.2.0" in engines, swapping these seams for t.mock.method / mock.module from node:test is not a safe default. The repo already avoids t.mock.fn for Bun compatibility.
Proposal
- Policy: add no new
_test*exports. Use dependency injection (adepsparameter or constructor option), assrc/docs/*(deps.stdout) andsrc/es/handler(EsHandlerDeps) already do. - Migrate the highest-churn seams first:
_testSetExecSync: this goes away with the execFile migration._testSetPlatform: passplatformas a dep instead of a module-scope copy.fetch: one injection point in the shared client core (see the sibling client-core consolidation issue, #728).
- Once the remaining seams are migrated, strip leftover
_test*symbols from published types, e.g. with@internal+stripInternal.
Only use node:test mocking APIs where they are confirmed to work under both Node ≥22.1 and Bun ≥1.2.
Acceptance criteria
-
AGENTS.md(orCONTRIBUTING.md) states the "no new_test*exports" rule. -
_testSetExecSyncand_testSetPlatformare removed. Callers take injected deps, and tests pass on Node and Bun. -
rg -n 'export function _test' src packages/*/srccount goes down. Track it in the PR description. - No
_testSet*symbol appears indist/**/*.d.tsfor the migrated modules.
Related
- #728 (shared HTTP client core): the natural single
fetchinjection point - A sibling issue on shared test helpers (e.g. promoting
spy()fromtest/es/handler.test.ts)
authored by Pi Coding Agent (Claude Opus 5.5)
- 主要言語
- TypeScript
- スター
- 53
- フォーク
- 28
- 平均マージ
- 22時間 48分
- マージ済み PR(30日)
- 48
環境構築
- Dockerfile・Docker Compose ファイルなし
- プルリクエストのテンプレートなし
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
elastic/cli のほかの issue
-
tracking
難易度 2/5 1〜3時間 初心者へのやさしさ 77/100
メンテナーはふだん 1 日以内に返信
-
feat: ship skills/elastic/SKILL.md for agent invocation対応中かも @onatozmenn が 31 日前に担当しました。 オープンenhancement
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
メンテナーはふだん 1 日以内に返信
-
難易度 3/5 1〜2日 初心者へのやさしさ 63/100
メンテナーはふだん 1 日以内に返信
-
`es search --input-file` drops top-level `aggs` (and any unrecognised key) without an error対応中かも このイシューにリンクされたプルリクエストがオープン中、またはマージ済みです。 オープン
難易度 3/5 1〜2日 初心者へのやさしさ 55/100
メンテナーはふだん 1 日以内に返信
似ている 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 ·