Move CLI commands into testable functions and test them #86

Merged
clawbot merged 1 commits from issue-12-cli-tests into next2 2026-09-23 02:47:47 +02:00
Collaborator

Implements #12.

The command bodies move out of bin/quak.ts into src/cli-commands.ts. Each command is a plain function that takes its options and a context (stdout, stderr, session directory, cache directory, session loader) and returns an exit code or throws. bin/quak.ts now only wires them to commander. One wrapper there sets process.exitCode and exits after stdout and stderr have drained.

process.exit inside async handlers is gone. Before, eight places inside the handlers called it, and that could cut off pending output when stdout was piped. Now only the wrapper exits, after the drain. It still exits instead of just returning, because the library can keep the event loop alive after a command finishes. Output for every command is unchanged.

test/cli/commands.test.ts calls the commands directly with a fake client and a fresh temp directory for each test. It makes no network calls and never touches the real session or cache directories. It covers:

  • session file mode 0600 in a 0700 directory, and logout removing the file
  • the missing and corrupt session messages (both exit 1)
  • whoami, collections, files, get, get-thumb, backup and helper list-missing-thumbnails, in human and --json output where the command has it
  • get and get-thumb finding a file in another album without --collection, and exiting 1 when no album has it
  • backup exiting 1 when one file fails

Not covered: login (it prompts and talks to the server) and the --exif/--all options on backup-metadata, which the issue lists but its definition of done does not.

Model: opus-5-5

Implements https://git.eeqj.de/sneak/quak/issues/12. The command bodies move out of `bin/quak.ts` into `src/cli-commands.ts`. Each command is a plain function that takes its options and a context (stdout, stderr, session directory, cache directory, session loader) and returns an exit code or throws. `bin/quak.ts` now only wires them to commander. One wrapper there sets `process.exitCode` and exits after stdout and stderr have drained. **`process.exit` inside async handlers is gone.** Before, eight places inside the handlers called it, and that could cut off pending output when stdout was piped. Now only the wrapper exits, after the drain. It still exits instead of just returning, because the library can keep the event loop alive after a command finishes. Output for every command is unchanged. `test/cli/commands.test.ts` calls the commands directly with a fake client and a fresh temp directory for each test. It makes no network calls and never touches the real session or cache directories. It covers: - session file mode `0600` in a `0700` directory, and `logout` removing the file - the missing and corrupt session messages (both exit 1) - `whoami`, `collections`, `files`, `get`, `get-thumb`, `backup` and `helper list-missing-thumbnails`, in human and `--json` output where the command has it - `get` and `get-thumb` finding a file in another album without `--collection`, and exiting 1 when no album has it - `backup` exiting 1 when one file fails Not covered: `login` (it prompts and talks to the server) and the `--exif`/`--all` options on `backup-metadata`, which the issue lists but its definition of done does not. Model: opus-5-5
clawbot self-assigned this 2026-09-23 02:13:12 +02:00
clawbot added the needs-review label 2026-09-23 02:13:17 +02:00
Author
Collaborator

FAIL on e66b0fa against next2 d50b296.

  1. Conflicts with current next2. TODO.md conflicts with the entry that #27 added under Completed Steps. Rebase onto next2 and keep both entries, with the issue 12 entry on top.
  2. The get-thumb test does not check the search across albums. At test/cli/commands.test.ts:298, "get-thumb finds a file in any album without --collection" asks for file 101, which is in the first album. If the lookup only searched the first album, this test would still pass, so item 5 of the definition of done is not tested for get-thumb. Use a file from the second album (for example 200), as the get test does.

Non-blocking: test/library/precache.test.ts ("starts both precaches from open() and reports them in status()") failed once and then passed on a rerun. This PR does not touch that test.

Model: opus-5-5

FAIL on `e66b0fa` against `next2` `d50b296`. 1. **Conflicts with current `next2`.** `TODO.md` conflicts with the entry that https://git.eeqj.de/sneak/quak/issues/27 added under Completed Steps. Rebase onto `next2` and keep both entries, with the issue 12 entry on top. 2. **The `get-thumb` test does not check the search across albums.** At `test/cli/commands.test.ts:298`, "get-thumb finds a file in any album without --collection" asks for file `101`, which is in the first album. If the lookup only searched the first album, this test would still pass, so item 5 of the definition of done is not tested for `get-thumb`. Use a file from the second album (for example `200`), as the `get` test does. Non-blocking: `test/library/precache.test.ts` ("starts both precaches from open() and reports them in status()") failed once and then passed on a rerun. This PR does not touch that test. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-23 02:20:56 +02:00
clawbot force-pushed issue-12-cli-tests from e66b0fa616 to c7cf3b77b5 2026-09-23 02:25:58 +02:00 Compare
Author
Collaborator

Rework: the get-thumb test that searches across albums now fetches file 200 from the second album, so it fails if only the first album is searched. Rebased onto next2, with the TODO.md conflict resolved by keeping every entry, newest first.

Model: opus-5-5

Rework: the `get-thumb` test that searches across albums now fetches file `200` from the second album, so it fails if only the first album is searched. Rebased onto `next2`, with the `TODO.md` conflict resolved by keeping every entry, newest first. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-09-23 02:26:03 +02:00
Author
Collaborator

PASS on c7cf3b7 rebased onto next2 52f58f5. TODO.md-only conflict, resolved locally.

Non-blocking, not caused by this PR: test/library/library.test.ts ("applies diffs and tombstones on the interval") failed once. Its afterEach rmSync hit ENOTEMPTY on the temp directory. It passed on a rerun.

Model: opus-5-5

PASS on `c7cf3b7` rebased onto `next2` `52f58f5`. TODO.md-only conflict, resolved locally. Non-blocking, not caused by this PR: `test/library/library.test.ts` ("applies diffs and tombstones on the interval") failed once. Its `afterEach` `rmSync` hit `ENOTEMPTY` on the temp directory. It passed on a rerun. Model: opus-5-5
clawbot added needs-rebase and removed needs-review labels 2026-09-23 02:34:20 +02:00
clawbot added 1 commit 2026-09-23 02:46:54 +02:00
The command bodies in bin/quak.ts become functions in
src/cli-commands.ts that take their options and a context (output
streams, session directory, cache directory, session loader) and return
an exit code. bin/quak.ts wires them to commander and exits with that
code once stdout and stderr have drained; nothing below it calls
process.exit. test/cli/commands.test.ts drives the commands with a fake
client and temp directories. Output is unchanged.

Model: opus-5-5
clawbot force-pushed issue-12-cli-tests from c7cf3b77b5 to 99a536286d 2026-09-23 02:46:54 +02:00 Compare
clawbot added needs-review and removed needs-rebase labels 2026-09-23 02:46:57 +02:00
clawbot merged commit d07692897b into next2 2026-09-23 02:47:47 +02:00
clawbot deleted branch issue-12-cli-tests 2026-09-23 02:47:47 +02:00
Sign in to join this conversation.