-
Notifications
You must be signed in to change notification settings - Fork 373
Add CI testing for slangpy-samples #8938
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
Add CI testing for slangpy-samples #8938
Conversation
Add a new `test-slangpy-examples` job to the CI workflow that tests the slangpy-samples repository to provide integration testing of slangpy with end-to-end examples. The new job: - Runs in parallel with existing `test-slangpy` job - Clones shader-slang/slangpy-samples repository - Installs slangpy and required dependencies - Executes pytest with 4 parallel workers - Runs on Windows (debug+release) and macOS (release) with GPU support
Update the test-slangpy-examples job to look for main.py as a fallback
if the example script matching the directory name is not found.
Supports both naming conventions:
- examples/{name}/{name}.py
- examples/{name}/main.py
Move slangpy-samples example execution from workflow to standalone script (extras/run-slangpy-examples.sh) with platform-specific skiplist support (extras/expected-slangpy-example-failure.txt). This also simplifies workflow yaml file and enables local testing.
Install Bash 4+ via Homebrew on macOS runners (default Bash 3.2 lacks associative arrays). Add array safety checks to prevent "bad array subscript" errors on Windows when parsing empty skiplist entries.
Add skiplist for windowing examples that fail on headless CI. Fix macOS by installing coreutils and using gtimeout instead of timeout. Changes: - Skip window, fwd-rasterizer, bitmap, render_pipeline, raytracing - Skip pathtracer (expensive), jupyter (needs kernel) - Install coreutils on macOS for gtimeout command - Auto-detect and use correct timeout command per platform
Self-hosted runners now attempt pip install but continue on failure. Add PyTorch examples to skiplist (too large/slow for CI).
Skip rendering, PyTorch, and windowing examples that fail in CI. Run core headless examples that test fundamental slangpy functionality. Can enable more examples incrementally later.
Use pure bash timeout function and string matching instead of requiring Bash 4+ and coreutils. This results no external dependencies, simpler code and faster CI.
Inline comments were being captured as part of example names, preventing proper skiplist matching. Now strips everything after '#' before parsing example names.
Example tries to connect to tev image viewer, causing timeouts in headless CI environments.
|
Ready for review. |
.github/workflows/ci-slang-test.yml
Outdated
| python -m pip install pytest-github-actions-annotate-failures --user | ||
| python -m pip install pytest-xdist --user | ||
| - name: Run slangpy-samples examples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like most of the set up are identical to the test-slangpy job above, and this job takes only ~1min. I'm wondering if we should combine this test-slangpy-example into the test-slangpy to reduce the duplicates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, parallelization does not give much runtime benefit in such short runs. I'll fix that and combine it with slangpy-tests.
Implements testing of slangpy-samples as requested in issue shader-slang#8186. The implementation follows the original issue's preferred solution: clone the slangpy-samples repository, install dependencies, and run pytest tests. Changes: - Added slangpy-samples testing to test-slangpy job - Runs 'pytest -n 4 slangpy-samples/tests' for parallel execution - Combined with slangpy pytest tests to reuse setup and reduce CI overhead The tests run alongside slangpy's own pytest tests when full-gpu-tests is enabled on pull requests or release builds.
|
Fixed as suggested. I first implemented enabling slangpy-samples examples instead of test in that repository and that caused extra complexity. |
|
Looks good to me. |
| - name: Install slangpy-samples dependencies | ||
| run: | | ||
| echo "Installing slangpy-samples dependencies..." | ||
| python -m pip install -r slangpy-samples/requirements.txt --user |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used to run into permission issues running the pip install in self-hosted machines if the packages have already been installed, no matter if --user flag is used or not.
Can you double check that? If the issue is resolved, we can remove if [[ ! "${{ inputs.runs-on }}" =~ self-hosted ]] in the Run slangpy tests step
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Removed the check in this PR.
gtong-nv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much cleaner after combining the jobs! Looks good to me!
Just left a request. It's fine to file another PR if you would like.
The conditional check is no longer needed as permission issues have been resolved. Slangpy-samples dependencies already install on self-hosted runners without issues.
Thanks for review. Uploaded one more change to remove the check from |
gtong-nv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Adds testing for the [slangpy-samples](https://github.com/shader-slang/slangpy-samples) repository to provide integration testing of slangpy with real-world examples. # Implementation The `test-slangpy` job now includes slangpy-samples testing alongside the existing slangpy pytest tests. After running slangpy's own tests, the job: 1. Clones the `slangpy-samples` repository 2. Installs dependencies from `requirements.txt` 3. Runs `pytest -n 4 slangpy-samples/tests` (parallel execution with 4 workers) The implementation follows the [original issue's preferred solution](#8186 (comment)). # When It Runs Same conditions as the main slangpy tests, ensuring we test integration examples alongside the core slangpy functionality. Fixes #8186
Adds testing for the slangpy-samples repository to provide integration testing of slangpy with real-world examples.
Implementation
The
test-slangpyjob now includes slangpy-samples testing alongside the existing slangpy pytest tests. After running slangpy's own tests, the job:slangpy-samplesrepositoryrequirements.txtpytest -n 4 slangpy-samples/tests(parallel execution with 4 workers)The implementation follows the original issue's preferred solution.
When It Runs
Same conditions as the main slangpy tests, ensuring we test integration examples alongside the core slangpy functionality.
Fixes #8186