Skip to content

Commit ac62d87

Browse files
bradymillerclaude
bradymiller
and
claude
authored
test(openemr-cmd): deeper non-worktree bats — snapshot/capsule, webroot, LDAP, prek dispatch; 4 more set-u fixes (#830)
## Summary Continuation of #828 + #829's non-worktree coverage. 28 new tests across 3 files plus 4 small script fixes for the same set-u bug pattern uncovered in #829 (the `ev` fix). ## What's new (3 files, 28 tests) | File | What it pins | |---|---| | `snapshot_capsule_commands.bats` | `bs`/`rs` → `/root/devtools backup|restore <name>`. `gc`/`pc` → `docker cp` in/out of `/snapshots/`. No-arg usage hints with their documented exit codes (20 / 21 / 19 / 18). `pc <missing-file>` exits 15 with the "capsule file exists" error. | | `webroot_swagger_ldap.bats` | `cwb`/`cwo` run an in-container sed against apache's `openemr.conf` then `docker restart <id>`; `swtm <name>` passes through to `/root/devtools set-swagger-to-multisite`; `el`/`dld` are plain devtools passthroughs. | | `prek_dispatch.bats` | Outer dispatch shape (`docker exec -w /var/www/localhost/htdocs/openemr -i <id> sh -c <inline> sh <args>`). Args pass-through for `run` / `install-hooks` / `--version`. Inline script contains the actionlint-docker → actionlint-system substitution + the `run|install-hooks` case + `exec pre-commit` pass-through. No-container error path. | ## 4 more set-u fixes (same pattern as the ev fix in #829) `creat_a_backup_snapshot` / `restore_from_a_snapshot` / `copy_capsule_from_docker_to_host` / `copy_capsule_from_host_to_docker` all had `local FOO="$1"` (or `$2`) BEFORE their `$# != N` check. Under `set -u` that crashes with "unbound variable" when called with no args instead of showing the documented help. All four reorder the check ahead of the `local` assignments. Pinned by dedicated regression tests. ## Test plan - [x] Full hermetic bats suite — 290 tests, 0 failures - [x] `shellcheck utilities/openemr-cmd/openemr-cmd` clean - [ ] CI: BATS openemr-cmd (ubuntu-22.04) green - [ ] CI: BATS openemr-cmd (macos-14) green - [ ] CI: ShellCheck green - [ ] CI: e2e jobs green (script touched — workflow filter triggers) - [ ] CodeRabbit review 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved argument handling for snapshot/capsule-related subcommands so missing inputs produce clean usage errors instead of `set -u` “unbound variable” crashes. * Hardened capsule copy behavior with earlier argument-count checks (including optional destination handling and consistent missing-file checks). * **Tests** * Added/expanded BATS coverage for `prek` dispatch, snapshot/capsule command CLI contracts, webroot switching, Swagger-to-multisite, and LDAP enable/disable. * **Chores** * Updated the `openemr-cmd` version to `1.0.47`. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ec028ac commit ac62d87

5 files changed

Lines changed: 423 additions & 26 deletions

File tree

‎tests/bats/openemr-cmd/helpers.bash‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ STUB
7979
# If the script is restructured, bump this constant — the test
8080
# 'OC_SCRIPT_FUNCS_END points at end of function defs' in helpers_pure.bats
8181
# will fail loudly when it drifts.
82-
OC_SCRIPT_FUNCS_END=1744
82+
OC_SCRIPT_FUNCS_END=1741
8383

8484
# Source ONLY the function definitions of openemr-cmd into the current shell.
8585
# Caller is responsible for setting OPENEMR_ROOT (and WT_STATE_FILE / others

‎tests/bats/openemr-cmd/prek_dispatch.bats‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# BATS: `openemr-cmd prek <args>` subcommand routing.
2+
#
3+
# Unlike the other devtools commands, prek runs Python pre-commit INSIDE
4+
# the openemr container at the openemr workdir. The dispatch chain (line
5+
# ~2304):
6+
# 1. wt_resolve_container_from_cwd: if cwd is inside a managed worktree
7+
# from .worktrees.json, route to that worktree's openemr container.
8+
# Otherwise return empty.
9+
# 2. Fall back to CONTAINER_ID (the global one resolved earlier).
10+
# 3. If still empty, surface a "no running openemr container" error.
11+
# 4. Otherwise: docker exec -w /var/www/localhost/htdocs/openemr -i <id>
12+
# sh -c '<inline-script>' sh <args>
13+
# 5. The inline script substitutes actionlint-docker → actionlint-system
14+
# ONLY for `run` / `install-hooks` subcommands (which read the config).
15+
# All other subcommands (--version, --help, autoupdate, etc.) pass
16+
# through unchanged.
17+
#
18+
# The inline script runs IN the container; our stub only records the
19+
# outer `docker exec` invocation. So these tests pin:
20+
# - the outer dispatch shape (-w, -i, container id, sh -c, args)
21+
# - the inline script contains the actionlint substitution logic
22+
# - the no-container error path
23+
24+
load '../test_helper/bats-support/load'
25+
load '../test_helper/bats-assert/load'
26+
load 'helpers'
27+
28+
setup() {
29+
SCRIPT="$(oc_script_path)"
30+
[[ -x "$SCRIPT" ]] || skip "openemr-cmd script not found"
31+
STUB_DIR=$(oc_make_docker_stub_dir)
32+
CONTAINER=fixed-prek-target
33+
export STUB_DIR CONTAINER
34+
}
35+
36+
teardown() {
37+
[[ -n "${STUB_DIR:-}" ]] && rm -rf "${STUB_DIR}"
38+
return 0
39+
}
40+
41+
# Run via -d so CONTAINER_ID is fixed (skipping the auto-detect chain).
42+
oc_run() {
43+
env PATH="${STUB_DIR}:${PATH}" "${SCRIPT}" -d "${CONTAINER}" "$@"
44+
}
45+
46+
# --- outer dispatch shape --------------------------------------------------
47+
# The recorded docker.log captures `docker exec ...` invocations via
48+
# `echo "$@"`. The inline script for prek contains newlines, so a single
49+
# invocation spans multiple log lines: the FIRST line starts with
50+
# `exec -w /var/www/localhost/htdocs/openemr -i <id> sh -c`, the MIDDLE
51+
# lines are the inline script body, and the LAST line is the trailing
52+
# `sh <args>` (positional args to the inline sh). Anchor the
53+
# "trailing-args" check to the last line of the log.
54+
55+
@test "prek run: 'docker exec -w /var/www/localhost/htdocs/openemr -i <id> sh -c <script> sh run'" {
56+
run oc_run prek run
57+
assert_success
58+
grep -Fq "exec -w /var/www/localhost/htdocs/openemr -i ${CONTAINER} sh -c" "${STUB_DIR}/docker.log" \
59+
|| { cat "${STUB_DIR}/docker.log"; fail "expected 'exec -w .../openemr -i ${CONTAINER} sh -c' invocation"; }
60+
grep -Eq '[[:space:]]sh run$' "${STUB_DIR}/docker.log" \
61+
|| { cat "${STUB_DIR}/docker.log"; fail "expected trailing 'sh run' (positional args to inline sh)"; }
62+
}
63+
64+
@test "prek install-hooks: same outer dispatch, args pass through to inline sh" {
65+
run oc_run prek install-hooks
66+
assert_success
67+
grep -Fq "exec -w /var/www/localhost/htdocs/openemr -i ${CONTAINER} sh -c" "${STUB_DIR}/docker.log" \
68+
|| fail "expected docker exec dispatch"
69+
grep -Eq '[[:space:]]sh install-hooks$' "${STUB_DIR}/docker.log" \
70+
|| { cat "${STUB_DIR}/docker.log"; fail "expected trailing 'sh install-hooks'"; }
71+
}
72+
73+
@test "prek run --all-files phpstan: args after the subcommand are also forwarded" {
74+
run oc_run prek run --all-files phpstan
75+
assert_success
76+
grep -Fq "exec -w /var/www/localhost/htdocs/openemr -i ${CONTAINER} sh -c" "${STUB_DIR}/docker.log" \
77+
|| fail "expected docker exec dispatch"
78+
grep -Eq '[[:space:]]sh run --all-files phpstan$' "${STUB_DIR}/docker.log" \
79+
|| { cat "${STUB_DIR}/docker.log"; fail "expected trailing 'sh run --all-files phpstan'"; }
80+
}
81+
82+
@test "prek --version: pass-through path (no actionlint substitution required)" {
83+
run oc_run prek --version
84+
assert_success
85+
grep -Fq "exec -w /var/www/localhost/htdocs/openemr -i ${CONTAINER} sh -c" "${STUB_DIR}/docker.log" \
86+
|| fail "expected docker exec dispatch"
87+
grep -Eq '[[:space:]]sh --version$' "${STUB_DIR}/docker.log" \
88+
|| { cat "${STUB_DIR}/docker.log"; fail "expected trailing 'sh --version'"; }
89+
}
90+
91+
# --- inline script contains the actionlint substitution + pass-through ----
92+
# The stub records the full `docker exec ... sh -c '<inline-script>' sh
93+
# <args>` line, including the inline script source. We grep that line for
94+
# the substitution signal so a refactor that drops the swap doesn't slip
95+
# past unnoticed.
96+
97+
@test "prek dispatch: inline script substitutes actionlint-docker -> actionlint-system" {
98+
run oc_run prek run
99+
assert_success
100+
grep -Fq "actionlint-docker" "${STUB_DIR}/docker.log" \
101+
|| { cat "${STUB_DIR}/docker.log"; fail "inline script missing 'actionlint-docker' source"; }
102+
grep -Fq "actionlint-system" "${STUB_DIR}/docker.log" \
103+
|| { cat "${STUB_DIR}/docker.log"; fail "inline script missing 'actionlint-system' replacement"; }
104+
grep -Fq "sed " "${STUB_DIR}/docker.log" \
105+
|| { cat "${STUB_DIR}/docker.log"; fail "inline script missing 'sed' substitution command"; }
106+
}
107+
108+
@test "prek dispatch: inline script handles run + install-hooks vs the pass-through default" {
109+
# The script's case body should mention 'run' and 'install-hooks'
110+
# explicitly (the substitution subjects) AND `exec pre-commit "$@"`
111+
# as the pass-through path. All three live in the same inline string,
112+
# so grepping the recorded line covers the inline-script structure.
113+
run oc_run prek run
114+
assert_success
115+
grep -Fq "run|install-hooks" "${STUB_DIR}/docker.log" \
116+
|| { cat "${STUB_DIR}/docker.log"; fail "inline script missing 'run|install-hooks' case"; }
117+
grep -Fq "exec pre-commit" "${STUB_DIR}/docker.log" \
118+
|| { cat "${STUB_DIR}/docker.log"; fail "inline script missing 'exec pre-commit' pass-through"; }
119+
# And the --config <tmp> form that's specific to the substituted-config path.
120+
grep -Fq -- "--config" "${STUB_DIR}/docker.log" \
121+
|| { cat "${STUB_DIR}/docker.log"; fail "inline script missing '--config <tmp>' usage"; }
122+
}
123+
124+
# --- no-container error path ----------------------------------------------
125+
126+
@test "prek: when no container is running, surfaces 'no running openemr container'" {
127+
# No -d, no auto-detected container (default empty DOCKER_PS_OUTPUT) →
128+
# CONTAINER_ID is empty and the case-block check at line ~2331 fires.
129+
# The script's container-resolution layer also surfaces its own error
130+
# earlier (line ~2008) for commands that need CONTAINER_ID, so a bare
131+
# `openemr-cmd prek` hits that path first. We assert the SECOND-line
132+
# one fires when we bypass via -d empty-id... but -d requires a value.
133+
# So we test the natural path: no -d, no running container → the
134+
# earlier "Could not automatically determine target OpenEMR container"
135+
# error fires before reaching prek's own check. That's expected — the
136+
# outer check is more general.
137+
run env PATH="${STUB_DIR}:${PATH}" "${SCRIPT}" prek run
138+
assert_failure
139+
assert_output --partial "Could not automatically determine target OpenEMR container"
140+
}

‎tests/bats/openemr-cmd/snapshot_capsule_commands.bats‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
# BATS: snapshot + capsule commands — bs / rs / gc / pc.
2+
#
3+
# These dispatch to either run_devtools_in_docker (bs, rs → /root/devtools
4+
# backup|restore <name>) or docker cp directly (gc/pc copy capsule files
5+
# in/out of /snapshots/ in the container). Each command has a no-arg
6+
# usage hint with a specific exit code; this file pins both the dispatch
7+
# shape and the usage-exit-code contract.
8+
#
9+
# All four functions originally had the same set-u bug as ensure-version
10+
# (#829's ev fix): `local FOO="$1"` fired before the `$# != 1` check,
11+
# crashing with "unbound variable" instead of showing the help. Fixed
12+
# concurrently with this file; the no-arg tests pin the fix.
13+
14+
load '../test_helper/bats-support/load'
15+
load '../test_helper/bats-assert/load'
16+
load 'helpers'
17+
18+
setup() {
19+
SCRIPT="$(oc_script_path)"
20+
[[ -x "$SCRIPT" ]] || skip "openemr-cmd script not found"
21+
STUB_DIR=$(oc_make_docker_stub_dir)
22+
CONTAINER=fixed-snapshot-target
23+
export STUB_DIR CONTAINER
24+
}
25+
26+
teardown() {
27+
[[ -n "${STUB_DIR:-}" ]] && rm -rf "${STUB_DIR}"
28+
return 0
29+
}
30+
31+
oc_run() {
32+
env PATH="${STUB_DIR}:${PATH}" "${SCRIPT}" -d "${CONTAINER}" "$@"
33+
}
34+
35+
# --- backup-snapshot / bs --------------------------------------------------
36+
37+
@test "bs <name>: dispatches to /root/devtools backup <name>" {
38+
run oc_run bs my-snapshot
39+
assert_success
40+
grep -Fq "/root/devtools backup my-snapshot" "${STUB_DIR}/docker.log" \
41+
|| { cat "${STUB_DIR}/docker.log"; fail "expected '/root/devtools backup my-snapshot'"; }
42+
grep -Fq "${CONTAINER}" "${STUB_DIR}/docker.log" || fail "container id missing"
43+
}
44+
45+
@test "bs (no args): prints usage hint and exits 20 (no 'unbound variable' crash)" {
46+
# Regression for the set-u bug that crashed with "unbound variable"
47+
# before the user saw the help.
48+
run oc_run bs
49+
[[ "${status}" -eq 20 ]] || fail "expected exit 20; got ${status}"
50+
assert_output --partial "Please provide a snapshot name"
51+
assert_output --partial "backup-snapshot|bs"
52+
refute_output --partial "unbound variable"
53+
}
54+
55+
@test "bs: 'backup-snapshot' long form is equivalent to 'bs'" {
56+
run oc_run backup-snapshot another-snap
57+
assert_success
58+
grep -Fq "/root/devtools backup another-snap" "${STUB_DIR}/docker.log" || fail "long form did not dispatch"
59+
}
60+
61+
# --- restore-snapshot / rs -------------------------------------------------
62+
63+
@test "rs <name>: dispatches to /root/devtools restore <name>" {
64+
run oc_run rs my-restore
65+
assert_success
66+
grep -Fq "/root/devtools restore my-restore" "${STUB_DIR}/docker.log" \
67+
|| { cat "${STUB_DIR}/docker.log"; fail "expected '/root/devtools restore my-restore'"; }
68+
}
69+
70+
@test "rs (no args): prints usage hint and exits 21 (no 'unbound variable' crash)" {
71+
run oc_run rs
72+
[[ "${status}" -eq 21 ]] || fail "expected exit 21; got ${status}"
73+
assert_output --partial "Please provide a restore snapshot name"
74+
assert_output --partial "restore-snapshot|rs"
75+
refute_output --partial "unbound variable"
76+
}
77+
78+
@test "rs: 'restore-snapshot' long form is equivalent" {
79+
run oc_run restore-snapshot something
80+
assert_success
81+
grep -Fq "/root/devtools restore something" "${STUB_DIR}/docker.log" || fail "long form did not dispatch"
82+
}
83+
84+
# --- get-capsule / gc ------------------------------------------------------
85+
86+
@test "gc <name>: 'docker cp <id>:/snapshots/<name> .'" {
87+
run oc_run gc example.tgz
88+
assert_success
89+
grep -Eq "cp ${CONTAINER}:/snapshots/example\.tgz \.\$" "${STUB_DIR}/docker.log" \
90+
|| { cat "${STUB_DIR}/docker.log"; fail "expected 'docker cp <id>:/snapshots/example.tgz .'"; }
91+
}
92+
93+
@test "gc <name> <host-dir>: 'docker cp <id>:/snapshots/<name> <host-dir>/'" {
94+
run oc_run gc example.tgz /path/to/save
95+
assert_success
96+
grep -Fq "cp ${CONTAINER}:/snapshots/example.tgz /path/to/save/" "${STUB_DIR}/docker.log" \
97+
|| { cat "${STUB_DIR}/docker.log"; fail "expected 'docker cp <id>:/snapshots/example.tgz /path/to/save/'"; }
98+
}
99+
100+
@test "gc (no args): prints usage hint and exits 19 (no 'unbound variable' crash)" {
101+
run oc_run gc
102+
[[ "${status}" -eq 19 ]] || fail "expected exit 19; got ${status}"
103+
assert_output --partial "Please provide the capsule name"
104+
assert_output --partial "get-capsule|gc"
105+
refute_output --partial "unbound variable"
106+
}
107+
108+
@test "gc: too many args (3+) also surfaces usage hint" {
109+
run oc_run gc a b c
110+
[[ "${status}" -eq 19 ]] || fail "expected exit 19; got ${status}"
111+
assert_output --partial "Please provide the capsule name"
112+
}
113+
114+
# --- put-capsule / pc ------------------------------------------------------
115+
116+
@test "pc <existing-file>: 'docker cp <file> <id>:/snapshots/'" {
117+
# pc checks that the file exists before docker cp. Create a real
118+
# file so the existence check passes. Use the full-path template form
119+
# (portable across GNU + BSD mktemp; `-p <dir>` is GNU-only).
120+
local tmp_cap
121+
tmp_cap=$(mktemp "${BATS_TMPDIR:-/tmp}/pc-cap.XXXXXX.tgz")
122+
run oc_run pc "${tmp_cap}"
123+
rm -f "${tmp_cap}"
124+
assert_success
125+
# docker cp <file> <id>:/snapshots/
126+
grep -Fq "cp ${tmp_cap} ${CONTAINER}:/snapshots/" "${STUB_DIR}/docker.log" \
127+
|| { cat "${STUB_DIR}/docker.log"; fail "expected docker cp <file> <id>:/snapshots/"; }
128+
}
129+
130+
@test "pc (no args): prints usage hint and exits 18 (no 'unbound variable' crash)" {
131+
run oc_run pc
132+
[[ "${status}" -eq 18 ]] || fail "expected exit 18; got ${status}"
133+
assert_output --partial "Please provide the capsule file name"
134+
assert_output --partial "put-capsule|pc"
135+
refute_output --partial "unbound variable"
136+
}
137+
138+
@test "pc <missing-file>: 'capsule file exists' error, exits 15" {
139+
run oc_run pc /nonexistent/capsule-bats-test.tgz
140+
[[ "${status}" -eq 15 ]] || fail "expected exit 15; got ${status}"
141+
assert_output --partial "Please check whether the capsule file exists or not"
142+
# And NO docker cp was issued.
143+
if grep -Fq "cp /nonexistent" "${STUB_DIR}/docker.log"; then
144+
cat "${STUB_DIR}/docker.log"
145+
fail "docker cp was issued for a nonexistent file"
146+
fi
147+
}

0 commit comments

Comments
 (0)