diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 35727b1..f062585 100644 --- a/.github/workflows/run_tests.yml +++ b/.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 "|