Skip to content

Fix: isolate cli_config_init_default via FLEDGE_CONFIG_DIR (#447 HOME-isolation) - #463

Merged
0xLeif merged 1 commit into
mainfrom
fix/447-test-home-isolation
Jul 2, 2026
Merged

Fix: isolate cli_config_init_default via FLEDGE_CONFIG_DIR (#447 HOME-isolation)#463
0xLeif merged 1 commit into
mainfrom
fix/447-test-home-isolation

Conversation

@0xLeif

@0xLeif 0xLeif commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • cli_config_init_default (tests/main.rs) previously ran fledge config init against the developer's real config path β€” it could write a real ~/.config/fledge/config.toml (or fail if one already existed) and asserted nothing (let _ = output.status;).
  • Rewrote it to be hermetic: point the child process at a fresh TempDir via the FLEDGE_CONFIG_DIR env var (honored by Config::config_path() in src/config.rs), then add real assertions β€” the command succeeds, prints a "Created" message, and actually writes config.toml into the isolated dir.
  • Setting the env var on the spawned child (not the test process) keeps this safe under parallel test execution; no serial guard needed.

Scope notes

  • This is the config-writing / HOME-isolation facet of Test infra: mocking harness + $HOME isolation + publish coverageΒ #447 only. The other config-writing integration tests (cli_config_set_and_get_roundtrip, cli_config_add_remove_list_key) already isolate via FLEDGE_CONFIG_DIR; the unknown-key tests fail before saving so they never write. The src/config.rs unit tests use in-memory TOML or explicit save_to(tempdir), so none touch the real path.
  • No production code changed.

Test Plan

  • cargo fmt --check clean
  • cargo clippy --bin fledge --tests -- -D warnings clean
  • cargo test --lib / --bin fledge (897 pass) and all integration binaries except spec pass; cli_config_init_default passes
  • Verified the two tests/spec.rs spec-check failures are pre-existing and environmental (installed specsync v4.6.0 emits a different spec check --json envelope) β€” they reproduce on the base branch via git stash and are unrelated to this change

πŸ€– Generated with Claude Code

https://claude.ai/code/session_016AvsKakjAc3EKN2ztcMfYi

Closes #447

The cli_config_init_default integration test invoked `fledge config init`
against the developer's real config path, letting it write (or fail on an
already-present) ~/.config/fledge/config.toml, and asserted nothing about
the outcome.

Point it at a fresh TempDir via the FLEDGE_CONFIG_DIR env var (honored by
Config::config_path()) so the run is hermetic, and add real assertions:
the command succeeds, prints a creation message, and writes config.toml
into the isolated dir. Setting the env var on the child process (not the
test process) keeps this safe under parallel test execution.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AvsKakjAc3EKN2ztcMfYi
@0xLeif
0xLeif requested a review from a team as a code owner July 2, 2026 15:05
@0xLeif
0xLeif requested review from 0xGaspar, Kyntrin and tofu-ux July 2, 2026 15:05

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the cli_config_init_default test in tests/main.rs to isolate its execution using a temporary directory and the FLEDGE_CONFIG_DIR environment variable. This prevents the test from modifying the developer's local configuration file during execution. The test now spawns a child process and asserts that the configuration is successfully initialized and written to the isolated directory. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@0xGaspar 0xGaspar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good hardening β€” the test previously wrote to the developer's real config path and asserted nothing; now it's hermetic via FLEDGE_CONFIG_DIR on the child process (parallel-safe, no serial guard needed) with real assertions on exit status, output, and the written file. Confirming the pre-existing tests/spec.rs failures reproduce on the base branch is the right diligence. LGTM.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

βœ… Corvin says...

      _
    <(^\  .oO(Caw! ^v^)
     |/(\
      \(\\
      " "\\

"Caw! Found a shiny new spec!"

CI Summary

Check Status
Dependency Audit βœ… Passed
Integration (3 OS) βœ… Passed
Lint (fmt + clippy) βœ… Passed
Spec Validation βœ… Passed
Tests (3 OS) βœ… Passed

Powered by corvid-pet

@0xLeif
0xLeif merged commit b333b5f into main Jul 2, 2026
13 checks passed
@0xLeif
0xLeif deleted the fix/447-test-home-isolation branch July 2, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test infra: mocking harness + $HOME isolation + publish coverage

2 participants