gh-71339: Use new assertion methods in tests #129046
Conversation
|
Not sure if it's me or not, but having that many files to review makes my GH interface crash (or load very slowly). Would it be possible to split this PR, at least per folders? just so that 1) it reduces the number of review requests 2) it helps us to review if possible? |
|
I have already already created 15 other PRs for groups of files with significant number of changes. But it is not worth to create a PR for 5 changed lines in 2 files. |
|
Merging in |
|
(Once #129060 has been merged) |
Doh, of course; in my head, CI was already done and my PR was merged 😆 |
Lib/test/test_typing.py
Outdated
| @@ -3251,14 +3251,14 @@ def x(self): pass | |||
| class EmptyProtocol(Protocol): pass | |||
|
|
|||
| @runtime_checkable | |||
| class SupportsStartsWith(Protocol): | |||
| class SupportsStartswith(Protocol): | |||
There was a problem hiding this comment.
Not sure why this name was changed, could you undo changes to this file?
There was a problem hiding this comment.
Good catch. I initially wrote all this for assertStartsWith. Then I replaced "With" with "with". Then I replaced back "with" with "With", but used more strict patterns to avoid false positives, so some names left changed.
There was a problem hiding this comment.
I'll trust you on this one as they are so many files to review (I've reviewed the first 30 files and my file). I assume you actually used a regex or something like that to replace the occurrences but if the test suite is running fine I think we're good. To be sure I'd run this with build bots as well in case local testing was skipping some tests.
Instead of splitting the PR by folders I should have probably asked to split it per usage (namely one PR for hasattr tests, one for issubclass tests, etc) but let's keep this one now that we have it (if there are more PRs after this one, just keep in mind that it's easier to review the same mechanical change across multiple files rather than multiple mechanical changes)
|
(Oh maybe I shouldn't have scheduled build bots if you had other commits... sorry in advance, though I can look at the buildbots status page) |
|
I was checking test_idle when I found this PR in my mailbox. (EDIT) 10 rather than just 2 idle tests can be changed and I want to backport them. I think it easiest if I make a separate PR (DONE, #129213) attached to the same issue I copied the 2 changes made here so there should be no conflict. EDIT The IDLE changes are merged and backported. |
|
I non-recursively searched This is the 1 AMD64 Android failure. |
|
The patch was initially created in 2016. It was then updated few years later, and just before creating this PR. Since the IDLE tests are located outside of the |
|
!buildbot ios |
|
!buildbot android |
|
!buildbot android |
|
Not sure what happened before with Android and iOS, but they both seem fine now. |
|
It was an issue in different tests which was fixed by #129133. No need to trigger buildbots. This PR is not emergent. It will be merged after merging all related PRs. Some changes can be extracted into separate PRs (like IDLE's). |
They provide better error report.
Only 1-2 lines are changed in the half of files, 3-4 lines in other quarter of files, and only in about 10 files there are more than 10 changed lines. So all this have been united in a single PR.