Skip to content

Conversation

@michieldegezelle
Copy link
Collaborator

Description

Include a summary of the changes made

Fixes # (link to the corresponding issue)

Type of change

  • Bug fix
  • New feature
  • Breaking change

Checklist

  • README updated (if needed)
  • Version updated (if needed)
  • Documentation updated (if needed)

if [[ "$line" =~ ^[[:space:]]+([^:]+):[[:space:]]+FAILED ]]; then
local test_name=$(echo "${BASH_REMATCH[1]}" | xargs)
if [[ -n "$current_template" ]]; then
ERRORS+=("${current_template}: ${test_name}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: maybe we can store also template type and firm id ? It may be relevant to know in which firm it was run and failed?

Suggested change
ERRORS+=("${current_template}: ${test_name}")
ERRORS+=("[${template_type}] ${current_template}: ${test_name} (in firm ${firm_id}")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusted!

# Build command with space-delimited handles
OUTPUT=$(node ./node_modules/silverfin-cli/bin/cli.js run-test -h ${HANDLES} -f "${FIRM_ID}" --status 2>&1)
parse_test_output "$OUTPUT"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why we build handles as "space delimited" and accounts "quoted"?
It should be safer to always pass all of them quoted.

Also, when we build the array we are using white spaces which may conflict with with white spaces in the name/handle? here

HANDLES_BY_FIRM[${FIRM_ID}]="${HANDLES_BY_FIRM[${FIRM_ID}]} ${HANDLE_OR_NAME}"

maybe use something like | and then split the array on the | ?

HANDLES_BY_FIRM[${FIRM_ID}]="${HANDLES_BY_FIRM[${FIRM_ID}]}|${HANDLE_OR_NAME}"

@michieldegezelle michieldegezelle merged commit 671d2df into main Jan 12, 2026
8 of 9 checks passed
@BenjaminLangenakenSF BenjaminLangenakenSF deleted the run-test-action branch January 20, 2026 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants