-
Notifications
You must be signed in to change notification settings - Fork 0
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
chore: enhance testing CI/CD #117
base: main
Are you sure you want to change the base?
Conversation
Warning Rate limit exceeded@jjjermiah has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request updates the testing configuration and workflow across the project. In the CI/CD workflow, the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CI as CI/CD Pipeline
participant Executor as Test Executor
participant Summary as JUnit Summary Step
participant Uploader as Coverage Uploader
Dev->>CI: Push commit with changes
CI->>Executor: Start Unit-Tests job
Executor->>Executor: Run tests with "pixi run -e ${{ matrix.env }} test -s -vv"
Executor->>Summary: Always execute test-summary step
Summary-->>CI: Produce JUnit test report
CI->>Uploader: Conditionally upload coverage report
Uploader-->>CI: Coverage artifact processed
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…w for closed pull requests
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
config/pytest.ini (1)
63-69
: Consider adding more specific warning filters.While the current warning filters are good, consider being more specific about which DeprecationWarnings to ignore.
filterwarnings = - ignore::DeprecationWarning + ignore::DeprecationWarning:pytest.* + ignore::DeprecationWarning:_pytest.* # Add specific warning messages to ignore here ignore::UserWarning:pydicom.*.github/workflows/ci-cd.yml (1)
44-50
: Fix trailing spaces in YAML.Remove trailing spaces to maintain consistent formatting.
- name: JUnit Test Summary id: pytest-summary uses: test-summary/action@v2 - with: + with: paths: .cache/test-results/**/*.xml show: "fail,skip" if: always()🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 47-47: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pixi.lock
is excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/ci-cd.yml
(2 hunks)config/pytest.ini
(1 hunks)pyproject.toml
(1 hunks)tests/test_feature_extraction.py
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/ci-cd.yml
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 47-47: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Unit-Tests (windows-latest, py312)
- GitHub Check: Unit-Tests (windows-latest, py311)
- GitHub Check: Unit-Tests (windows-latest, py310)
- GitHub Check: Unit-Tests (macos-14, py312)
- GitHub Check: Unit-Tests (macos-14, py311)
- GitHub Check: Unit-Tests (macos-14, py310)
- GitHub Check: Unit-Tests (macos-latest, py311)
- GitHub Check: Unit-Tests (macos-latest, py310)
🔇 Additional comments (7)
tests/test_feature_extraction.py (2)
47-57
: LGTM! Improved fixture management with module scope and safer cleanup.The changes enhance test reliability by:
- Using module scope to reduce fixture setup/teardown overhead
- Adding existence check before cleanup to prevent errors during parallel test execution
58-68
: LGTM! Consistent fixture improvements applied.The same improvements have been correctly applied to the lung4DMetadataPath fixture.
config/pytest.ini (3)
1-11
: LGTM! Well-structured basic configuration.The configuration properly sets:
- Minimum pytest version for compatibility
- Cache directory for test artifacts
- Progress-style console output for better visibility
12-45
: LGTM! Comprehensive test execution configuration.The addopts section effectively configures:
- Detailed test output with --showlocals
- Coverage reporting in multiple formats
- Parallel test execution with load grouping
- JUnit XML reporting for CI integration
51-61
: LGTM! Clear test discovery patterns.Well-defined patterns for:
- Test paths
- Excluded directories
- Test file naming conventions
pyproject.toml (1)
76-78
: LGTM! Simplified test execution configuration.Test command now properly uses the centralized pytest.ini configuration, making it easier to maintain and modify test settings.
.github/workflows/ci-cd.yml (1)
32-34
: LGTM! Enhanced test execution visibility.Added verbosity flags (-s -vv) to improve test output in CI logs.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #117 +/- ##
===========================================
+ Coverage 48.12% 83.39% +35.26%
===========================================
Files 34 34
Lines 1492 1734 +242
===========================================
+ Hits 718 1446 +728
+ Misses 774 288 -486 ☔ View full report in Codecov by Sentry. |
copied over the pytest config from med-imagetools
can easily view errors in gha during pytest without going into the action logs
small fix to the fixtures that delete temporary paths and are used by multiple tests (can cause errors during parallel runs)
also cleanups for each PRs gha caches
Summary by CodeRabbit