test: create assert_not_equal util by kevkevinpal · Pull Request #29500 · bitcoin/bitcoin

kevkevinpal · 2024-02-28T02:55:22Z

In the functional tests there are lots of cases where we assert != which we now swap with assert_not_equal to be more readable

This is motivated/uses logic from this PR which was closed #28528
This partially helps #23119

I've broken it up to just assert_not_equal to keep the PR smaller as suggested in #28528 (comment)

I can create follow up PR's if this is wanted

DrahtBot · 2024-02-28T02:55:24Z

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/29500.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hodlinator, ryanofsky, janb84
Concept ACK	theStack, rkrux
Approach ACK	BrandonOdiwuor
Stale ACK	vasild

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32155 (miner: timelock the coinbase to the mined block's height by darosior)
#31492 (Execute Discover() when bind=0.0.0.0 or :: is set by andremralves)
#29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)

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.

Empact · 2024-02-28T07:03:54Z

Would be nice to structure this as a scripted diff: one commit where you add the function and requires, another where you convert the assert !=s.

That would ensure programmatically that you hit all of the cases.

kevkevinpal · 2024-03-02T16:15:41Z

added scripted diff in dc087f1 with the following script
git grep -l "assert.*!=" ./test/functional

I will have to amend the commit as I see more spots I need to modify with the assert_not_equal util

theStack · 2024-03-02T20:35:50Z

Concept ACK, thanks for picking this up.

brunoerg · 2024-03-04T14:48:21Z

There are some places you are using assert_not_equal but not importing it and other ones you're importing it twice. Check CI.

Running Unit Tests for Test Framework Modules
Traceback (most recent call last):
  File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_runner.py", line 906, in <module>
    main()
  File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_runner.py", line 539, in main
    run_tests(
  File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_runner.py", line 580, in run_tests
    test_framework_tests.addTest(unittest.TestLoader().loadTestsFromName("test_framework.{}".format(module)))
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/unittest/loader.py", line 137, in loadTestsFromName
    module = __import__(module_name)
             ^^^^^^^^^^^^^^^^^^^^^^^
  File "/ci_container_base/test/functional/test_framework/address.py", line 14, in <module>
    from .script import (
  File "/ci_container_base/test/functional/test_framework/script.py", line 14, in <module>
    from .key import TaggedHash, tweak_add_pubkey, compute_xonly_pubkey
  File "/ci_container_base/test/functional/test_framework/key.py", line 16, in <module>
    from test_framework.crypto import secp256k1
  File "/ci_container_base/test/functional/test_framework/crypto/secp256k1.py", line 316, in <module>
    G = GE.lift_x(0x79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/ci_container_base/test/functional/test_framework/crypto/secp256k1.py", line 257, in lift_x
    y = (FE(x)**3 + 7).sqrt()
         ^^^^^
  File "/ci_container_base/test/functional/test_framework/crypto/secp256k1.py", line 41, in __init__
    assert_not_equal(den, 0)
    ^^^^^^^^^^^^^^^^
NameError: name 'assert_not_equal' is not defined

test/functional/p2p_blockfilters.py

kevkevinpal · 2024-11-13T02:43:25Z

yes @rkrux you are right if I want to use a scripted diff then all the changes must be done using the script

I can remove the scripted diff and just squash them into a single diff since right now the script is getting very large already if that is preferred by others

fanquake · 2025-02-20T17:29:19Z

@kevkevinpal what's the status of this?

vasild · 2025-02-21T13:17:05Z

Almost ACK dcf9a45aab4c4a7f9f900d5dafcdfe0e9a598a38, modulo squash the two commits into one and rebase.

kevkevinpal · 2025-02-22T02:22:35Z

@kevkevinpal what's the status of this?

Just rebased and addressed @vasild comment

Almost ACK dcf9a45, modulo squash the two commits into one and rebase.

Just rebased and squashed the commits, I removed the scripted-diff because it was getting more difficult to review the diff than the actual code. But now all the changes are in d9cb032

if you do grep -nr assert\ .*!= ./test/functional/ there should only be one result

assert len(ss) == 175 - (in_type == SIGHASH_ANYONECANPAY) * 49 - (out_type != SIGHASH_ALL and out_type != SIGHASH_SINGLE) * 32 + (annex is not None) * 32 + scriptpath * 37

which we don't need to add assert_not_equal to

hodlinator

Concept ACK d9cb032ec1aa7000fc3f5b9fa48405f269879497

hodlinator · 2025-02-24T13:43:35Z

test/functional/feature_coinstatsindex.py

nit: Should appear after assert_equal alphabetically.

Same in test/functional/feature_dbcrash.py and others.

hodlinator · 2025-02-24T13:54:53Z

test/functional/test_framework/util.py

Should use f-strings:

bitcoin/test/functional/README.md

Line 40 in e486597

- Use `f'{x}'` for string formatting in preference to `'{}'.format(x)` or `'%s' % x`.

nit: And possibly rephrase:

Suggested change

raise AssertionError("%s == %s %s" % (str(thing1), str(thing2), str(error_message)))

raise AssertionError(f"Both values are {thing1}{f', {error_message}' if error_message else ''}")

Would be more eager to ACK if code introduced by the PR followed bitcoin/test/functional/README.md as noted above. If you don't want to change the phrasing towards what's in the nit-part that's fine.

Went ahead an used your suggestion in be71af3

it now looks like this

2025-03-27T23:14:10.629000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/mnt/shared_drive/DEVDIR/bitcoin/test/functional/test_framework/test_framework.py", line 182, in main self.run_test() File "/mnt/shared_drive/DEVDIR/bitcoin/./build_dir/test/functional/p2p_blockfilters.py", line 98, in run_test assert_not_equal(stale_block_hash, stale_block_hash, "node 0 chain did not reorganize") File "/mnt/shared_drive/DEVDIR/bitcoin/test/functional/test_framework/util.py", line 81, in assert_not_equal raise AssertionError(f"Both values are {thing1}, {f'{error_message}' if error_message else ''}") AssertionError: Both values are 05ba865bb920e2ff5763411a9dd3c225ace8ff4393469bb25acffd56df1bfb6e, node 0 chain did not reorganize

Thread #29500 (comment):
Thanks for taking my suggestion!

hodlinator · 2025-02-24T14:00:54Z

test/functional/p2p_blockfilters.py

nit: Feels a bit weird to not have assert_equal() here and other directly adjacent places. Could touch up adjacent lines like this in separate commit?

hodlinator · 2025-02-24T14:10:08Z

test/functional/feature_dbcrash.py

In thread #29500 (comment):

Agree this is a valid argument against the PR. It is very seldom useful to print the values when they are equal.

I still prefer assert_not_equal() for:

Symmetry with assert_equal().

Distinguishing itself from assert which is skipped when running Python with -O.

vasild

ACK d9cb032ec1aa7000fc3f5b9fa48405f269879497

ryanofsky

Code review ACK d9cb032ec1aa7000fc3f5b9fa48405f269879497. Seems like a nice change, though needs to be rebased. I think this change is good because it makes tests more readable, prints more information on failures, avoids misusing the assert keyword, and makes the test framework api more consistent internally and consistent with other frameworks.

kevkevinpal · 2025-03-27T20:40:33Z

rebased to 29fa2b9

maflcko · 2025-03-28T07:11:19Z

test/functional/test_framework/util.py

what is the point of the error_message, given that it is unused and highly fragile:

It is not a named arg, nor type-safe, so someone writing assert_not_equal("a", "b", "c") or assert_not_equal(1, 2, 3) will be wrong and confusing at best.

It is not used anywhere else in this file for another assert

Thread #29500 (comment):

It would be good if all assert_ functions supported error messages explaining why the condition should never fail, or what could be wrong if it has failed. (Built-in assert supports it too).

To avoid mixing the error message up with a value to be checked, one could do:

Suggested change

def assert_not_equal(thing1, thing2, error_message=""):

if thing1 == thing2:

raise AssertionError(f"Both values are {thing1}, {f'{error_message}' if error_message else ''}")

def assert_not_equal(thing1, thing2, *, message=None):

if thing1 == thing2:

raise AssertionError(f"Both values are {thing1}{f', {message}' if message else ''}")

* enforces named parameters from that point.

It would be good to shorten the name for callers.

", " is only printed if there is a message.

Feels more correct to use None as default value.

The traceback in this case does print the source code line, so a comment on the same line could be sufficient. It depends on how the error is presented. Example failure:

2025-03-28T09:58:37.993000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/home/hodlinator/bitcoin/test/functional/test_framework/test_framework.py", line 182, in main self.run_test() File "/home/hodlinator/bitcoin/build/test/functional/p2p_blockfilters.py", line 98, in run_test assert_not_equal(main_block_hash, stale_block_hash, message="node 0 chain did not reorganize") File "/home/hodlinator/bitcoin/test/functional/test_framework/util.py", line 81, in assert_not_equal raise AssertionError(f"Both values are {thing1}{f', {message}' if message else ''}") AssertionError: Both values are 1b0ec65a9941d57790524ff90cc1cb93d2f8e70680c1c7486d9edf934d96a041, node 0 chain did not reorganize

assert_not_equal(main_block_hash, stale_block_hash, message="node 0 chain did not reorganize")

I'd say it is already self-explanatory to see assert_not_equal(main_block_hash, stale_block_hash) and the message "AssertionError: Both values are (the same)". It should be clear that the chains are the same, when they should be different.

Even more so, given that the test logs before the assert: self.log.info("Reorg node 0 to a new chain.").

Unique and descriptive error messages make sense when it is the only piece of information given (for example in the context of an init error during startup of the program). Given that no other asserts in this file give the option, it would be best to defer this to a future where it is actually widely needed. However, given the extensive context in tests and test failures, I doubt it will ever be widely needed. Until then, it is just bike-shedding and overhead that has to be maintained and reviewed.

"AssertionError: Both values are (the same)"

Don't know if you mean literally "Both values are (the same)" or f"Both values are {thing1}". Find the latter clearly more useful; providing clues of cause of the failure, making it easier to reproduce.

What it comes down to with custom error messages is being able to directly communicate with whoever is troubleshooting a failure. If one doesn't even need to go to the logs to see what happened before, I consider it a small win. But agree it is bikeshedding-prone, so I would be okay with removing it.

Don't know if you mean literally "Both values are (the same)" or f"Both values are {thing1}". Find the latter clearly more useful; providing clues of cause of the failure, making it easier to reproduce.

Agree. Sorry for being unclear, I meant f"Both values are {thing1}" when I said "(the same)".

re: #29500 (comment)

I do like error_message parameter idea, fwiw. I could see these making tests more readable if they started being used for more important or more confusing checks. Tests are often written with some important checks directly related to the thing being tested, and other less important checks to see if other things are consistent. Having a place to indicate what it actually means when a check fails could be helpful for more important checks to distinguish them.

The traceback in this case does print the source code line, so a comment on the same line could be sufficient. It depends on how the error is presented. Example failure:

This is an annoying draw back that the stack trace won't contain the line in the test that's in error, but instead the line in the assert_not_equal helper. In Rust, one can decorate the helper function with the track_caller macro. I don't see anything that is similar for C++ sadly.

We're in Python here though. I spent a couple of hours with an LLM in mid-February to brute force this embryo:
master...hodlinator:bitcoin:2025/02/assert_ergonomics

Output examples

Current assert_equal()-behavior:

2025-03-28T21:15:36.379000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/home/hodlinator/bitcoin/test/functional/test_framework/test_framework.py", line 178, in main self.run_test() File "/home/hodlinator/bitcoin/./build/test/functional/wallet_multisig_descriptor_psbt.py", line 80, in run_test assert_equal( File "/home/hodlinator/bitcoin/test/functional/test_framework/util.py", line 77, in assert_equal raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args)) AssertionError: not(0 == 123)

With first commit, chomping off util.py traceback entry you complained about:

2025-03-28T21:16:19.814000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/home/hodlinator/bitcoin/test/functional/test_framework/test_framework.py", line 178, in main self.run_test() File "/home/hodlinator/bitcoin/./build/test/functional/wallet_multisig_descriptor_psbt.py", line 80, in run_test assert_equal( AssertionError: not(0 == 123)

With second commit, replacing another traceback entry with proper context:

2025-03-28T21:17:39.494000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/home/hodlinator/bitcoin/test/functional/test_framework/test_framework.py", line 178, in main self.run_test() AssertionError: not(0 == 123) Failing expression: /home/hodlinator/bitcoin/./build/test/functional/wallet_multisig_descriptor_psbt.py:80 - 83 assert_equal( 123 if (False and True) else 0, 123)

The code needs more work, should be more defensive, forgot to output function name (run_test). And I wouldn't bet on it being accepted anyway.

No strong opinion, but I think we shouldn't spend too much time on the error message explaining the context because the traceback could contain too little context, or on the traceback containing too much (redundant) context by default. A full log of a test failure could be thousands of lines, so one additional line, even if could be redundant in a strict sense should be fine. Also, looking at the historic failures (in Python) at https://github.com/bitcoin/bitcoin/issues?q=is%3Aissue%20%20label%3A%22CI%20failed%22%20, it was never(?) an issue to figure out what failed where. The problem usually is to figure out why something failed, and how to reproduce and fix it.

I don't mind having an error message being logged and I do agree with the following points:

It is not a named arg, nor type-safe, so someone writing assert_not_equal("a", "b", "c") or assert_not_equal(1, 2, 3) will be wrong and confusing at best.

Unique and descriptive error messages make sense when it is the only piece of information given (for example in the context of an init error during startup of the program).

As I see from the usages of assert_not_equal, there is only 1 case where the error message is passed out of ~77 total usages. I believe a named argument for the error message could help in avoiding unintended usages of this utility function.

hodlinator

ACK be71af3cc0b0bcb7d917cc6f2e5fda119f1b1bd6

This change adds the missing opposite of assert_equal() and decreases usage of built-in assert for performing always-on checks during tests.

hodlinator · 2025-03-28T09:42:08Z

test/functional/test_framework/util.py

Thread #29500 (comment):

It would be good if all assert_ functions supported error messages explaining why the condition should never fail, or what could be wrong if it has failed. (Built-in assert supports it too).

To avoid mixing the error message up with a value to be checked, one could do:

Suggested change

def assert_not_equal(thing1, thing2, error_message=""):

if thing1 == thing2:

raise AssertionError(f"Both values are {thing1}, {f'{error_message}' if error_message else ''}")

def assert_not_equal(thing1, thing2, *, message=None):

if thing1 == thing2:

raise AssertionError(f"Both values are {thing1}{f', {message}' if message else ''}")

* enforces named parameters from that point.

It would be good to shorten the name for callers.

", " is only printed if there is a message.

Feels more correct to use None as default value.

The traceback in this case does print the source code line, so a comment on the same line could be sufficient. It depends on how the error is presented. Example failure:

2025-03-28T09:58:37.993000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/home/hodlinator/bitcoin/test/functional/test_framework/test_framework.py", line 182, in main self.run_test() File "/home/hodlinator/bitcoin/build/test/functional/p2p_blockfilters.py", line 98, in run_test assert_not_equal(main_block_hash, stale_block_hash, message="node 0 chain did not reorganize") File "/home/hodlinator/bitcoin/test/functional/test_framework/util.py", line 81, in assert_not_equal raise AssertionError(f"Both values are {thing1}{f', {message}' if message else ''}") AssertionError: Both values are 1b0ec65a9941d57790524ff90cc1cb93d2f8e70680c1c7486d9edf934d96a041, node 0 chain did not reorganize

hodlinator · 2025-03-28T09:43:28Z

test/functional/test_framework/util.py

Thread #29500 (comment):
Thanks for taking my suggestion!

janb84

Concept ACK be71af3

Code review ✅
Build & tested ✅

I agree with @hodlinator on the import order NIT — let's keep it consistent and maintain clean, high-quality code. #29500 (comment)

ryanofsky

Code review ACK be71af3cc0b0bcb7d917cc6f2e5fda119f1b1bd6. Just rebased and added f-string since last review.

ryanofsky · 2025-03-28T15:24:03Z

test/functional/test_framework/util.py

re: #29500 (comment)

I do like error_message parameter idea, fwiw. I could see these making tests more readable if they started being used for more important or more confusing checks. Tests are often written with some important checks directly related to the thing being tested, and other less important checks to see if other things are consistent. Having a place to indicate what it actually means when a check fails could be helpful for more important checks to distinguish them.

rkrux

Concept ACK be71af3cc0b0bcb7d917cc6f2e5fda119f1b1bd6

Requesting changes mainly because an additional comma is printed in case of an assertion error when the error message is not passed, which can be confusing for the reader later.

2025-03-31T07:28:44.958000Z TestFramework (INFO): Test fundrawtxn with locked wallet and hardened derivation
2025-03-31T07:28:46.455000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/test_framework.py", line 182, in main
    self.run_test()
    ~~~~~~~~~~~~~^^
  File "/Users/rkrux/projects/bitcoin/./build/test/functional/wallet_fundrawtransaction.py", line 141, in run_test
    self.test_locked_wallet()
    ~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/Users/rkrux/projects/bitcoin/./build/test/functional/wallet_fundrawtransaction.py", line 665, in test_locked_wallet
    assert_not_equal(fundedTx["changepos"], 1)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/util.py", line 81, in assert_not_equal
    raise AssertionError(f"Both values are {thing1}, {f'{error_message}' if error_message else ''}")
AssertionError: Both values are 1, 
2025-03-31T07:28:46.512000Z TestFramework (INFO): Not stopping nodes as test failed. The dangling processes will be cleaned up later.

rkrux · 2025-03-31T07:56:53Z

test/functional/test_framework/util.py

I don't mind having an error message being logged and I do agree with the following points:

It is not a named arg, nor type-safe, so someone writing assert_not_equal("a", "b", "c") or assert_not_equal(1, 2, 3) will be wrong and confusing at best.

Unique and descriptive error messages make sense when it is the only piece of information given (for example in the context of an init error during startup of the program).

As I see from the usages of assert_not_equal, there is only 1 case where the error message is passed out of ~77 total usages. I believe a named argument for the error message could help in avoiding unintended usages of this utility function.

kevkevinpal · 2025-03-31T21:57:10Z

Updated to f3d4b6d

Added named variable for error_message and also made the assert able to take more than two variables to compare to each other. The error will now say
Two or more values equal to <value>: <error_message> or
Two or more values equal to <value>

if no error_message provided

hodlinator · 2025-04-01T08:22:59Z

test/functional/test_framework/util.py

Tested with:

assert_not_equal(stale_block_hash, stale_block_hash, "node 0 chain did not reorganize")

Output was:

2025-04-01T08:15:47.847000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/home/hodlinator/b2/test/functional/test_framework/test_framework.py", line 182, in main self.run_test() File "/home/hodlinator/b2/build/test/functional/p2p_blockfilters.py", line 66, in run_test assert_not_equal(stale_block_hash, stale_block_hash, "node 0 chain did not reorganize") File "/home/hodlinator/b2/test/functional/test_framework/util.py", line 85, in assert_not_equal raise AssertionError( AssertionError: Two or more values equal to 775ea4a5e5005376a49f2da6d20ce0449fbd627d766c8fc3108d22db0d41e116

This indicates that the message wasn't recognized as such. Naming the parameter has the desired effect:

assert_not_equal(stale_block_hash, stale_block_hash, error_message="node 0 chain did not reorganize")

A possible step forward would be to update callers to name the parameter. As I said before, I think the name should be shortened.

(Phantom space)

Suggested change

def assert_not_equal(*things, error_message=""):

def assert_not_equal(*things, error_message=""):

resolved in 7bb83f6

I went back to using two params and made error_message a named parameter that cannot be used as a positional argument

maflcko · 2025-04-01T06:34:47Z

test/functional/test_framework/util.py

This is still wrong (the error message is not a named arg). Also, adding support for more values is unused and dead code.

My recommendation would be to just use assert_not_equal(thing1, thing2):. Any other attempt pushed to this pull so far was wrong or brittle.

fair point and thanks for the review

I've updated to 7bb83f6

we are now using the error message (f"Both values are {thing1}{f', {error_message}' if error_message else ''}"

we only take two positional arguments and a named error_message arg which is used in p2p_blockfilters

In the functional tests there are lots of cases where we assert != which this new util will replace, we also are adding the imports and the new assertion

hodlinator

re-ACK 7bb83f6

Remaining reservations from #29500 (comment) (sorry for broken record):

Current push still uses falsy error_message="" with the if error_message, instead of clear error_message=None
error_message= at call sites is quite verbose.

ryanofsky

Code review ACK 7bb83f6. Only change since last review is fixing error message formatting and passing it as a keyword argument

janb84

Re-ACK 7bb83f6

rkrux

Post-merge ACK 7bb83f6

rkrux · 2025-04-02T08:11:56Z

test/functional/test_framework/test_node.py

-                assert ret != 0  # Exit code must indicate failure
+                assert_not_equal(ret, 0) # Exit code must indicate failure


The error message arg can/could have been used here instead of the comment.

DrahtBot added the Tests label Feb 28, 2024

This comment was marked as spam.

Sign in to view

maflcko requested a review from theStack February 28, 2024 08:43

This was referenced Feb 29, 2024

test/BIP324: disconnection scenarios during v2 handshake #29431

Merged

Remove the legacy wallet and BDB dependency #28710

Merged

kevkevinpal force-pushed the testFrameworkUtilsOne branch 2 times, most recently from 32710c6 to 4cf2f1e Compare March 2, 2024 16:12

DrahtBot added the CI failed label Mar 2, 2024

kevkevinpal force-pushed the testFrameworkUtilsOne branch from 4cf2f1e to 005a8e9 Compare March 2, 2024 16:13

kevkevinpal force-pushed the testFrameworkUtilsOne branch 6 times, most recently from 57ecac9 to 6ee4175 Compare March 2, 2024 20:35

kevkevinpal force-pushed the testFrameworkUtilsOne branch 5 times, most recently from 8d6da40 to ad1e879 Compare March 2, 2024 21:47

DrahtBot mentioned this pull request Mar 3, 2024

policy: enable sibling eviction for v3 transactions #29306

Merged

brunoerg reviewed Mar 4, 2024

View reviewed changes

test/functional/p2p_blockfilters.py Outdated Show resolved Hide resolved

kevkevinpal force-pushed the testFrameworkUtilsOne branch 3 times, most recently from 1c0929e to a6a6210 Compare March 5, 2024 04:21

kevkevinpal force-pushed the testFrameworkUtilsOne branch from 6b3260e to dcf9a45 Compare November 13, 2024 02:39

DrahtBot added Needs rebase and removed CI failed labels Nov 13, 2024

l0rinc mentioned this pull request Feb 17, 2025

qa wallet: Activate dormant checks in wallet_multisig_descriptor_psbt.py #31874

Merged

fanquake marked this pull request as draft February 20, 2025 17:28

DrahtBot mentioned this pull request Feb 22, 2025

wallet: fix crash on double block disconnection #31757

Merged

hodlinator reviewed Feb 24, 2025

View reviewed changes

DrahtBot mentioned this pull request Feb 26, 2025

Execute Discover() when bind=0.0.0.0 or :: is set #31492

Closed

DrahtBot mentioned this pull request Mar 13, 2025

test: avoid treating hash results as integers #32050

Merged

vasild approved these changes Mar 19, 2025

View reviewed changes

ryanofsky approved these changes Mar 27, 2025

View reviewed changes

maflcko reviewed Mar 28, 2025

View reviewed changes

DrahtBot mentioned this pull request Mar 28, 2025

miner: timelock the coinbase to the mined block's height #32155

Merged

hodlinator approved these changes Mar 28, 2025

View reviewed changes

janb84 reviewed Mar 28, 2025

View reviewed changes

ryanofsky approved these changes Mar 28, 2025

View reviewed changes

rkrux suggested changes Mar 31, 2025

View reviewed changes

hodlinator reviewed Apr 1, 2025

View reviewed changes

maflcko reviewed Apr 1, 2025

View reviewed changes

test: create assert_not_equal util and add to where imports are needed

7bb83f6

In the functional tests there are lots of cases where we assert != which this new util will replace, we also are adding the imports and the new assertion

hodlinator approved these changes Apr 1, 2025

View reviewed changes

ryanofsky approved these changes Apr 1, 2025

View reviewed changes

janb84 reviewed Apr 1, 2025

View reviewed changes

rkrux reviewed Apr 2, 2025

View reviewed changes

	raise AssertionError("%s == %s %s" % (str(thing1), str(thing2), str(error_message)))
	raise AssertionError(f"Both values are {thing1}{f', {error_message}' if error_message else ''}")

-def assert_not_equal(thing1, thing2, error_message=""):
-    if thing1 == thing2:
-        raise AssertionError(f"Both values are {thing1}, {f'{error_message}' if error_message else ''}")
+def assert_not_equal(thing1, thing2, *, message=None):
+    if thing1 == thing2:
+        raise AssertionError(f"Both values are {thing1}{f', {message}' if message else ''}")

	def assert_not_equal(*things, error_message=""):
	def assert_not_equal(*things, error_message=""):

		assert ret != 0 # Exit code must indicate failure
		assert_not_equal(ret, 0) # Exit code must indicate failure

Conversation

kevkevinpal commented Feb 28, 2024

Uh oh!

DrahtBot commented Feb 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

This comment was marked as spam.

Uh oh!

Empact commented Feb 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevkevinpal commented Mar 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theStack commented Mar 2, 2024

Uh oh!

brunoerg commented Mar 4, 2024

Uh oh!

Uh oh!

kevkevinpal commented Nov 13, 2024

Uh oh!

fanquake commented Feb 20, 2025

Uh oh!

vasild commented Feb 21, 2025

Uh oh!

kevkevinpal commented Feb 22, 2025

Uh oh!

hodlinator left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hodlinator Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

kevkevinpal commented Mar 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hodlinator Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hodlinator left a comment

DrahtBot commented Feb 28, 2024 •

edited

Loading

Empact commented Feb 28, 2024 •

edited

Loading

kevkevinpal commented Mar 2, 2024 •

edited

Loading

hodlinator Feb 24, 2025 •

edited

Loading

hodlinator Mar 28, 2025 •

edited

Loading

janb84 left a comment •

edited

Loading

ryanofsky left a comment •

edited

Loading

maflcko Apr 1, 2025 •

edited

Loading