-
Notifications
You must be signed in to change notification settings - Fork 77
Add optional testing for Python 3.15 #311
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #311 +/- ##
==========================================
+ Coverage 98.29% 98.30% +0.01%
==========================================
Files 11 11
Lines 2460 2483 +23
Branches 423 432 +9
==========================================
+ Hits 2418 2441 +23
Misses 32 32
Partials 10 10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Sure, just ignore it within the as_subprocess context manager looks like it would take care of it all, my pending PR's have a new pty helper that it would also be ignored in that covers the manual uses of pty.fork. It is safe to ignore, we are not calling urllib/ssl etc. |
|
That fixed most of them. Two cases of test_SequenceWrapper() are still failing. I need to dig in more, but wondering if it's related to python/cpython@1c598e0 since it's the only change since 3.14. |
|
It does look from the test this is the cause, we should adapt the fix into our own wrap() whether or not it is, unfortunately our wrap method is duplicated from upstream and this test is part of ensuring it behaves the same. This will "invert" the test results, failing for versions 3.13 and under. From there, I think it is best for code coverage to do a "fixup" in the tests for versions under 3.14, the standard library wrap result is modified for the extra Older versions of python would benefit from a slightly improved term.wrap() fix, that standard library textwrap.wrap does not have, if i understand correctly |
|
I was thinking I'd submit an issue and see if the change was an unintended side effect. It seems like it might be. But I was going to wait until I had a native package to generate examples from. I would expect that in a few weeks. |
|
It looks like the new behavior will stay and it's currently expected to be in 3.13 and 3.14 when they do their next point release. I updated most of Since the new behavior isn't in any released versions, it probably makes sense to old off on this until 3.13 and 3.14 are updated, or at least 3.14. This should only affect cases where the preceding whitespace is longer than the width. The updated logic should now handle the There are some uncovered lines in the tests. I need to look at that. It probably needs a new test rather than updating the keyword args.
|
|
whatever code is simplest or easiest to support for older versions, its ok if it mismatches behavior if its easier, and yes, it should be held off until such change is at least released in std python |
Added initial testing for 3.15-rc. Most of the tests are failing for.
There is an entry about this in the docs that references a discussion thread.
I haven't dug into this because I don't have a system package for 3.15 yet. One option is to ignore the warning because this is testing and it has worked fine for years. Just figured it would be good to have it on the radar.