-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Update Lib/test/support/__init__.py
from 3.13.5
#6037
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
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
@@ -6,8 +6,8 @@ | |||
import contextlib | |||
import dataclasses | |||
import functools | |||
import getpass | |||
import opcode | |||
import logging |
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.
The CI fails on wasi because logging
uses time.time_ns
which is not available on wasi (yet).
How should I approach this? should I patch this here so we won't crash if this raises an AttributeError
, or should this PR be blocked until we implement time.time_ns
for wasi?
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.
That's up to the situation. In this case, we already have _time
for wasi, so it must be easy to add time_ns in wasi. (it will be inaccurate though) So I will add time_ns in this case.
If it was not easy, I would try to patch logging a little bit to ignore time_ns or to use time
@@ -121,7 +121,6 @@ mod decl { | |||
Ok(()) | |||
} | |||
|
|||
#[cfg(not(target_os = "wasi"))] |
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.
This is what I ended up doing. This line was inserted 4 years ago. maybe things changed since then? now it passes for wasi 🤷 .
Usure if this is a correct fix for the issue
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 good
@coderabbitai generate unit tests |
An unexpected error occurred during Unit Test Generation. |
✅ Actions performedUnit Test Generation started... |
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.
Thank you so much. This is a big step to support other tests
@@ -121,7 +121,6 @@ mod decl { | |||
Ok(()) | |||
} | |||
|
|||
#[cfg(not(target_os = "wasi"))] |
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 good
Summary by CodeRabbit