-
Notifications
You must be signed in to change notification settings - Fork 32
🐛 unit-, integration- and system- tests don't fail on cancellation
#8032
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
🐛 unit-, integration- and system- tests don't fail on cancellation
#8032
Conversation
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.
Pull Request Overview
This PR refines a module docstring and enhances the CI workflow to treat cancelled jobs as failures.
- Standardizes the module-level docstring in
_meta.py - Expands CI test steps to fail on cancelled jobs in unit, integration, and system tests
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| services/director-v2/src/simcore_service_director_v2/_meta.py | Clean up and consolidate the module docstring |
| .github/workflows/ci-testing-deploy.yml | Update test-failure conditions to include cancelled results |
Comments suppressed due to low confidence (1)
.github/workflows/ci-testing-deploy.yml:1548
- [nitpick] Consider assigning
join(needs.*.result, ',')to an environment variable or step output to avoid repeating the expression and improve readability.
if: ${{ contains(join(needs.*.result, ','), 'failure') || contains(join(needs.*.result, ','), 'cancelled') }}
pcrespov
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.
i gues you want to reasign
unit-, integration- and system- tests don't fail on cancellation
? |
matusdrobuliak66
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.
Thanks!
bisgaard-itis
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.
Very nice! Thanks for catching this 🐟
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8032 +/- ##
===========================================
- Coverage 85.96% 69.79% -16.17%
===========================================
Files 1853 696 -1157
Lines 71495 32833 -38662
Branches 1258 176 -1082
===========================================
- Hits 61459 22917 -38542
- Misses 9672 9858 +186
+ Partials 364 58 -306
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
sanderegg
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.
thanks!
|
@Mergifyio queue |
🟠 Waiting for conditions to match
|
0a6eef0
into
ITISFoundation:master



What do these changes do?
This PR fixes GitHub actions in order to include the case in which jobs are "cancelled" as a condition to make cumulative
unit-tests,integration-testsandsystem-testsfailing.Related issue/s
How to test
Dev-ops