Skip to content

Add error handling for sgx ci #4222

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add error handling for sgx ci #4222

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 16 additions & 0 deletions .github/workflows/compilation_on_sgx.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,22 @@ jobs:

- name: run spec tests
run: |
set +e
source /opt/intel/sgxsdk/environment
./test_wamr.sh ${{ matrix.test_option }} -t ${{ matrix.running_mode }}
exitcode="$?"

if [ $exitcode -eq 143 ]; then
echo "$exitcode is a known GitHub-hosted runner issue"
echo "::notice::Bypass an error with code 143 in SGX CI"
exit 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

doesn't this risk us to merge broken changes?

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 believe it will not introduce any other unexpected breaking changes.

  • Only bypass exit code 143. Other non-zero codes will still be captured.
  • Spec_test scripts, all.py and runtest.py, will clamp return codes, and 143 is not one of them.
  • 143 is not an errno in errno.h

Copy link
Collaborator

Choose a reason for hiding this comment

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

well, my concern is:

  1. someone submits a PR with broken changes
  2. the ci happened to pass because of this workaround
  3. reviewers don't notice it and merge

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Based on my observations, if there is an error from the spec-test itself, 143 won't be generated. I believe the 143 code will not cover other errors. Those accidental 143 errors are quite annoying, so I want to relieve our daily work. If directly bypassing is too aggressive, how about automatically rerunning if it fails?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Those accidental 143 errors are quite annoying, so I want to relieve our daily work.

i agree.

If directly bypassing is too aggressive, how about automatically rerunning if it fails?

it seems nicer.

fi

if [ $exitcode -ne 0 ]; then
echo "Spec test failed with error code $exitcode"
exit 1
fi

echo "Spec test passed"
exit 0
working-directory: ./tests/wamr-test-suites
Loading