-
Notifications
You must be signed in to change notification settings - Fork 74
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
misc: fix tests that secretly rely on optional dependencies #3361
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.
👍
tests/interactive/test_app.py
Outdated
# Mock pyclip_copy for testing | ||
_copied_text: str | None = None | ||
|
||
|
||
def pyclip_copy(text: str) -> None: | ||
"""Mock version of pyclip_copy that stores copied text for inspection during tests""" | ||
global _copied_text | ||
_copied_text = text |
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.
Why was that necessary?
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.
Because pyclip is also an optional dependency, and it would be nice to actually test what the button copies, even if we don't right now. I could add this quickly
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 actually couldn't get this to work, and added a different API instead.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3361 +/- ##
==========================================
+ Coverage 90.09% 90.12% +0.02%
==========================================
Files 447 450 +3
Lines 56578 56809 +231
Branches 5431 5460 +29
==========================================
+ Hits 50975 51197 +222
- Misses 4166 4167 +1
- Partials 1437 1445 +8 ☔ View full report in Codecov by Sentry. |
…ect#3361) Also changes CI-Core to test only the core dev dependencies, letting CI-MLIR check the full coverage
Also changes CI-Core to test only the core dev dependencies, letting CI-MLIR check the full coverage