◐ Shell
reader mode source ↗
Skip to content

gh-71339: Use new assertion methods in tests#129046

Merged
serhiy-storchaka merged 12 commits into
python:mainfrom
serhiy-storchaka:use-extra-assertions
May 22, 2025
Merged

gh-71339: Use new assertion methods in tests#129046
serhiy-storchaka merged 12 commits into
python:mainfrom
serhiy-storchaka:use-extra-assertions

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented Jan 20, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

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.

@serhiy-storchaka serhiy-storchaka added tests Tests in the Lib/test dir skip news labels Jan 20, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Jan 20, 2025
Closed
@picnixz

picnixz commented Jan 20, 2025

Copy link
Copy Markdown
Member

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?

@serhiy-storchaka

serhiy-storchaka commented Jan 20, 2025

Copy link
Copy Markdown
Member Author

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.

@serhiy-storchaka serhiy-storchaka marked this pull request as ready for review January 20, 2025 12:04
29 hidden items Load more…
@terryjreedy

terryjreedy commented Jan 23, 2025 •
edited by mhsmith
Loading

Copy link
Copy Markdown
Member

I non-recursively searched self.assert.*(issubclass|hasattr|startswith|endswith) inF:\dev\3x\Lib\test\test_t*.py (with IDLE) and found the same change candidates in the test_t*.py files included here (and only a couple of false positives). The replacements all looked right.

This is the 1 AMD64 Android failure.

======================================================================
FAIL: testAssertHasAttr (test.test_unittest.test_case.Test_TestCase.testAssertHasAttr)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/test/test_unittest/test_case.py", line 803, in testAssertHasAttr
    with self.assertRaises(self.failureException) as cm:
         ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: AssertionError not raised

@serhiy-storchaka

serhiy-storchaka commented Jan 23, 2025

Copy link
Copy Markdown
Member Author

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 Lib/test tree, I missed new tests there.

@mhsmith

mhsmith commented Jan 23, 2025

Copy link
Copy Markdown
Member

!buildbot ios

@bedevere-bot

bedevere-bot commented Jan 23, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @mhsmith for commit cc1f37a 🤖

The command will test the builders whose names match following regular expression: ios

The builders matched are:

  • iOS ARM64 Simulator PR

@mhsmith

mhsmith commented Jan 23, 2025

Copy link
Copy Markdown
Member

!buildbot android

@bedevere-bot

bedevere-bot commented Jan 23, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @mhsmith for commit cc1f37a 🤖

The command will test the builders whose names match following regular expression: android

The builders matched are:

  • aarch64 Android PR
  • AMD64 Android PR

@mhsmith

mhsmith commented Jan 23, 2025

Copy link
Copy Markdown
Member

!buildbot android

@bedevere-bot

bedevere-bot commented Jan 23, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @mhsmith for commit cc1f37a 🤖

The command will test the builders whose names match following regular expression: android

The builders matched are:

  • AMD64 Android PR
  • aarch64 Android PR

@mhsmith

mhsmith commented Jan 23, 2025

Copy link
Copy Markdown
Member

Not sure what happened before with Android and iOS, but they both seem fine now.

@serhiy-storchaka

serhiy-storchaka commented Jan 24, 2025

Copy link
Copy Markdown
Member Author

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).

@kumaraditya303 kumaraditya303 removed their request for review February 21, 2025 18:16
@brettcannon brettcannon removed their request for review April 14, 2025 17:01
@serhiy-storchaka

serhiy-storchaka commented May 22, 2025

Copy link
Copy Markdown
Member Author

There have been practically no conflicts in the last few months. This means that this PR affects a very small part of the code that is actively changing. So I plan to merge this PR without backporting to 3.13. Probability of conflicts in future backports to 3.13 is low.

Hide details View details @serhiy-storchaka serhiy-storchaka merged commit 2602d8a into python:main May 22, 2025
38 checks passed
@serhiy-storchaka serhiy-storchaka deleted the use-extra-assertions branch May 22, 2025 10:17
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.14 bugs and security fixes label May 22, 2025
@miss-islington-app

miss-islington-app Bot commented May 22, 2025

Copy link
Copy Markdown

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 22, 2025
@serhiy-storchaka @miss-islington
pythongh-71339: Use new assertion methods in tests (pythonGH-129046) …
1b9bb43 
(cherry picked from commit 2602d8a)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app

bedevere-app Bot commented May 22, 2025

Copy link
Copy Markdown

GH-134498 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label May 22, 2025
serhiy-storchaka added a commit that referenced this pull request May 22, 2025
@miss-islington @serhiy-storchaka
[3.14] gh-71339: Use new assertion methods in tests (GH-129046) (GH-1… …
db98e0b 
…34498)

(cherry picked from commit 2602d8a)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
lkollar pushed a commit to lkollar/cpython that referenced this pull request May 26, 2025
@serhiy-storchaka @lkollar
pythongh-71339: Use new assertion methods in tests (pythonGH-129046)
09a063d
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@serhiy-storchaka @Pranjal095
pythongh-71339: Use new assertion methods in tests (pythonGH-129046)
90c73e9
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@serhiy-storchaka @taegyunkim
pythongh-71339: Use new assertion methods in tests (pythonGH-129046)
e557711
gostak-dd pushed a commit to gostak-dd/cpython that referenced this pull request Jun 2, 2026
@serhiy-storchaka @gostak-dd
pythongh-71339: Use new assertion methods in tests (pythonGH-129046)
a20a70d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments

@picnixz picnixz picnixz approved these changes

@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner

@barneygale barneygale Awaiting requested review from barneygale barneygale is a code owner

@FFY00 FFY00

@ethanfurman ethanfurman Awaiting requested review from ethanfurman ethanfurman is a code owner

@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood

@terryjreedy terryjreedy

@ericvsmith ericvsmith Awaiting requested review from ericvsmith ericvsmith is a code owner

@rhettinger rhettinger Awaiting requested review from rhettinger rhettinger is a code owner

@gpshead gpshead Awaiting requested review from gpshead gpshead is a code owner

@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag

@Eclips4 Eclips4 Awaiting requested review from Eclips4 Eclips4 is a code owner

@pablogsal pablogsal Awaiting requested review from pablogsal

@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner

@pganssle pganssle Awaiting requested review from pganssle pganssle is a code owner

@abalkin abalkin Awaiting requested review from abalkin

@ericsnowcurrently ericsnowcurrently ericsnowcurrently is a code owner

@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner

@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner

@vsajip vsajip Awaiting requested review from vsajip vsajip is a code owner

@tiran tiran Awaiting requested review from tiran

@iritkatriel iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner

@corona10 corona10 Awaiting requested review from corona10 corona10 is a code owner

@1st1 1st1 Awaiting requested review from 1st1

@savannahostrowski savannahostrowski Awaiting requested review from savannahostrowski savannahostrowski is a code owner

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@serhiy-storchaka @picnixz @erlend-aasland @bedevere-bot @terryjreedy @mhsmith @JelleZijlstra