Skip to content

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

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

Merged
merged 12 commits into from
May 22, 2025

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jan 20, 2025

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.

@picnixz
Copy link
Member

picnixz commented Jan 20, 2025

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
Copy link
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.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 21, 2025
@picnixz
Copy link
Member

picnixz commented Jan 21, 2025

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

@terryjreedy
Copy link
Member

terryjreedy commented Jan 23, 2025

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.

@terryjreedy
Copy link
Member

terryjreedy commented Jan 23, 2025

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
Copy link
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
Copy link
Member

mhsmith commented Jan 23, 2025

!buildbot ios

@bedevere-bot
Copy link

🤖 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
Copy link
Member

mhsmith commented Jan 23, 2025

!buildbot android

@bedevere-bot
Copy link

🤖 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
Copy link
Member

mhsmith commented Jan 23, 2025

!buildbot android

@bedevere-bot
Copy link

🤖 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
Copy link
Member

mhsmith commented Jan 23, 2025

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

@serhiy-storchaka
Copy link
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
Copy link
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.

@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
Copy link

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
(cherry picked from commit 2602d8a)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2025

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
lkollar pushed a commit to lkollar/cpython that referenced this pull request May 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants