Skip to content

Conversation

@avylove
Copy link
Collaborator

@avylove avylove commented Oct 15, 2025

Added initial testing for 3.15-rc. Most of the tests are failing for.

DeprecationWarning: This process (pid=238) is multi-threaded, use of forkpty() may lead to deadlocks in the child.

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.

@codecov
Copy link

codecov bot commented Oct 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.30%. Comparing base (496071b) to head (dd35a3c).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jquast
Copy link
Owner

jquast commented Oct 15, 2025

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.

@avylove
Copy link
Collaborator Author

avylove commented Oct 15, 2025

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.

@jquast
Copy link
Owner

jquast commented Oct 16, 2025

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 ' ' item, only for versions and parametrized test combinations affected. This should cause all tests to pass on all python versions and still retain full coverage.

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

@avylove
Copy link
Collaborator Author

avylove commented Oct 16, 2025

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.

@jquast jquast mentioned this pull request Oct 20, 2025
@avylove
Copy link
Collaborator Author

avylove commented Oct 31, 2025

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 _wrap_chunks() to match the 3.15 behavior, then added a workaround for older versions. It currently looks for 3.15+ specifically, so will need to be updated when 3.13 and 3.14 are updated. Another option is to generalize it and implement the workaround any time the first line is only whitespace. Thoughts?

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 max_lines and placeholder options, so I added a keyword combinations for that in the tests.

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.

_handle_long_words() got a minor change, but I might take another look and see if I can get break_on_hyphens to work

@jquast
Copy link
Owner

jquast commented Oct 31, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants