Repository navigation
Conversation
github-actions
Bot
added
github actions
project: none
| @@ -5,7 +5,7 @@ on: | |||
| inputs: | |||
| cmake_options: | |||
| type: string | |||
| projects_to_test: | |||
| project_to_test: | |||
There was a problem hiding this comment.
Please pick up the latest changes to the workflow files. This has been renamed to projects_to_test with c124e5c
| set -euo pipefail | ||
| EXTRA_OPTS="-DTHEROCK_ROCM_LIBRARIES_SOURCE_DIR=../ ${{ inputs.cmake_options }}" | ||
| PROJECT_KEY="$(echo "${{inputs.project_to_test }}" | tr '[:lower:]' '[:upper:]')" | ||
| if [[ "${{ inputs.test_type }}" == "coverage" ]]; then | ||
| EXTRA_OPTS="${EXTRA_OPTS} -D${PROJECT_KEY_UPPER}_ENABLE_COVERAGE=ON" | ||
| echo "Coverage enabled for ${PROJECT_KEY_UPPER}: ${EXTRA_OPTS}" | ||
| fi | ||
| export extra_cmake_options="${EXTRA_OPTS}" |
There was a problem hiding this comment.
Please move as much of this into the env block of this step, or if some processing needs to be done, It can be a prior, separate step that produces output values that can be fed into env.
| secrets: inherit | ||
| with: | ||
| cmake_options: ${{ matrix.projects.cmake_options }} | ||
| project_to_test: ${{ matrix.projects.project_to_test }} |
There was a problem hiding this comment.
Please see previous comment on projects_to_test. This input might contain multiple projects.
geomin12
left a comment
geomin12
left a comment
There was a problem hiding this comment.
idk if this PR is relevant anymore after our discussoin, but some comments
| @@ -101,6 +101,14 @@ jobs: | |||
| extra_cmake_options: "-DTHEROCK_ROCM_LIBRARIES_SOURCE_DIR=../ ${{ inputs.cmake_options }}" | |||
| BUILD_DIR: build | |||
| run: | | |||
| set -euo pipefail | |||
There was a problem hiding this comment.
does this apply for windows too? or not yet?
There was a problem hiding this comment.
We don't do code coverage on Windows. I don't think there would be any added value to doing this on Windows either
| @@ -101,6 +101,14 @@ jobs: | |||
| extra_cmake_options: "-DTHEROCK_ROCM_LIBRARIES_SOURCE_DIR=../ ${{ inputs.cmake_options }}" | |||
| BUILD_DIR: build | |||
| run: | | |||
| set -euo pipefail | |||
| EXTRA_OPTS="-DTHEROCK_ROCM_LIBRARIES_SOURCE_DIR=../ ${{ inputs.cmake_options }}" | |||
| PROJECT_KEY="$(echo "${{inputs.project_to_test }}" | tr '[:lower:]' '[:upper:]')" | |||
There was a problem hiding this comment.
could we get a quick comment explaining this logic?
| run: | | ||
| BUILD_DIR="/build/TheRock/rocm-libraries/${{ inputs.project_to_test }}/build" | ||
| cmake --build "${BUILD_DIR}" --target coverage || make -C "${BUILD_DIR}" coverage | ||
| - name: Upload coverage to Codecov |
38e3dde to
83a693e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #4441 +/- ##
============================================
+ Coverage 70.89% 80.50% +9.61%
============================================
Files 1106 388 -718
Lines 203899 49053 -154846
Branches 30141 6783 -23358
============================================
- Hits 144553 39490 -105063
+ Misses 47226 7286 -39940
+ Partials 12120 2277 -9843
🚀 New features to boost your workflow:
|
38e4fcd to
aed410b
Compare
|
Waiting on ROCm/TheRock#4931 |
|
This pull request has been inactive for 25 days and will be marked as stale. If you would like to keep this PR open, please:
This PR will be automatically closed in 5 days if no further activity occurs. |
|
This pull request has been automatically closed due to inactivity (30 days with no updates). If you'd like to continue working on this, feel free to reopen the PR or create a new one. |
Motivation
This PR adds a GitHub workflow to generate code coverage reports using TheRock CI.
Technical Details
Currently a work in progress. Current assumptions are:
project_to_testusingextra_cmake_optionsvariableproject_to_testwill expose thecoveragetarget somewhere within the build, currently it points to/build/TheRock/rocm-libraries/${{ inputs.project_to_test }}/build, which probably is incorrectTest Plan
We need to verify this CI works for a PR that touches one or multiple projects. We can make a small change that can be reverted before merge to trigger this pipeline to test the two scenarios.
Test Result
Submission Checklist