Skip to content

Test harness.#21

Merged
wpak-ai merged 8 commits into
cppalliance:masterfrom
AuraMindNest:feature/test
Jun 17, 2026
Merged

Test harness.#21
wpak-ai merged 8 commits into
cppalliance:masterfrom
AuraMindNest:feature/test

Conversation

@AuraMindNest

@AuraMindNest AuraMindNest commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Close #19.

Summary

Adds a bats test suite for start-translation workflow logic, with mock gh and git fixtures so destructive mirror operations can be tested safely without hitting real GitHub.

  • Extract translation helpers from start-translation.yml into translation.sh
  • Add 22 bats tests for lib helpers, merge guards, and submodule processing
  • Add make lint, make test, and make check; optional pre-commit hook
  • Run tests in CI via a new test job in lint.yml

Test plan

  • make test passes
  • make lint passes
  • CI lint and test jobs pass

Summary by CodeRabbit

  • New Features

    • Added automated code quality checks (linting and testing) for development
    • Enhanced translation workflow automation for documentation synchronization
  • Documentation

    • New Development section in README with local setup instructions and available build commands
  • Tests

    • Introduced comprehensive test suite with fixtures and helpers for validating core functionality

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AuraMindNest, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 58 minutes and 26 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bc05d6d9-c0a6-4d69-9e16-53fc41ea1dda

📥 Commits

Reviewing files that changed from the base of the PR and between 1bfb25d and 3b1aca0.

📒 Files selected for processing (3)
  • scripts/test.sh
  • tests/helpers/common.bash
  • tests/helpers/git_fixtures.bash
📝 Walkthrough

Walkthrough

Inline translation workflow helpers are extracted from start-translation.yml into .github/workflows/assets/translation.sh. Two new functions (has_open_translation_pr, parse_extensions) are added to lib.sh. A local dev toolchain is introduced with a Makefile, lint/test/pre-commit scripts, and git hooks. A Bats test suite is added covering lib helpers, merge-guard behavior, and submodule orchestration.

Changes

Translation Logic Extraction + Dev Toolchain + Tests

Layer / File(s) Summary
Translation orchestration helpers and lib.sh additions
.github/workflows/assets/lib.sh, .github/workflows/assets/translation.sh, .github/workflows/start-translation.yml
Adds has_open_translation_pr and parse_extensions to lib.sh; introduces translation.sh with sync_repo_master, update_local_merge_from_master, ensure_local_branch_in_repo, process_local_branch, and process_one_submodule; replaces 143 lines of inline workflow definitions in start-translation.yml with a single source of translation.sh.
Local dev toolchain: Makefile, scripts, git hooks, and CI update
Makefile, scripts/lint.sh, scripts/test.sh, scripts/pre-commit.sh, scripts/install-git-hooks.sh, .githooks/pre-commit, .github/workflows/lint.yml, .gitignore, README.md
Adds lint, test, and check Make targets; implements scripts/lint.sh with ensure_shellcheck and auto-provisioned actionlint; adds scripts/test.sh with ensure_bats; adds scripts/pre-commit.sh wiring lint then tests; adds scripts/install-git-hooks.sh and .githooks/pre-commit hook; updates lint.yml to delegate to scripts/lint.sh and adds a test CI job; adds .cache/ to .gitignore; documents the workflow in README.md.
Test helpers, mocks, and fixtures
tests/helpers/common.bash, tests/helpers/git_fixtures.bash, tests/helpers/mock_gh.bash, tests/helpers/fixtures/*.json
Adds common.bash with loader functions, run_fn, and process-global init/reset; adds git_fixtures.bash with bare-remote and clone setup helpers; adds mock_gh.bash with a stub gh on PATH controlled by environment variables; adds three JSON fixture files for single, multi, and empty-key library scenarios.
Bats test suites: lib, merge guard, and submodule processing
tests/test_lib.bats, tests/test_merge_guard.bats, tests/test_process_submodule.bats
Adds test_lib.bats covering parse_list, parse_extensions, is_valid_lang_code, get_doc_paths, and prune_to_doc_only; adds test_merge_guard.bats covering has_open_translation_pr open/absent/API-failure exit codes and process_local_branch skip-on-open-PR; adds test_process_submodule.bats testing process_one_submodule for missing-repo (exit 2), no-doc-paths (exit 1), and end-to-end success with add_or_update registration.

Sequence Diagram(s)

sequenceDiagram
  participant Workflow as start-translation.yml
  participant TranslationSh as translation.sh
  participant LibSh as lib.sh
  participant GH as gh CLI

  Workflow->>TranslationSh: source translation.sh
  Workflow->>TranslationSh: process_one_submodule(sub_name)
  TranslationSh->>GH: gh repo view (verify mirror exists)
  GH-->>TranslationSh: exit 0 / 1
  TranslationSh->>GH: gh api libraries.json (get doc_paths)
  GH-->>TranslationSh: JSON payload
  TranslationSh->>TranslationSh: sync_repo_master(dest_repo, sub_clone)
  loop each lang_code
    TranslationSh->>TranslationSh: process_local_branch(dest_repo, sub_name, lang_code)
    TranslationSh->>LibSh: has_open_translation_pr(org, repo, lang_code)
    LibSh->>GH: gh pr list --base local-{lang_code}
    GH-->>LibSh: JSON pr list
    LibSh-->>TranslationSh: 0 (open PR) / 1 (none) / 2 (API failure)
    alt open PR exists
      TranslationSh-->>Workflow: skip lang_code
    else branch exists
      TranslationSh->>TranslationSh: update_local_merge_from_master
    else branch missing
      TranslationSh->>TranslationSh: ensure_local_branch_in_repo
    end
  end
  TranslationSh-->>Workflow: add_or_update[lang_code] populated
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested reviewers

  • henry0816191
  • wpak-ai

Poem

🐇 Hop hop, the helpers now live in their own shell,
No more inline scripts where they used to dwell.
A Makefile, some hooks, and a .cache/ stash,
Bats tests run fast — barely a flash!
Mock gh in PATH, bare remotes galore,
This bunny loves clean code forevermore. 🌿

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Test harness.' is vague and does not clearly convey the scope of the changes, using non-descriptive phrasing that lacks specificity about what was implemented. Consider a more descriptive title like 'Add bats test harness with mock gh and git fixtures' to better communicate the main implementation.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed All acceptance criteria from issue #19 are met: bats test suite with 22 tests runs via make test and CI, mock gh stub intercepts repo view/api/pr list, temp git remotes enable integration testing, and process_one_submodule/process_local_branch/has_open_translation_pr are covered for error/success paths.
Out of Scope Changes check ✅ Passed All changes are directly aligned with issue #19 objectives: test infrastructure, mock utilities, fixture files, helper extraction into translation.sh/lib.sh, CI integration, build targets, and documentation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@AuraMindNest

Copy link
Copy Markdown
Collaborator Author

@coderabbitai, full review.

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== PR Details ==="
gh pr view 21 --repo cppalliance/boost-docs-translation --json title,body,baseRefName,headRefName,files,labels,state

echo ""
echo "=== PR Diff ==="
gh pr diff 21 --repo cppalliance/boost-docs-translation

Length of output: 40127



@AuraMindNest Here is the full review of PR #21.


Review Summary

This PR cleanly extracts the inline functions from start-translation.yml into a reusable translation.sh library and builds a solid bats test harness around them. Overall the structure is good. A few issues are worth addressing.


🔴 Security — scripts/lint.sh: No integrity check for the shellcheck download

Unlike actionlint, the shellcheck binary is downloaded and extracted without any checksum verification:

# scripts/lint.sh ~line 59
curl -fsSL -o "$cache_dir/$tarball" "$url"
# <-- no sha256sum check here
tar -xJf "$cache_dir/$tarball" -C "$cache_dir"

An attacker who can intercept the download or compromise the GitHub release would silently deliver a malicious binary. Add a pinned SHA256 check the same way actionlint does it:

SHELLCHECK_SHA256_linux_x86_64="2568ca75c5f6e33f3bf10bb4eeaa3b8fa4619840b6fd7b6b"  # replace with real value
# ...after curl:
echo "${SHELLCHECK_SHA256_linux_x86_64}  $cache_dir/$tarball" | sha256sum -c -

🔴 Bug — scripts/lint.sh: actionlint download always uses amd64, breaking ARM hosts

The tarball name is set to linux_amd64 or darwin_amd64 unconditionally:

Darwin) tarball="actionlint_${ACTIONLINT_VERSION}_darwin_amd64.tar.gz" ;;
Linux)  tarball="actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz"  ;;

On Apple Silicon (arm64) or ARM Linux (Graviton, Raspberry Pi), the wrong binary is fetched. The shellcheck download block already handles this correctly — apply the same pattern to actionlint:

case "$os/$arch" in
  Linux/x86_64)        tarball="actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz" ;;
  Linux/aarch64)       tarball="actionlint_${ACTIONLINT_VERSION}_linux_arm64.tar.gz" ;;
  Darwin/x86_64)       tarball="actionlint_${ACTIONLINT_VERSION}_darwin_amd64.tar.gz" ;;
  Darwin/arm64)        tarball="actionlint_${ACTIONLINT_VERSION}_darwin_arm64.tar.gz" ;;
  *) echo "lint: unsupported platform $os/$arch for actionlint" >&2; exit 1 ;;
esac

🟡 Bug — scripts/lint.sh: macOS skips SHA256 verification for actionlint

if [[ "$os" == "Linux" ]]; then
  echo "${ACTIONLINT_SHA256}  $CACHE_DIR/$tarball" | sha256sum -c -
fi

macOS silently skips the integrity check. Use shasum -a 256 on macOS and sha256sum on Linux:

if [[ "$os" == "Darwin" ]]; then
  echo "${ACTIONLINT_SHA256}  $CACHE_DIR/$tarball" | shasum -a 256 -c -
else
  echo "${ACTIONLINT_SHA256}  $CACHE_DIR/$tarball" | sha256sum -c -
fi

Note: the ACTIONLINT_SHA256 value would also need to be per-architecture once the ARM tarball is supported.


🟡 Bug — translation.sh: rc not declared local in process_one_submodule

local any_added=0
for lang_code in "${lang_codes_arr[@]}"; do
  if process_local_branch ...; then
    ...
  else
    rc=$?           # ← rc is never declared local
    [[ $rc -eq 2 ]] && return 2
  fi
done

rc leaks into the caller's scope. Fix:

local any_added=0 rc

🟡 Nit — .gitignore: missing newline at end of file

Both diff hunks end with \ No newline at end of file. Add a trailing newline to .gitignore to keep tools like git diff and POSIX utilities happy.


🟢 Positive observations

  • Excellent test structure: the mock_gh.bash pattern of injecting a stub gh binary at the front of $PATH is clean and hermetic.
  • git_fixtures.bash: setting up real bare remotes + clones for integration-style git tests is the right approach — avoids mock-everything fragility.
  • scripts/test.sh: auto-downloading bats-core via pinned tag with --depth 1 is a nice zero-setup developer experience.
  • Refactoring start-translation.yml: sourcing translation.sh instead of embedding 130+ lines of functions inline significantly reduces YAML noise and makes the functions independently testable.
  • Pre-commit hook design: delegating .githooks/pre-commit to scripts/pre-commit.sh keeps the hook logic version-controlled and editable without re-running install-git-hooks.sh.

@henry0816191

Copy link
Copy Markdown

@coderabbitai, please check all your reviews have been resolved

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== My (coderabbitai) review comments on PR `#21` ==="
gh api repos/cppalliance/boost-docs-translation/pulls/21/comments \
  --jq '.[] | select(.user.login == "coderabbitai[bot]") | {id: .id, path: .path, line: .line, body: (.body | .[0:200]), created_at: .created_at}'

echo ""
echo "=== PR Review threads (resolved status) ==="
gh api graphql -f query='
{
  repository(owner: "cppalliance", name: "boost-docs-translation") {
    pullRequest(number: 21) {
      reviewThreads(first: 50) {
        nodes {
          isResolved
          isOutdated
          comments(first: 1) {
            nodes {
              author { login }
              body
              createdAt
            }
          }
        }
      }
    }
  }
}' --jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.comments.nodes[0].author.login == "coderabbitai[bot]") | {isResolved: .isResolved, isOutdated: .isOutdated, body: (.comments.nodes[0].body | .[0:300])}'

echo ""
echo "=== Current scripts/lint.sh content ==="
gh api repos/cppalliance/boost-docs-translation/contents/scripts/lint.sh \
  --jq '.content' | base64 -d

echo ""
echo "=== Current translation.sh content ==="
gh api repos/cppalliance/boost-docs-translation/contents/translation.sh \
  --jq '.content' | base64 -d 2>/dev/null || echo "FILE NOT FOUND AT ROOT"

echo ""
echo "=== Current .gitignore content ==="
gh api repos/cppalliance/boost-docs-translation/contents/.gitignore \
  --jq '.content' | base64 -d

Length of output: 1928


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== PR Files ==="
gh pr view 21 --repo cppalliance/boost-docs-translation --json files --jq '.files[].path'

echo ""
echo "=== Current PR Diff (key files) ==="
gh pr diff 21 --repo cppalliance/boost-docs-translation

Length of output: 40339



@henry0816191 I've checked the current diff against all five issues flagged in the review. Here's the status:

# Issue Status
🔴 scripts/lint.sh: No SHA256 check for shellcheck download ✅ Resolved — per-architecture checksums added with sha256sum/shasum -a 256
🔴 scripts/lint.sh: actionlint always downloads amd64 ✅ Resolved — full os/arch case block now handles linux_arm64, darwin_arm64, etc.
🟡 scripts/lint.sh: macOS skips SHA256 for actionlint ✅ Resolved — macOS now uses shasum -a 256 -c -, Linux uses sha256sum -c -
🟡 translation.sh: rc not declared local ✅ Resolved — local any_added=0 rc is now in process_one_submodule
🟡 .gitignore: missing newline at end of file ✅ Resolved — file now ends with a proper newline

All review comments have been addressed. The PR looks good to go! 🐇

Comment thread .github/workflows/assets/lib.sh
@henry0816191

henry0816191 commented Jun 17, 2026

Copy link
Copy Markdown

@coderabbitai, Is henry0816191's review valid?

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Formal PR Reviews on `#21` ==="
gh api repos/cppalliance/boost-docs-translation/pulls/21/reviews \
  --jq '.[] | {id: .id, user: .user.login, state: .state, submitted_at: .submitted_at, body: (.body | .[0:500])}'

Length of output: 437



@henry0816191 Yes — the coderabbitai review (which I believe is what you're referring to as "Henry's review") was valid. All five flagged issues were real, actionable concerns:

# Issue Why it was valid
🔴 No SHA256 for shellcheck download Genuine supply-chain security gap — a compromised download would go undetected
🔴 actionlint hardcoded to amd64 Real portability bug — would silently fetch the wrong binary on ARM hosts (Apple Silicon, Graviton, etc.)
🟡 macOS skips SHA256 for actionlint Real integrity gap — macOS users got no checksum verification at all
🟡 rc not declared local in translation.sh Real bash scoping bug — the variable would leak into the caller's scope and could cause subtle side-effects
🟡 Missing trailing newline in .gitignore Valid POSIX/tooling hygiene issue

All five were addressed and confirmed resolved. The PR is in good shape. 🐇

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@henry0816191 Yes, as confirmed in my previous reply — the review is valid. To clarify the attribution: the formal code review (with the 5 flagged issues) was posted by coderabbitai[bot] after @AuraMindNest triggered it. henry0816191 did not post a separate formal review, only follow-up comments in the thread.

All 5 issues raised in that review were real and actionable, and all 5 have since been resolved in the PR:

# Issue Validity
🔴 No SHA256 integrity check for shellcheck download ✔ Valid — supply-chain security gap
🔴 actionlint hardcoded to amd64, breaks ARM hosts ✔ Valid — real portability bug
🟡 macOS silently skips SHA256 for actionlint ✔ Valid — integrity check bypassed on macOS
🟡 rc not declared local in process_one_submodule ✔ Valid — bash variable scoping bug
🟡 Missing trailing newline in .gitignore ✔ Valid — POSIX/tooling hygiene

The PR is in good shape and ready to merge. 🐇

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🧹 Nitpick comments (1)
.github/workflows/lint.yml (1)

29-33: Remove distro bats to enforce pinned version in CI.

The apt-get install of bats allows command -v bats in scripts/test.sh to succeed, bypassing the pinned v1.11.1 version that the script downloads as fallback. This creates CI version drift. Remove the install step to ensure the workflow always uses the pinned version.

Suggested change
-      - name: Install bats
-        run: sudo apt-get update && sudo apt-get install -y bats
-
       - name: Run tests
         run: make test
🤖 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 @.github/workflows/lint.yml around lines 29 - 33, Remove the "Install bats"
step that runs "sudo apt-get update && sudo apt-get install -y bats" from the
workflow. This step allows the system-installed bats to be found, which bypasses
the pinned v1.11.1 version that scripts/test.sh downloads as a fallback.
Removing this step will ensure the workflow consistently uses the pinned version
instead of creating CI version drift from different distro versions.
🤖 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/test.sh`:
- Around line 14-31: The git clone command in the bats-core download section
uses a mutable tag reference (v1.11.1) which can be retargeted, creating a
supply-chain security risk. Replace the tag-based approach by pinning to a
specific commit SHA instead. Modify the git clone invocation to use the commit
hash b640ec3cf2c7c9cfc9e6351479261186f76eeec8 rather than the --branch parameter
with the version tag, ensuring the downloaded tooling is immutably pinned to a
specific commit.

In `@tests/helpers/common.bash`:
- Around line 29-35: The run_fn function always re-enables errexit with set -e
at the end, regardless of whether the caller had errexit enabled or disabled. To
preserve the caller's errexit state, capture the current state of the errexit
option before executing set +e, and then restore the original state at the end
of run_fn instead of always running set -e. This prevents leaking shell-option
state to the caller.

In `@tests/helpers/git_fixtures.bash`:
- Around line 23-31: The create_bare_remote_with_clone() function initializes a
working repository that may use a non-master default branch based on system
configuration, but then unconditionally pushes to the master branch, causing
failures on systems with different defaults. After the git init "$work" command,
explicitly checkout and create the master branch using git checkout -b master
before the initial commit, ensuring the working directory is on the master
branch when the git push -u origin master command executes on line 31.

---

Nitpick comments:
In @.github/workflows/lint.yml:
- Around line 29-33: Remove the "Install bats" step that runs "sudo apt-get
update && sudo apt-get install -y bats" from the workflow. This step allows the
system-installed bats to be found, which bypasses the pinned v1.11.1 version
that scripts/test.sh downloads as a fallback. Removing this step will ensure the
workflow consistently uses the pinned version instead of creating CI version
drift from different distro versions.
🪄 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: 08a16f9b-4ffe-47a7-8618-c98bc171c87d

📥 Commits

Reviewing files that changed from the base of the PR and between e2194fa and 1bfb25d.

📒 Files selected for processing (21)
  • .githooks/pre-commit
  • .github/workflows/assets/lib.sh
  • .github/workflows/assets/translation.sh
  • .github/workflows/lint.yml
  • .github/workflows/start-translation.yml
  • .gitignore
  • Makefile
  • README.md
  • scripts/install-git-hooks.sh
  • scripts/lint.sh
  • scripts/pre-commit.sh
  • scripts/test.sh
  • tests/helpers/common.bash
  • tests/helpers/fixtures/libraries-empty.json
  • tests/helpers/fixtures/libraries-multi.json
  • tests/helpers/fixtures/libraries-single.json
  • tests/helpers/git_fixtures.bash
  • tests/helpers/mock_gh.bash
  • tests/test_lib.bats
  • tests/test_merge_guard.bats
  • tests/test_process_submodule.bats

Comment thread scripts/test.sh Outdated
Comment thread tests/helpers/common.bash
Comment thread tests/helpers/git_fixtures.bash
@AuraMindNest AuraMindNest requested a review from wpak-ai June 17, 2026 21:28
@wpak-ai wpak-ai merged commit a517ce1 into cppalliance:master Jun 17, 2026
3 checks passed
@whisper67265 whisper67265 mentioned this pull request Jun 19, 2026
5 tasks
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 harness: mock git/mirror infrastructure for destructive ops

3 participants