feat(ci): run liquid tests in parallel, grouped per firm and template type

.github/workflows/run_tests.yml

@@ -31,39 +31,50 @@ 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.
+          # escape_json: false keeps it valid JSON for jq (the default double-escapes the quotes).
+          # safe_output: false stops backslash-escaping of shell metacharacters (& ( ) ...) inside
+          # the JSON values, which is invalid JSON and would silently empty the template list.
+          # Safe here: the list is passed via env and parsed with jq, never eval'd by the shell.
+          # quotepath: false emits paths with non-ASCII characters (e.g. a curly apostrophe in an
+          # account template name) as-is; the action defaults this to true, which octal-escapes
+          # them so they no longer match the glob and the template is silently dropped (setting
+          # git's global core.quotepath does not help - the action overrides it from this input).
+          json: true
+          escape_json: false
+          safe_output: false
+          quotepath: false
           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=()
+          # all_changed_files is a JSON array (json: true). Extract the unique template
+          # directories, space-safely. Only reconciliation_texts and account_templates have
+          # liquid tests; shared_parts is left out so a shared-parts-only PR skips the test
+          # job entirely instead of spinning it up just to skip every template.
+          pattern='(reconciliation_texts|account_templates)/[^/]+'
+          # Fail loudly if the changed-files output is not valid JSON. A silent jq failure
+          # here would empty the template list and skip every test while CI stays green.
+          if ! printf '%s' "${ALL_CHANGED_FILES:-[]}" | jq -e . > /dev/null; then
+            echo "::error::changed-files output is not valid JSON, refusing to skip tests silently"
+            exit 1
           fi
-          # Store outputs
+          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
@@ -72,6 +83,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: 10
     if: ${{ needs.check-changed-templates.outputs.changed_templates != '[]' }}
     needs: [check-auth, check-changed-templates]
     steps:
@@ -93,24 +109,57 @@ 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:-10}"
+          # Testable templates grouped by "<firm id>|<template type>". Each bucket maps to one
+          # `run-test` call, because the CLI takes a single firm and a single template type per
+          # invocation. Identifiers are stored newline-separated so account template names
+          # (which contain spaces and other characters) stay intact.
+          declare -A TEMPLATE_BUCKETS
           declare -a ERRORS
-          for CURRENT_DIR in ${{ env.CHANGED_TEMPLATES }}; do
-            echo "Checking ${CURRENT_DIR}"
+          declare -a SUMMARY
+          # CHANGED_TEMPLATES is a JSON array; read it space-safely. The process substitution
+          # keeps the loop in the current shell so the arrays above survive.
+          while IFS= read -r CURRENT_DIR; do
+            [[ -z "${CURRENT_DIR}" ]] && continue
             while [[ "${CURRENT_DIR}" != "." ]]; do
               if [[ -e "${CURRENT_DIR}/config.json" ]]; then
-                HANDLE=$(cat ${CURRENT_DIR}/config.json | jq -r ".handle // .name")
-
+                # Decide the template type from the path. Only reconciliation_texts and
+                # account_templates have liquid tests; the filter step already excludes other
+                # types, this branch is defense in depth.
+                if [[ "${CURRENT_DIR}" == *reconciliation_texts* ]]; then
+                  TEMPLATE_TYPE="reconciliationText"
+                  IDENTIFIER=$(cat "${CURRENT_DIR}/config.json" | jq -r ".handle // .name")
+                elif [[ "${CURRENT_DIR}" == *account_templates* ]]; then
+                  TEMPLATE_TYPE="accountTemplate"
+                  # Account template configs often have a null handle/name, so use the folder name.
+                  IDENTIFIER=$(basename "${CURRENT_DIR}")
+                else
+                  echo "Skipping ${CURRENT_DIR} (no liquid tests for this template type)"
+                  break
+                fi
+
+                # Skip templates whose test YAML is missing or has no real content. The CLI
+                # reports those as FAILED ("there are no tests stored in the YAML file"), and a
+                # template without tests should not fail CI.
+                TEST_REL=$(cat "${CURRENT_DIR}/config.json" | jq -r '.test // empty')
+                TEST_PATH="${CURRENT_DIR}/${TEST_REL}"
+                if [[ -z "${TEST_REL}" || ! -s "${TEST_PATH}" ]] \
+                   || [[ "$(grep -cvE '^[[:space:]]*(#|$)' "${TEST_PATH}")" -eq 0 ]]; then
+                  echo "Skipping ${IDENTIFIER} (no liquid tests defined)"
+                  break
+                fi
+
                 # Initialize FIRM_ID
                 FIRM_ID=""
 
                 # Check if test_firm_id is present in config
-                TEST_FIRM_ID=$(cat ${CURRENT_DIR}/config.json | jq -r ".test_firm_id // empty")
+                TEST_FIRM_ID=$(cat "${CURRENT_DIR}/config.json" | jq -r ".test_firm_id // empty")
 
                 if [[ -n "$TEST_FIRM_ID" && "$TEST_FIRM_ID" != "null" ]]; then
                   # 1. Template-specific test_firm_id (highest priority)
-                  AVAILABLE_FIRM_IDS=$(cat ${CURRENT_DIR}/config.json | jq -r ".id | keys[]" 2>/dev/null || echo "")
+                  AVAILABLE_FIRM_IDS=$(cat "${CURRENT_DIR}/config.json" | jq -r ".id | keys[]" 2>/dev/null || echo "")
 
                   # Check for exact match by looping through available IDs
                   FOUND_MATCH=false
@@ -123,15 +172,14 @@ jobs:
 
                   if [[ "$FOUND_MATCH" == "true" ]]; then
                     FIRM_ID="$TEST_FIRM_ID"
-                    echo "Using configured test_firm_id: ${FIRM_ID}"
                   else
                     echo "Warning: test_firm_id '${TEST_FIRM_ID}' not found in .id object, falling back to environment variable or default"
                   fi
                 fi
 
                 if [[ -z "$FIRM_ID" && -n "$SF_TEST_FIRM_ID" ]]; then
                   # 2. Environment variable fallback
-                  AVAILABLE_FIRM_IDS=$(cat ${CURRENT_DIR}/config.json | jq -r ".id | keys[]" 2>/dev/null || echo "")
+                  AVAILABLE_FIRM_IDS=$(cat "${CURRENT_DIR}/config.json" | jq -r ".id | keys[]" 2>/dev/null || echo "")
 
                   # Check for exact match by looping through available IDs
                   FOUND_MATCH=false
@@ -144,42 +192,102 @@ jobs:
 
                   if [[ "$FOUND_MATCH" == "true" ]]; then
                     FIRM_ID="$SF_TEST_FIRM_ID"
-                    echo "Using SF_TEST_FIRM_ID environment variable: ${FIRM_ID}"
                   else
                     echo "Warning: SF_TEST_FIRM_ID '${SF_TEST_FIRM_ID}' not found in .id object, falling back to default"
                   fi
                 fi
 
                 if [[ -z "$FIRM_ID" ]]; then
                   # 3. Default behavior - use first available firm ID
-                  FIRM_ID=$(cat ${CURRENT_DIR}/config.json | jq -r ".id" | jq "keys_unsorted" | jq "first" | tr -d '"')
-                  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
+                  FIRM_ID=$(cat "${CURRENT_DIR}/config.json" | jq -r ".id" | jq "keys_unsorted" | jq "first" | tr -d '"')
                 fi
+
+                # Group this template under "<firm>|<type>" so all templates that share a firm
+                # and type run together in one parallel batch. Newline-separated keeps identifiers
+                # that contain spaces (account template names) intact.
+                TEMPLATE_BUCKETS["${FIRM_ID}|${TEMPLATE_TYPE}"]+="${IDENTIFIER}"$'\n'
+                echo "+ ${IDENTIFIER} → firm ${FIRM_ID}"
                 break
               else
                 echo "Config file not found in ${CURRENT_DIR}"
                 CURRENT_DIR="$(dirname "${CURRENT_DIR}")"
               fi
             done
+          done < <(printf '%s' "${CHANGED_TEMPLATES}" | jq -r '.[]')
+          # Run each bucket. All identifiers passed to a single `run-test --status` call run
+          # concurrently in the CLI (Promise.all), so we cap each batch at MAX_PARALLEL and run
+          # batches sequentially to keep the number of in-flight tests against the platform bounded.
+          for KEY in "${!TEMPLATE_BUCKETS[@]}"; do
+            FIRM_ID="${KEY%%|*}"
+            TEMPLATE_TYPE="${KEY##*|}"
+            # The CLI uses a different flag per template type.
+            if [[ "${TEMPLATE_TYPE}" == "accountTemplate" ]]; then
+              CLI_FLAG="--account-template"
+            else
+              CLI_FLAG="--handle"
+            fi
+            # Deduplicate the identifiers for this bucket (newline-separated, so names with
+            # spaces stay intact).
+            mapfile -t IDENTIFIERS < <(printf '%s' "${TEMPLATE_BUCKETS[$KEY]}" | sort -u | grep -v '^$')
+            TOTAL=${#IDENTIFIERS[@]}
+            for (( START=0; START<TOTAL; START+=MAX_PARALLEL )); do
+              BATCH=("${IDENTIFIERS[@]:START:MAX_PARALLEL}")
+              # Fold each parallel batch into a collapsible log group so the raw CLI output
+              # (spinner frames included) stays out of the way unless you open it.
+              echo "::group::firm ${FIRM_ID} · ${TEMPLATE_TYPE} · ${#BATCH[@]} in parallel"
+              if OUTPUT=$(node ./node_modules/silverfin-cli/bin/cli.js run-test --firm "${FIRM_ID}" --status ${CLI_FLAG} "${BATCH[@]}" 2>&1); then
+                BATCH_EXIT=0
+              else
+                BATCH_EXIT=$?
+              fi
+              echo "${OUTPUT}"
+              # Parse the per-template status. In CI, consola prefixes every line with "[log] "
+              # and the test spinner interleaves frames via carriage returns, so split on carriage
+              # returns and strip ANSI escapes first. We then read the top-level
+              # "[log] <name>: PASSED|FAILED" lines (a leading space after "[log] " marks an
+              # indented sub-result, which is skipped; trailing whitespace is tolerated) and match
+              # by EXACT name: account template names contain spaces and characters that are
+              # unsafe inside a regex.
+              CLEAN_OUTPUT=$(printf '%s\n' "${OUTPUT}" | tr '\r' '\n' | sed -E $'s/\033\\[[0-9;?]*[A-Za-z]//g')
+              declare -A RESULTS=()
+              while IFS= read -r LINE; do
+                STATUS="${LINE##*: }"
+                STATUS="${STATUS%%[[:space:]]*}"
+                NAME="${LINE#\[log\] }"
+                NAME="${NAME%: *}"
+                RESULTS["${NAME}"]="${STATUS}"
+              done < <(printf '%s\n' "${CLEAN_OUTPUT}" | grep -oE '\[log\] [^[:space:]].*: (PASSED|FAILED)[[:space:]]*$')
+              for IDENTIFIER in "${BATCH[@]}"; do
+                case "${RESULTS[${IDENTIFIER}]:-}" in
+                  PASSED)
+                    echo "${IDENTIFIER}: passed"
+                    SUMMARY+=("PASS"$'\t'"${FIRM_ID}"$'\t'"${IDENTIFIER}")
+                    ;;
+                  FAILED)
+                    echo "${IDENTIFIER}: failed"
+                    ERRORS+=("${IDENTIFIER}")
+                    SUMMARY+=("FAIL"$'\t'"${FIRM_ID}"$'\t'"${IDENTIFIER}")
+                    ;;
+                  *)
+                    echo "${IDENTIFIER}: status could not be determined (batch exit code ${BATCH_EXIT})"
+                    ERRORS+=("${IDENTIFIER}")
+                    SUMMARY+=("????"$'\t'"${FIRM_ID}"$'\t'"${IDENTIFIER}")
+                    ;;
+                esac
+              done
+              echo "::endgroup::"
+            done
           done
+          # Flat summary keyed on the two columns that matter: template and firm.
+          if [ ${#SUMMARY[@]} -gt 0 ]; then
+            echo ""
+            echo "─ Results ─────────────────────────────────────────────────────"
+            while IFS=$'\t' read -r RESULT FIRM NAME; do
+              printf ' %-4s %-45s firm %s\n' "${RESULT}" "${NAME}" "${FIRM}"
+            done < <(printf '%s\n' "${SUMMARY[@]}")
+            echo "────────────────────────────────────────────────────────────────"
+            echo "${#ERRORS[@]} failed, $(( ${#SUMMARY[@]} - ${#ERRORS[@]} )) passed"
+          fi
           # CHECK ERRORS PRESENT
           if [ ${#ERRORS[@]} -eq 0 ]; then
               echo "All tests have passed"