Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add callback to log the ERROR status for the error job #14

Merged
merged 2 commits into from
Mar 28, 2024
Merged

add callback to log the ERROR status for the error job #14

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0e10999
add callback to log the ERROR status for the error job
gnarf Mar 27, 2024
1390571
remove expression syntax test
gnarf Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions .github/workflows/nvda-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,23 @@ jobs:

- name: Run harness
shell: powershell
continue-on-error: true
env:
NVDA_PORTABLE_ZIP: ${{ fromJson(steps.nvda-portable-download.outputs.downloaded_files)[0] }}
run: |
& .\run-tester.ps1

- name: Log job state ERROR
shell: powershell
if: failure() && inputs.status_url
run: |
$headerbits = $env:ARIA_AT_CALLBACK_HEADER -split ":\s*", 2
$headers = @{$headerbits[0]=$headerbits[1]; "Content-Type" = "application/json"}
$body = @{'status'='ERROR'; 'externalLogsUrl'="$env:GITHUB_SERVER_URL/$env:GITHUB_REPOSITORY/actions/runs/$env:GITHUB_RUN_ID"} | ConvertTo-JSON
Invoke-WebRequest $env:ARIA_AT_STATUS_URL -Headers $headers -Method 'POST' -Body $body

- name: Log job state COMPLETED
shell: powershell
if: inputs.status_url
if: success() && inputs.status_url
run: |
$headerbits = $env:ARIA_AT_CALLBACK_HEADER -split ":\s*", 2
$headers = @{$headerbits[0]=$headerbits[1]; "Content-Type" = "application/json"}
Expand All @@ -193,6 +201,7 @@ jobs:

- name: upload *.{log,png}
uses: actions/upload-artifact@v4
if: always()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if: always()
if: ${{ always() }}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I actually believe it's preferred to not use the expression syntax for always by itself.

https://github.com/w3c/aria-at-app/pull/961/files#diff-fcc964e27d1e22339051a8ce39304eb2c721e3d96ab72d9fb20b4a2192f74b60R47

I seem to recall reading old issues saying that using the expression syntax with one of these alone was actually causing errors

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually - my memory was slightly incorrect, seems you can omit the expression syntax entirely.

https://stackoverflow.com/questions/58858429/how-to-run-a-github-actions-step-even-if-the-previous-step-fails-while-still-f#comment112735158_58859404

1390571

https://github.com/bocoup/aria-at-gh-actions-helper/actions/runs/8472751048/job/23215516701 is a test run with no expression braces at all

Copy link
Member

Choose a reason for hiding this comment

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

My preference is for the most consistent syntax even at the expense of verbosity for the same reason I don't lean on ASI when writing JS--honoring the exceptions results in more cognitive load and more risk than I think is warranted. I'm willing to be flexible on a few lines, though.

with:
name: logs
path: |
Expand Down
Loading