From aa4eb23e45ff3504637a322f7750cc9f39ae2d28 Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Tue, 16 Jun 2026 17:31:24 +0200 Subject: [PATCH 1/5] feat(ci): run liquid tests in parallel grouped per firm Replace the per-template sequential test loop with a two-phase approach: group changed reconciliations by their resolved firm, then run each firm's handles through a single `run-test --status` call that executes them concurrently (CLI Promise.all). Batches are capped at MAX_PARALLEL_TESTS (default 5) and run sequentially so the total number of in-flight tests against the live platform stays bounded, since the CLI has no client-side rate limiting. Also gate the reconciliation_texts check before firm resolution so shared-part changes (whose ".id" is a plain string) are skipped cleanly instead of aborting the step on a jq error, and drop the per-iteration CONFIG_JSON re-write (a no-op; token refresh is handled elsewhere). --- .github/workflows/run_tests.yml | 71 ++++++++++++++++++++++++--------- 1 file changed, 53 insertions(+), 18 deletions(-) diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 35727b1..7628cc6 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -72,6 +72,11 @@ jobs: SF_API_SECRET: "${{ secrets.SF_API_SECRET }}" SF_TEST_FIRM_ID: "${{ vars.SF_TEST_FIRM_ID }}" CHANGED_TEMPLATES: "${{ needs.check-changed-templates.outputs.changed_templates }}" + # Maximum number of liquid tests run in parallel against the live platform per batch. + # A single `run-test --status` call fires all of its handles concurrently (Promise.all) + # with no client-side rate limiting, so we cap the batch size and run batches + # sequentially to keep the global number of in-flight tests bounded. + MAX_PARALLEL_TESTS: 5 if: ${{ needs.check-changed-templates.outputs.changed_templates != '[]' }} needs: [check-auth, check-changed-templates] steps: @@ -93,13 +98,22 @@ jobs: npm install https://github.com/silverfin/silverfin-cli.git VERSION=$(./node_modules/silverfin-cli/bin/cli.js -V) echo "CLI version: ${VERSION}" - - name: Run liquid tests for updated templates + - name: Run liquid tests for updated templates (grouped per firm, run in parallel) run: | + MAX_PARALLEL="${MAX_PARALLEL_TESTS:-5}" + # Reconciliation handles grouped by their resolved firm id ("firm id" -> "handle1 handle2 ..."). + declare -A FIRM_HANDLES declare -a ERRORS for CURRENT_DIR in ${{ env.CHANGED_TEMPLATES }}; do echo "Checking ${CURRENT_DIR}" while [[ "${CURRENT_DIR}" != "." ]]; do if [[ -e "${CURRENT_DIR}/config.json" ]]; then + # Only reconciliation_texts have liquid tests executed in CI; skip anything + # else (e.g. shared_parts, whose ".id" is a plain string) before resolving a firm. + if [[ "${CURRENT_DIR}" != *reconciliation_texts* ]]; then + echo "Skipping ${CURRENT_DIR} (not a reconciliation template)" + break + fi HANDLE=$(cat ${CURRENT_DIR}/config.json | jq -r ".handle // .name") # Initialize FIRM_ID @@ -156,23 +170,10 @@ jobs: echo "Using first available firm ID: ${FIRM_ID}" fi - if [[ "${CURRENT_DIR}" == *reconciliation_texts* ]]; then - # FETCH THE NEWEST VERSION OF THE TOKENS FROM THE SECRETS, IN CASE THEY WERE UPDATED BY THE INITIATION OF A CONCURRENT WORKFLOW - echo '${{ secrets.CONFIG_JSON }}' > $HOME/.silverfin/config.json - # RUN TEST - echo "Running tests for ${HANDLE} in firm ${FIRM_ID}" - OUTPUT=$(node ./node_modules/silverfin-cli/bin/cli.js run-test --handle "${HANDLE}" --firm "${FIRM_ID}" --status 2>&1) - # CHECK OUTPUT - if [[ "$OUTPUT" =~ "PASSED" ]]; then - echo "${HANDLE}: passed" - elif [[ "$OUTPUT" =~ "FAILED" ]]; then - echo "${HANDLE}: failed" - ERRORS+=("${HANDLE}") - else - echo "${HANDLE}: other errors: ${OUTPUT}" - ERRORS+=("${OUTPUT}") - fi - fi + # Group this reconciliation under its resolved firm so all reconciliations + # for the same firm can be run together in a single parallel batch later. + FIRM_HANDLES["${FIRM_ID}"]+=" ${HANDLE}" + echo "Queued ${HANDLE} for firm ${FIRM_ID}" break else echo "Config file not found in ${CURRENT_DIR}" @@ -180,6 +181,40 @@ jobs: fi done done + # Run the queued reconciliations grouped per firm. All handles passed to a single + # `run-test --status` call are executed concurrently by the CLI (Promise.all), so we + # cap each batch at MAX_PARALLEL handles and run batches sequentially to keep the + # total number of in-flight tests against the live platform bounded. + for FIRM_ID in "${!FIRM_HANDLES[@]}"; do + # Deduplicate and sort the handles queued for this firm. + read -r -a HANDLES <<< "$(printf '%s\n' ${FIRM_HANDLES[$FIRM_ID]} | sort -u | tr '\n' ' ')" + TOTAL=${#HANDLES[@]} + echo "Firm ${FIRM_ID}: ${TOTAL} reconciliation(s) to test: ${HANDLES[*]}" + for (( START=0; START&1); then + BATCH_EXIT=0 + else + BATCH_EXIT=$? + fi + echo "${OUTPUT}" + # Strip ANSI colour codes so the per-handle status lines can be parsed reliably. + CLEAN_OUTPUT=$(printf '%s\n' "${OUTPUT}" | sed -E "s/\x1b\[[0-9;]*m//g") + # The CLI prints one top-level ": PASSED" / ": FAILED" line per handle. + for HANDLE in "${BATCH[@]}"; do + if printf '%s\n' "${CLEAN_OUTPUT}" | grep -qE "^[[:space:]]*${HANDLE}: PASSED[[:space:]]*$"; then + echo "${HANDLE}: passed" + elif printf '%s\n' "${CLEAN_OUTPUT}" | grep -qE "^[[:space:]]*${HANDLE}: FAILED[[:space:]]*$"; then + echo "${HANDLE}: failed" + ERRORS+=("${HANDLE}") + else + echo "${HANDLE}: status could not be determined (batch exit code ${BATCH_EXIT})" + ERRORS+=("${HANDLE}") + fi + done + done + done # CHECK ERRORS PRESENT if [ ${#ERRORS[@]} -eq 0 ]; then echo "All tests have passed" From ddf487e09d5c65a6fd8d9e1a5ff9171dca35409e Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Wed, 17 Jun 2026 10:52:47 +0200 Subject: [PATCH 2/5] fix(ci): parse run-test output with [log] prefix and spinner noise In CI, consola prefixes every line with a '[log] ' tag and the test spinner interleaves ANSI/braille frames onto the same line, so the per-handle status is not at the start of a line. Normalise carriage returns and ANSI escapes and match ': PASSED/FAILED' on a left word boundary instead of anchoring to '^', which previously caused every handle to be reported as 'status could not be determined'. --- .github/workflows/run_tests.yml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 7628cc6..eba4aa5 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -199,13 +199,17 @@ jobs: BATCH_EXIT=$? fi echo "${OUTPUT}" - # Strip ANSI colour codes so the per-handle status lines can be parsed reliably. - CLEAN_OUTPUT=$(printf '%s\n' "${OUTPUT}" | sed -E "s/\x1b\[[0-9;]*m//g") - # The CLI prints one top-level ": PASSED" / ": FAILED" line per handle. + # Normalise the output before parsing: split carriage-return spinner frames onto + # their own lines and strip ANSI / cursor escape sequences. In CI, consola prefixes + # every line with a "[log] " tag and the test spinner interleaves frames onto the + # same line, so the per-handle status (": PASSED") is not at the start of a + # line. Match the handle on a left word boundary instead of anchoring to "^". + CLEAN_OUTPUT=$(printf '%s\n' "${OUTPUT}" | tr '\r' '\n' | sed -E $'s/\033\\[[0-9;?]*[A-Za-z]//g') + # The CLI prints one ": PASSED" / ": FAILED" line per handle. for HANDLE in "${BATCH[@]}"; do - if printf '%s\n' "${CLEAN_OUTPUT}" | grep -qE "^[[:space:]]*${HANDLE}: PASSED[[:space:]]*$"; then + if printf '%s\n' "${CLEAN_OUTPUT}" | grep -qE "(^|[^[:alnum:]_])${HANDLE}: PASSED"; then echo "${HANDLE}: passed" - elif printf '%s\n' "${CLEAN_OUTPUT}" | grep -qE "^[[:space:]]*${HANDLE}: FAILED[[:space:]]*$"; then + elif printf '%s\n' "${CLEAN_OUTPUT}" | grep -qE "(^|[^[:alnum:]_])${HANDLE}: FAILED"; then echo "${HANDLE}: failed" ERRORS+=("${HANDLE}") else From b2ddd3cda4da76c5ab26fd66fe63e50ab9cd7ee6 Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Wed, 17 Jun 2026 11:22:07 +0200 Subject: [PATCH 3/5] feat(ci): also test account templates, grouped per firm and type The CLI runs liquid tests for both reconciliation_texts (--handle) and account_templates (--account-template). Group changed templates by '|' so each bucket maps to one run-test call (the CLI takes a single firm and a single type per call) and pick the flag per bucket. Account template folder names contain spaces and characters like & ( ) ', so make the whole path space-safe: emit the changed-files list as JSON, hand it between steps/jobs as a JSON array, quote the config reads, and parse the run-test output by exact template name instead of a regex. Shared parts (and anything without liquid tests) are still skipped. --- .github/workflows/run_tests.yml | 165 ++++++++++++++++++-------------- 1 file changed, 95 insertions(+), 70 deletions(-) diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index eba4aa5..3f12e99 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -31,39 +31,31 @@ jobs: since_last_remote_commit: false dir_names: false base_sha: 'main' + # Emit the file list as a JSON array so paths containing spaces (e.g. account + # template names) survive the hand-off between steps and jobs intact. + json: true files: | **/**.{liquid,yml,yaml,json} - name: Filter templates changed id: templates_changed + env: + # Passed via env (not inlined) so a path with quotes/spaces cannot break the script. + ALL_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }} run: | - pattern="(?:reconciliation_texts|shared_parts)/([^/]+)/" - changed_files="${{ steps.changed-files.outputs.all_changed_files }}" - if [ -n "$changed_files" ]; then - filtered_names=($(printf "%s\n" "$changed_files" | grep -oP "$pattern" || true)) - if [ $? -ne 0 ]; then - echo "No files match the pattern" - changed_templates=() - else - # Remove the trailing "/" from the extracted names - filtered_names=("${filtered_names[@]%/}") - # Remove duplicates - changed_templates=($(printf "%s\n" "${filtered_names[@]}" | sort -u)) - fi - else - echo "No changed files" - changed_templates=() - fi - # Store outputs + # all_changed_files is a JSON array (json: true). Extract the unique template + # directories (reconciliation_texts / account_templates / shared_parts), space-safely. + pattern='(reconciliation_texts|account_templates|shared_parts)/[^/]+' + mapfile -t changed_templates < <(printf '%s' "${ALL_CHANGED_FILES:-[]}" | jq -r '.[]' | grep -oE "$pattern" | sort -u) if [ ${#changed_templates[@]} -eq 0 ]; then - echo "changed_templates=[]" >> $GITHUB_OUTPUT + echo "No templates changed" + echo "changed_templates=[]" >> "$GITHUB_OUTPUT" else - echo "changed_templates=${changed_templates[*]}" >> $GITHUB_OUTPUT - # Print the templates names - for name in "${changed_templates[@]}"; do - echo "$name" - done + echo "Changed templates:" + printf ' %s\n' "${changed_templates[@]}" + # Emit a JSON array so directory names containing spaces survive as single entries. + json=$(printf '%s\n' "${changed_templates[@]}" | jq -R . | jq -s -c .) + echo "changed_templates=${json}" >> "$GITHUB_OUTPUT" fi - exit 0 test-templates: runs-on: ubuntu-latest @@ -101,30 +93,43 @@ jobs: - name: Run liquid tests for updated templates (grouped per firm, run in parallel) run: | MAX_PARALLEL="${MAX_PARALLEL_TESTS:-5}" - # Reconciliation handles grouped by their resolved firm id ("firm id" -> "handle1 handle2 ..."). - declare -A FIRM_HANDLES + # Testable templates grouped by "|