Skip to content

Fix test assertions that never compared against the expected value - #161774

Merged
StanFromIreland merged 2 commits into
python:mainfrom
andresdelfino:fix-asserttrue-misuse
Nov 11, 2026
Merged

StanFromIreland merged 2 commits into
python:mainfrom
andresdelfino:fix-asserttrue-misuse

Conversation

@andresdelfino

Copy link
Copy Markdown
Contributor

assertTrue(first, second) treats second as the failure message, so each of these assertions only checked that first was truthy and never compared it with the intended value. This switches them to assertEqual() or assertIs().

They were found by walking the AST of every tracked .py file for assertTrue/assertFalse calls with two positional arguments and reviewing each hit by hand; the remaining ones pass a genuine failure message.

Two of the corrected assertions would have failed as-is, because the expected value had gone stale:

  • test_ssl.test_ecc_cert: TLS 1.3 is negotiated here, and TLS 1.3 cipher suite names (e.g. TLS_AES_256_GCM_SHA384) don't include the key exchange or authentication algorithm, so the ECDHE-ECDSA prefix check only makes sense for TLS 1.2. It is now done only when TLS 1.2 is negotiated. test_dual_rsa_ecc pins TLS 1.2 and keeps the unconditional check. Dropping the check from test_ecc_cert instead would also be reasonable.
  • test_ttk_textonly.test_format_elemcreate: _format_elemcreate(None) has returned ((),) rather than (None, ()) since ttk.Style.element_create using incorrect tk.call syntax #68166 changed the function's return format; the expected value is updated to match.

In the ctypes tests the expected values were str while the functions return bytes, so they are now bytes literals.

Test-only change, so this should be fine for skip issue / skip news. Tested locally on Linux with OpenSSL 3.0.13; test_ttk_textonly was not run locally (no _tkinter).

🤖 Generated with Claude Code

assertTrue(first, second) treats the second argument as the failure
message, so these assertions only checked that the first argument was
truthy.  Use assertEqual() or assertIs() so the expected value is
actually compared.

Two of them were hiding stale expectations:

* test_ssl.test_ecc_cert: since TLS 1.3 is negotiated, the cipher suite
  name no longer starts with "ECDHE-ECDSA".  Only check it for TLS 1.2.
* test_ttk_textonly.test_format_elemcreate: _format_elemcreate(None)
  returns ((),) since pythongh-68166 changed its return format.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread Lib/test/test_ssl.py Outdated
self.assertTrue(cert, "Can't get peer certificate.")
cipher = s.cipher()[0].split('-')
self.assertTrue(cipher[:2], ('ECDHE', 'ECDSA'))
# TLS 1.3 cipher suite names don't include the key exchange

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now duplicates test_dual_rsa_ecc below, let's drop this.

Comment thread Lib/test/test_ssl.py Resolved
@StanFromIreland StanFromIreland changed the title Fix assertTrue() calls that pass the expected value as msg Fix test assertions that never compared against the expected value Nov 11, 2026
@StanFromIreland
StanFromIreland enabled auto-merge (squash) November 11, 2026 18:16
@StanFromIreland
StanFromIreland merged commit aecef70 into python:main Nov 11, 2026
50 checks passed
@andresdelfino
andresdelfino deleted the fix-asserttrue-misuse branch November 11, 2026 19:08
@StanFromIreland StanFromIreland added needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes labels Nov 11, 2026
@miss-islington-app

Copy link
Copy Markdown

Thanks @andresdelfino for the PR, and @StanFromIreland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app

Copy link
Copy Markdown

Thanks @andresdelfino for the PR, and @StanFromIreland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖

@bedevere-app

bedevere-app Bot commented Nov 11, 2026

Copy link
Copy Markdown

GH-161823 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 Nov 11, 2026
@bedevere-app

bedevere-app Bot commented Nov 11, 2026

Copy link
Copy Markdown

GH-161824 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label Nov 11, 2026
StanFromIreland added a commit that referenced this pull request Nov 11, 2026
…alue (GH-161774) (#161823)

(cherry picked from commit aecef70)

Co-authored-by: Andrés Delfino <adelfino@gmail.com>
Co-authored-by: Stan Ulbrych <stan@python.org>
StanFromIreland added a commit that referenced this pull request Nov 11, 2026
…alue (GH-161774) (#161824)

(cherry picked from commit aecef70)

Co-authored-by: Andrés Delfino <adelfino@gmail.com>
Co-authored-by: Stan Ulbrych <stan@python.org>
orsenthil pushed a commit to orsenthil/cpython that referenced this pull request Nov 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip issue skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants