test: migrate functional test equality asserts to `assert

l0rinc commented at 10:22 pm on March 8, 2026: contributor

Problem

Plain assert x == y is a poor fit for this test framework, assert_equal() gives more useful failure output and keeps equality checks consistent with the rest of the functional tests.

Design

A simple scripted diff cannot safely rewrite all of them because many files use == inside larger expressions, such as chained conditions, comprehensions, and other compound assertions. That makes a one-shot mechanical conversion either incorrect or harder to review.

Fix

This series first rewrites the non-mechanical cases into simpler assertions so they stand on their own and failures are easier to identify, converging on a simple scripted diff that is straightforward to verify.

The stack is structured so the manual cleanup comes first, the import-only setup comes immediately before it, and the final commit only touches the cases that are safe to rewrite mechanically.

Partially fixes https://github.com/bitcoin/bitcoin/issues/23119

DrahtBot added the label Tests on Mar 8, 2026

DrahtBot commented at 10:23 pm on March 8, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	maflcko
Concept ACK	kevkevinpal

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34287 ([RFC] test: integrate secp256k1lab as subtree and use it for low-level EC ops by theStack)
#21283 (Implement BIP 370 PSBTv2 by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

l0rinc force-pushed on Mar 8, 2026

DrahtBot added the label CI failed on Mar 8, 2026

DrahtBot removed the label CI failed on Mar 8, 2026

maflcko commented at 5:11 pm on March 10, 2026: member

Fix

Nerd-sniped by #34761,

Instead of referring to a random closed draft-pull request, it would be better to refer to the tracking issue, wihch is https://github.com/bitcoin/bitcoin/issues/23119

kevkevinpal commented at 8:42 pm on March 13, 2026: contributor

Concept ACK

The use of scripted diff is good, glad someone is picking up this cleanup change!

in test/functional/mempool_cluster.py:84 in 068333c5c3 outdated

80@@ -81,7 +81,7 @@ def check_feerate_diagram(self, node):
81         last_val = {"weight": 0, "fee": 0}
82         for x in feeratediagram:
83             # The weight is always positive, except for the first iteration
84-            assert x['weight'] > 0 or x['fee'] == 0
85+            assert x['weight'] > 0 or not x['fee']

maflcko commented at 4:17 pm on March 16, 2026:

068333c5c3387cf0809fa779f9fb7ac5fc7d4118: I don’t think this is a good change, nor correct.

fee==0 reads easier than not fee. Also, the replacement is wrong:

0>>> fee=None; fee==0
1False
2>>> fee=None; not fee
3True

l0rinc commented at 4:31 pm on March 16, 2026:

It seems to me fee isn’t optional here: https://github.com/bitcoin/bitcoin/blob/e31ab8040fc85311dae5390f7ad5d9320828f30b/src/rpc/mempool.cpp#L644

But I can improve it regardless if you think that should also be covered, thanks!

maflcko commented at 9:59 am on March 17, 2026:

The goal of the tests is to also catch type errors. I know there are internal type checks, but they are incomplete: E.g. #34799 (et al)

In any case, this reads harder, so I just don’t see the benefit of making something harder to read and more brittle at the same time.

l0rinc commented at 10:23 am on March 18, 2026:

Valid point, thanks, fixed in latest push!

in test/functional/p2p_segwit.py:1361 in 068333c5c3 outdated

1357@@ -1358,7 +1358,7 @@ def test_segwit_versions(self):
1358             temp_utxo.append(UTXO(tx.txid_int, 0, tx.vout[0].nValue))
1359 
1360         self.generate(self.nodes[0], 1)  # Mine all the transactions
1361-        assert len(self.nodes[0].getrawmempool()) == 0
1362+        assert not self.nodes[0].getrawmempool()

maflcko commented at 4:17 pm on March 16, 2026:

Same here

maflcko commented at 4:17 pm on March 16, 2026:

and all other places …

l0rinc commented at 4:33 pm on March 16, 2026:

Thanks, if you think these can be None, I’ll adjust them differently to avoid the comparison that hinders the scripted diff

test: use `in` for two-value equality asserts

Some tests spell a two-value membership check as `assert x == a or x == b`.
Rewrite those sites as `assert x in (a, b)`.
This keeps the check the same and removes `==` forms that the later cleanup should not touch.

5840d4c938

test: split equality asserts joined by `and`

Some tests combine multiple equality checks in one `assert`.
Split those checks into separate assertions so failures point at the exact mismatch.
This also removes mixed `==` expressions that the later cleanup should not touch.

b318d4b09f

test: prep manual equality assert conversions

The later scripted diff only handles plain `assert x == y` lines.
Some remaining tests still use equality inside comprehensions, parenthesized asserts, and other shapes that the line-based rewrite would misread.
Rewrite those sites by hand first so the later mechanical conversion stays safe.
The commit also simplifies the dead `len(["errors"]) == 0` branch in `blocktools.py`, which can never be true.

0bc3496f82

test: convert equality asserts with comments or special chars

Some remaining `assert x == y` lines include inline comments, list literals, or descriptor strings with `#`.
Convert those sites by hand so the later mechanical rewrite can stay simple.

84f86017bf

test: convert simple equality asserts in excluded files

A few files still need to stay out of the later scripted diff because they contain more complicated assert shapes.
Convert the straightforward equality checks in those files by hand first.
Use a local import in `authproxy.py` so the change does not create an import cycle.

98da22c88e

test: add missing `assert_equal` imports

The later scripted diff only rewrites call sites.
Add `assert_equal` imports in the remaining files that still need them so the mechanical replacement can apply cleanly.

58574a53cd

scripted-diff: replace remaining Python test equality asserts

Some Python functional tests still use plain `assert x == y`.
The earlier commits convert the ambiguous assert patterns by hand, so this commit can rewrite the remaining safe cases mechanically.
The verify script excludes `wallet_bumpfee.py`, `test_framework/netutil.py`, and `test_framework/authproxy.py`, which still contain assert forms that the plain line-based substitution would misidentify.

-BEGIN VERIFY SCRIPT-
perl -pi -e 's/^(\s*)assert (.+?) == ([^,#]+?)$/\1assert_equal(\2, \3)/' $(git ls-files -- 'test/functional' \
':(exclude)test/functional/wallet_bumpfee.py' ':(exclude)test/functional/test_framework/netutil.py' ':(exclude)test/functional/test_framework/authproxy.py')
-END VERIFY SCRIPT-

2680ef6503

l0rinc force-pushed on Mar 18, 2026

in test/functional/wallet_miniscript.py:247 in b318d4b09f

242@@ -243,7 +243,8 @@ def watchonly_test(self, desc):
243             lambda: len(self.ms_wo_wallet.listunspent(minconf=0, addresses=[addr])) == 1
244         )
245         utxo = self.ms_wo_wallet.listunspent(minconf=0, addresses=[addr])[0]
246-        assert utxo["txid"] == txid and utxo["solvable"]
247+        assert_equal(utxo["txid"], txid)
248+        assert utxo["solvable"]

maflcko commented at 1:22 pm on March 30, 2026:

nit in b318d4b09f9de1fc4f18a479a2078b818e1bc778: Should use assert_equal (probably with `True)

in test/functional/rpc_psbt.py:966 in b318d4b09f

965         # After signing and finalizing, needs extracting
966         signed = self.nodes[1].walletprocesspsbt(updated)['psbt']
967         analyzed = self.nodes[0].analyzepsbt(signed)
968-        assert analyzed['inputs'][0]['has_utxo'] and analyzed['inputs'][0]['is_final'] and analyzed['next'] == 'extractor'
969+        assert analyzed['inputs'][0]['has_utxo']
970+        assert analyzed['inputs'][0]['is_final']

maflcko commented at 1:22 pm on March 30, 2026:

same

in test/functional/rpc_psbt.py:944 in b318d4b09f

939@@ -937,20 +940,31 @@ def test_psbt_input_keys(psbt_input, keys):
940         self.generate(self.nodes[0], 6)[0]
941         psbt = self.nodes[1].createpsbt([utxo], {self.nodes[0].getnewaddress("", "p2sh-segwit"):Decimal('6.999')})
942         analyzed = self.nodes[0].analyzepsbt(psbt)
943-        assert not analyzed['inputs'][0]['has_utxo'] and not analyzed['inputs'][0]['is_final'] and analyzed['inputs'][0]['next'] == 'updater' and analyzed['next'] == 'updater'
944+        assert not analyzed['inputs'][0]['has_utxo']
945+        assert not analyzed['inputs'][0]['is_final']

maflcko commented at 1:23 pm on March 30, 2026:

same (False)

in test/functional/test_framework/script.py:819 in 0bc3496f82

814@@ -814,8 +815,8 @@ def BIP341_sha_outputs(txTo):
815     return sha256(b"".join(o.serialize() for o in txTo.vout))
816 
817 def TaprootSignatureMsg(txTo, spent_utxos, hash_type, input_index=0, *, scriptpath=False, leaf_script=None, codeseparator_pos=-1, annex=None, leaf_ver=LEAF_VERSION_TAPSCRIPT):
818-    assert (len(txTo.vin) == len(spent_utxos))
819-    assert (input_index < len(txTo.vin))
820+    assert_equal(len(txTo.vin), len(spent_utxos))
821+    assert input_index < len(txTo.vin)

maflcko commented at 1:38 pm on March 30, 2026:

nit in 0bc3496f82267ce1fc0c591088921a640f5ae1c9:

assert input_index < len(txTo.vin) → assert_greater_than(len(txTo.vin), input_index)

maflcko approved

maflcko commented at 1:51 pm on March 30, 2026: member

review ACK 2680ef65035f3da58097f3b34c68bdf7a6d4346b 🍹

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK 2680ef65035f3da58097f3b34c68bdf7a6d4346b 🍹
3ubvetapgdre5kedJ9KqlFlce1nJ+V8M/ll8CJLl3m/PAxKPcYQTfWpBBPk2sFSCD3+yp7om7zCW2709FWZ1SCw==

test: migrate functional test equality asserts to assert_equal #34773

Problem

Design

Fix

Reviews

Conflicts

Fix

test: migrate functional test equality asserts to `assert_equal` #34773