Repository navigation
Fix test assertions that never compared against the expected value - #161774
Merged
StanFromIreland merged 2 commits intoNov 11, 2026
Merged
Conversation
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>
andresdelfino
requested review from
barneygale,
ethanfurman,
gpshead and
picnixz
as code owners
November 10, 2026 18:30
ethanfurman
approved these changes
Nov 10, 2026
| 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 |
Member
There was a problem hiding this comment.
This now duplicates test_dual_rsa_ecc below, let's drop this.
StanFromIreland
approved these changes
Nov 11, 2026
StanFromIreland
enabled auto-merge (squash)
November 11, 2026 18:16
|
Thanks @andresdelfino for the PR, and @StanFromIreland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
|
Thanks @andresdelfino for the PR, and @StanFromIreland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15. |
|
GH-161823 is a backport of this pull request to the 3.14 branch. |
|
GH-161824 is a backport of this pull request to the 3.15 branch. |
This was referenced Nov 11, 2026
orsenthil
pushed a commit
to orsenthil/cpython
that referenced
this pull request
Nov 12, 2026
…ython#161774) Co-authored-by: Stan Ulbrych <stan@python.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
assertTrue(first, second)treatssecondas the failure message, so each of these assertions only checked thatfirstwas truthy and never compared it with the intended value. This switches them toassertEqual()orassertIs().They were found by walking the AST of every tracked
.pyfile forassertTrue/assertFalsecalls 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 theECDHE-ECDSAprefix check only makes sense for TLS 1.2. It is now done only when TLS 1.2 is negotiated.test_dual_rsa_eccpins TLS 1.2 and keeps the unconditional check. Dropping the check fromtest_ecc_certinstead 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
strwhile the functions returnbytes, 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_textonlywas not run locally (no_tkinter).🤖 Generated with Claude Code