Skip to content

fix(ga-cli): propagate exit code when update steps fail#658

Open
Kailigithub wants to merge 1 commit into
lsdefine:mainfrom
Kailigithub:optimize/ga-update-fail-fast
Open

fix(ga-cli): propagate exit code when update steps fail#658
Kailigithub wants to merge 1 commit into
lsdefine:mainfrom
Kailigithub:optimize/ga-update-fail-fast

Conversation

@Kailigithub

Copy link
Copy Markdown
Contributor

Summary

ga update previously swallowed errors from git pull and pip install -e ., printed them to stdout, and then proceeded to run the next step. When git pull failed (network down, merge conflict, dirty tree, non-fast-forward), the command still ran pip install -e . on top of stale or conflicted source, silently corrupting the editable install and leaving the user with a ga that behaved unpredictably.

This change makes cmd_update fail fast:

  • Route git and pip stderr to sys.stderr (where it belongs).
  • sys.exit(rc) on either failure so the shell sees the non-zero status and the caller can detect the failed update.

Test plan

tests/test_ga_cli_update_fail_fast.py covers the three cases:

  1. git pull fails -> SystemExit(128), pip install never runs.
  2. git pull ok + pip install fails -> SystemExit(1), both steps called in order.
  3. Both ok -> no SystemExit, both steps called in order.

Verified via the three-step dance:

  • Tests fail on main (unfixed code does not raise SystemExit).
  • Tests pass on the fix branch (3/3).

Test patches builtins.print rather than the module's print binding, because print in ga_cli/cli.py resolves through the builtins namespace.

Diff scope

ga_cli/cli.py: +6 / -3 lines (one source file, focused on cmd_update).
tests/test_ga_cli_update_fail_fast.py: +121 lines (new test file).

Total: under 130 lines, one commit, one PR.

Behavioural impact

  • A failing git pull no longer cascades into a corrupted pip install. The shell prompt returns to the user with a non-zero exit code and the error visible on stderr.
  • A successful update is unchanged.
  • No new runtime dependencies; sys and subprocess are already used.

When `ga update` runs `git pull` and it fails (network down, merge
conflict, dirty tree, non-fast-forward), the old code printed the git
error to stdout and proceeded to `pip install -e .` on top of stale
or conflicted source. The next `ga` invocation would then run against
a corrupted editable install with unpredictable behaviour.

Make the command fail fast: route git and pip errors to stderr, and
`sys.exit(rc)` on either failure so the shell sees a non-zero status
and the caller knows the update was not applied.

Add a behavioural test in `tests/test_ga_cli_update_fail_fast.py`:
- `git pull` fails -> SystemExit(128), pip never runs
- `git pull` ok + `pip install` fails -> SystemExit(1), both steps called
- both ok -> no SystemExit, both steps called

Test patches `builtins.print` (not the module-level binding) because
`print` in the module resolves through the builtins namespace.
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.

1 participant