This PR fixed the test's real defect -- it compared the child's answer against the PARENT process's getGlobalConfigDir(), two different environments, agreeing only because the child inherited the developer's ambient CLAUDE_CONFIG_DIR -- but fixed it by INJECTING CLAUDE_CONFIG_DIR, which moved the test onto the env-first branch and silently dropped the only coverage #2003 had of the home-derived fallback. Sandbox HOME/USERPROFILE and leave the config vars blank instead: the expectation stays test-controlled AND the branch under test is unchanged. Drops the notStrictEqual against the codex dir. It could not fail whenever the strictEqual on the line above passed. Addresses review finding: Minor 7.
This commit is contained in:
@@ -2108,24 +2108,28 @@ describe('regressions: --runtime override bypasses persisted runtime (#2003)', (
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-rt-cli-'));
|
||||
try {
|
||||
writePersistedRuntime(tmpDir, 'codex');
|
||||
// #2665: redirect both runtime homes into the sandbox so the expectation is
|
||||
// built from values this test controls. Comparing the child's answer against
|
||||
// the PARENT process's getGlobalConfigDir() compared two different
|
||||
// environments -- it agreed only because the child inherited the developer's
|
||||
// ambient CLAUDE_CONFIG_DIR, which is the leak this test's helper now blocks.
|
||||
const claudeConfigDir = path.join(tmpDir, 'claude-config');
|
||||
const codexHome = path.join(tmpDir, 'codex-home');
|
||||
// #2665: sandbox HOME and let the config-location vars stay BLANK (helpers'
|
||||
// TEST_ENV_BASE zeroes them), so the child resolves through the home-derived
|
||||
// FALLBACK branch of getGlobalConfigDir. The original test compared the
|
||||
// child's answer against the PARENT process's getGlobalConfigDir() -- two
|
||||
// different environments, agreeing only because the child inherited the
|
||||
// developer's ambient CLAUDE_CONFIG_DIR.
|
||||
//
|
||||
// Injecting CLAUDE_CONFIG_DIR here instead would fix that leak but move the
|
||||
// test onto the env-FIRST branch, silently dropping the only coverage this
|
||||
// #2003 regression has of the fallback branch. Sandboxing HOME keeps the
|
||||
// expectation test-controlled AND keeps the branch under test unchanged.
|
||||
const result = runGsdTools('capability state --runtime claude --raw', tmpDir, {
|
||||
HOME: tmpDir,
|
||||
CLAUDE_CONFIG_DIR: claudeConfigDir,
|
||||
CODEX_HOME: codexHome,
|
||||
USERPROFILE: tmpDir,
|
||||
});
|
||||
assert.ok(result.success, `capability state --runtime should succeed: ${result.error || ''}`);
|
||||
const parsed = JSON.parse(result.output);
|
||||
assert.strictEqual(parsed.runtimeConfigDir, claudeConfigDir,
|
||||
// Home-derived Claude dir. That this is NOT <tmpDir>/.codex is the #2003
|
||||
// assertion; a separate notStrictEqual against the codex dir would be dead,
|
||||
// since it cannot fail whenever this strictEqual passes.
|
||||
assert.strictEqual(parsed.runtimeConfigDir, path.join(tmpDir, '.claude'),
|
||||
'`capability state --runtime claude` must resolve to the Claude config dir, not the persisted codex dir');
|
||||
assert.notStrictEqual(parsed.runtimeConfigDir, codexHome,
|
||||
'must NOT resolve to the codex config dir when --runtime claude is explicit');
|
||||
} finally {
|
||||
cleanup(tmpDir);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user