feat(test): bats-core suite for cdcf_queue_worker.sh (Refs #63)#87
Merged
Conversation
Pure-mechanical extraction — no behaviour change. Every function moves verbatim into a sibling lib file the main script sources at startup. Two new pure helpers (parse_processed, parse_domain_count) replace inline python3 -c '...' heredocs that the callers previously embedded. A new should_run_daily_tasks function replaces the inline cadence check in the main loop (the original inner cadence check inside run_daily_tasks is retained for byte-equivalence). Sets up the helpers for unit testing with bats-core in scripts/tests/. Refs #63. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Refs #63. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Refs #63. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cases: redis EXISTS=1 (paused), EXISTS=0 (not paused), redis-cli fails (safe fall-back to not-paused so a Redis outage does not stall the worker indefinitely). Refs #63. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cases: truly empty (ZCARD=0+ZCOUNT=0), immediate work pending (ZCARD=3), delayed work ready (ZCOUNT=2), redis-cli failure (safe fall-back to HTTP poll so a Redis outage does not silently mask backlog). Uses the shim's multi-call mode (SHIM_REDIS_OUTPUTS + SHIM_REDIS_STATE) since queue_is_empty issues two ZCARD/ZCOUNT calls in one invocation. Refs #63. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cases: flat shape ({\"processed\": 5}), nested shape (the actual
production response shape — {\"processed\": {\"processed\": N}}),
missing key (defaults to 0), invalid JSON (\"error\"), empty input
(\"error\").
python3 is intentionally NOT shimmed — the real JSON parser is the
helper's actual dependency and the thing we want to exercise.
Refs #63.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cases: never-run sentinel (LAST_DAILY_RUN=0), within-interval (1s ago, 24h interval — should not fire), exact-boundary (delta == DAILY_INTERVAL — should fire, since the check is >=), short-interval override (DAILY_INTERVAL=1 with never-run). Refs #63. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cases: HTTP 200 + valid JSON (\"Processed N job(s)\"), HTTP 500 with HTML body (\"WARNING: HTTP 500\" with tags stripped), HTTP 000 / curl-level failure (\"WARNING: request failed\"), HTTP 200 + invalid JSON (\"WARNING: invalid JSON response\"). curl is PATH-shimmed; python3 is the real binary (so parse_processed exercises the actual JSON parser in the 200 + valid-JSON case). Refs #63. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Non-blocking job (continue-on-error: true) gated on changes to scripts/cdcf_queue_worker*, scripts/tests/**, or this workflow itself. Mirrors the gating pattern from test-nextjs.yml. Checks out the bats-core submodule recursively and verifies python3 is available (parse_processed tests exercise the real JSON parser). Refs #63. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Commands" entry Documents how to run the suite, the shim convention (PATH-injected fakes configured via SHIM_* env vars), how to add a new test, and how to bump the pinned bats-core tag. AGENTS.md / CLAUDE.md (symlink) now lists the bats invocation in the "Build & Development Commands" block alongside npm test, with a note that fresh clones need --recurse-submodules. Refs #63. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR refactors the queue worker script by extracting testable functions into a shared library, adds comprehensive Bash unit tests using Bats, and establishes CI automation. The main behavioral change is conditional gating of daily maintenance tasks. ChangesQueue Worker Testing and Refactoring
Sequence Diagram(s)sequenceDiagram
participant Test as Test File<br/>(*.bats)
participant Setup as setup() Hook
participant Shim as PATH-injected<br/>Shim
participant Lib as cdcf_queue_worker<br/>.lib.sh
participant LogOut as stdout<br/>(log/result)
Test->>Setup: Bats runs setup
Setup->>Setup: Prepend shims dir to PATH
Setup->>Setup: Set SHIM_* env vars
Setup->>Lib: source lib.sh
Test->>Lib: call library function<br/>(e.g., process_one)
Lib->>Shim: curl/redis-cli<br/>(mocked via PATH)
Shim-->>Lib: controlled output/exit code
Lib->>LogOut: log results
Lib-->>Test: return status
Test->>Test: assert stdout/status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/cdcf_queue_worker.lib.sh (1)
109-115: 💤 Low valueConsider using
-eqfor numeric comparison.Line 114 uses string comparison
=for an arithmetic result. While this works (arithmetic expansion produces canonical integer strings),-eqis more idiomatic for numeric comparisons in bash.♻️ More idiomatic numeric comparison
- [ $((immediate + delayed)) = "0" ] + [ $((immediate + delayed)) -eq 0 ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/cdcf_queue_worker.lib.sh` around lines 109 - 115, In queue_is_empty(), the final test uses a string comparison ([ $((immediate + delayed)) = "0" ]) which is non-idiomatic for numbers; change it to a numeric comparison (e.g., use [ $((immediate + delayed)) -eq 0 ] or (( immediate + delayed == 0 )) ) so the sum of immediate and delayed is compared as integers; update the test in the queue_is_empty function accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/tests/README.md`:
- Line 73: The fenced code block in scripts/tests/README.md (the file-layout
example using triple backticks) lacks a language tag and triggers markdownlint
MD040; update the opening fence from ``` to something like ```text (e.g., change
the opening fence that precedes the directory tree to ```text) so the block is
annotated and the linter warning is resolved.
---
Nitpick comments:
In `@scripts/cdcf_queue_worker.lib.sh`:
- Around line 109-115: In queue_is_empty(), the final test uses a string
comparison ([ $((immediate + delayed)) = "0" ]) which is non-idiomatic for
numbers; change it to a numeric comparison (e.g., use [ $((immediate + delayed))
-eq 0 ] or (( immediate + delayed == 0 )) ) so the sum of immediate and delayed
is compared as integers; update the test in the queue_is_empty function
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2bb47ee0-634c-4adc-a5a9-5a012beb5a2f
📒 Files selected for processing (14)
.github/workflows/test-worker.yml.gitmodulesAGENTS.mdscripts/cdcf_queue_worker.lib.shscripts/cdcf_queue_worker.shscripts/tests/README.mdscripts/tests/batsscripts/tests/helpers/shims/curlscripts/tests/helpers/shims/redis-cliscripts/tests/in_maintenance.batsscripts/tests/parse_processed.batsscripts/tests/process_one.batsscripts/tests/queue_is_empty.batsscripts/tests/should_run_daily_tasks.bats
- scripts/cdcf_queue_worker.lib.sh: queue_is_empty() now uses arithmetic evaluation `(( immediate + delayed == 0 ))` instead of the string-form `[ $((immediate + delayed)) = "0" ]`. Identical behaviour, idiomatic numeric comparison. - scripts/tests/README.md: file-layout fenced block annotated as `text` (was a bare ```), clearing markdownlint MD040. Bats suite re-run after both edits: 20/20 green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # AGENTS.md
scripts/tests/bats/ is the upstream bats-core git submodule — its markdown is the project's, not ours, and we shouldn't reformat or gate on it. Adding the path to both negative globs (the markdownlint- cli2 `#...` form and the prettier `!.../**` form) brings the runs down from a dozen-files-with-warnings to clean across the 18 .md files this repo actually owns. Also prettier-formatted scripts/tests/README.md and applied the markdownlint --fix for MD031 (blanks around fences) on it — that file was authored before the prettier/markdownlint tooling landed on main, so it didn't match the current style. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third of three planned test suites for #63 (following the Vitest pilot in #86 and parallel to the PHPUnit plan in
docs/superpowers/plans/2026-05-02-test-suites-phpunit.md).Summary
scripts/tests/bats/, pinned to v1.13.0 (latest stable; the plan suggested v1.11.0 but v1.13.0 is current as of plan execution).cdcf_queue_worker.shinto a siblingcdcf_queue_worker.lib.shthat the main script sources at startup. No runtime behaviour change — the inner cadence check insiderun_daily_taskswas retained alongside the new outershould_run_daily_tasksgate for strict byte-equivalence.in_maintenance.bats(3) — Redis key present / absent / redis-down fallbackqueue_is_empty.bats(4) — empty / immediate work / delayed work / redis-downparse_processed.bats(5) — flat shape / nested shape / missing key / invalid JSON / empty inputshould_run_daily_tasks.bats(4) — never-run sentinel / within-interval / exact-boundary / short-interval overrideprocess_one.bats(4) — HTTP 200+valid / HTTP 500 with HTML stripped / HTTP 000 connection failure / HTTP 200+invalid JSONredis-cliandcurlconfigured per test viaSHIM_*env vars; realpython3for the JSON parsing tests (so the actual parser is exercised, not a mock).continue-on-error: true) gated on changes toscripts/cdcf_queue_worker*,scripts/tests/**, or the workflow itself. Mirrors the gating pattern fromtest-nextjs.yml.AGENTS.mdupdated with the bats invocation under "Build & Development Commands";scripts/tests/README.mddocuments the shim convention and how to extend the suite.Plan deviations
setup()instead ofsetup_file()in every.batsfile.setup_file()runs in a different process where the sourced lib's functions are not visible to@testblocks (gotexit code 127on first run). Per-testsetup()runs in the same shell as the test bodies, so the helpers are reachable. Documented inscripts/tests/README.md.run_daily_tasks— the plan suggested moving the cadence check entirely to the main loop, but keeping the original inner guard alongside the new outershould_run_daily_tasksgate is strictly byte-equivalent (the inner check becomes a no-op when the outer gate has already let us through).Verification
scripts/tests/bats/bin/bats scripts/tests/exits 0 locally; 20/20 green.bash -nclean on both the refactoredcdcf_queue_worker.shand the newcdcf_queue_worker.lib.sh, plus both shims.shellcheckwas not available in the execution environment.parse_processed,parse_domain_count,run_daily_tasks,should_run_daily_tasks,in_maintenance,queue_is_empty,process_one.Post-merge step (manual)
The worker isn't deployed via the CI pipeline, so the operator needs to copy both
scripts/cdcf_queue_worker.shand the new siblingscripts/cdcf_queue_worker.lib.shonto the VPS at/usr/local/bin/, thensystemctl restart cdcf-queue-worker. The lib file is new — installing only the main script will leave systemd failing on thesourcecall.Verify by checking the unit is active and tailing its journal — expect output identical in shape to before (silent on empty queue, periodic transition lines on maintenance flips).
Refs #63. Related: #66 (
in_maintenance), #85 (queue_is_empty), #86 (Vitest pilot).Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests
Refactor