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.

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

Update CI to maitain the artifacts and add a upper job run limit #535

Merged
merged 4 commits into from
Oct 18, 2024
Merged
Changes from 3 commits
Commits
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
34 changes: 32 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ jobs:
ACT-sail-spike:
name: ACT-sail-spike (RV${{ matrix.xlen }})
runs-on: ubuntu-22.04
# Set a 15-minute time limit for this job
Copy link
Contributor

Choose a reason for hiding this comment

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

As we add other extensions to CI (like F) the duration of the CI will increase significantly and likely take more than 15 minutes. I wonder if we’re better off just letting it fail by timing out. Or at least setting a higher timeout time around 1 hour.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I updated the time limit to 30 minutes but surely we can update it to 60 minutes as well.

timeout-minutes: 15
strategy:
fail-fast: false
matrix:
Expand All @@ -42,8 +44,8 @@ jobs:
sudo apt-get update
sudo apt-get install -y python3 python3-pip python3-venv
sudo apt-get install -y gcc git autoconf automake libtool curl make unzip
sudo apt-get install autoconf automake autotools-dev curl python3 python3-pip libmpc-dev libmpfr-dev libgmp-dev gawk build-essential bison flex texinfo gperf libtool patchutils bc zlib1g-dev libexpat-dev ninja-build git cmake libglib2.0-dev libslirp-dev pkg-config
sudo apt-get install device-tree-compiler libboost-regex-dev libboost-system-dev
sudo apt-get install -y autoconf automake autotools-dev curl python3 python3-pip libmpc-dev libmpfr-dev libgmp-dev gawk build-essential bison flex texinfo gperf libtool patchutils bc zlib1g-dev libexpat-dev ninja-build git cmake libglib2.0-dev libslirp-dev pkg-config
sudo apt-get install -y device-tree-compiler libboost-regex-dev libboost-system-dev
pip3 install git+https://github.com/riscv/riscof.git

- name: Build RISCV-GNU Toolchain (${{ matrix.xlen }} bit)
Expand Down Expand Up @@ -134,3 +136,31 @@ jobs:
run: |
cd riscof-plugins/rv${{ matrix.xlen }}
riscof run --config config.ini --suite ../../riscv-test-suite/rv${{ matrix.xlen }}i_m/ --env ../../riscv-test-suite/env


- name: Check size of riscof_work folder
id: check_size
run: |
folder_size=$(du -sm /home/runner/work/riscv-arch-test/riscv-arch-test/riscof-plugins/rv${{ matrix.xlen }}/riscof_work | cut -f1)
echo "Folder size: ${folder_size} MB"
if [ "$folder_size" -gt 1000 ]; then
echo "Size exceeds 1 GB. Skipping upload."
echo "upload=false" >> $GITHUB_ENV # Set an environment variable to skip upload
else
echo "Size is within limit. Proceeding with upload."
echo "upload=true" >> $GITHUB_ENV # Set an environment variable to proceed with upload
fi

# Upload the riscof_work for rv${{ matrix.xlen }} folder if size check passes
- name: Upload the riscof_work for rv${{ matrix.xlen }} folder
# Proceed with upload only if the size is within limit
if: ${{ env.upload == 'true' }}
uses: actions/upload-artifact@v4
with:
name: riscof-test-report-rv${{ matrix.xlen }}
path: /home/runner/work/riscv-arch-test/riscv-arch-test/riscof-plugins/rv${{ matrix.xlen }}/riscof_work/
MuhammadHammad001 marked this conversation as resolved.
Show resolved Hide resolved
if-no-files-found: warn
retention-days: 3
Copy link
Contributor

Choose a reason for hiding this comment

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

Many of these are the defaults already so probably aren’t necessary to specify. Also, is there a reason to override the default, longer retention period? I believe the files will get deleted automatically if the repo runs out of space. 3 days seems kind of short as sometimes people don’t come back immediately to fix PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I thought it would be better to retain the artifacts only for 3 days since the developers usually need them in case of a failure and to save the space for other PRs as well. But, the default is 90 days, please let me know if you want me to change it to default and regarding the other default values, yes, I added include-hidden-files option in case someone wants to change in future but in our case, it's currently redundant, so I will remove that as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it runs out of space it will delete the oldest ones automatically, so deleting them earlier doesn't really save space for other PRs. Because of that I don't really see a reason to force them to be deleted early. @UmerShahidengr any opinion on this?

compression-level: 6
overwrite: true
include-hidden-files: false