Merge bitcoin/bitcoin#33001: test: Do not pass tests on unhandled exceptions

fanquake · Tom Trevethan · commit b7ee5fb8a536 · 2025-11-25T16:25:39.000Z
faa3e684118bffa7a98cf76eeeb59243219df900 test: Log KeyboardInterrupt as exception (MarcoFalke) fa30b34026f76a5b8af997152fced2d281782e0d test: Do not pass tests on unhandled exceptions (MarcoFalke) Pull request description: Currently the functional tests are problematic, because they pass, even if they encounter an unhanded exception. Fix this by handling all exceptions: Catch `BaseException` as fallback and mark it as failure. Can be tested via: ```diff diff --git a/test/functional/wallet_disable.py b/test/functional/wallet_disable.py index da6e5d4..ecc41fb041 100755 --- a/test/functional/wallet_disable.py +++ b/test/functional/wallet_disable.py @@ -19,6 +19,7 @@ class DisableWalletTest (BitcoinTestFramework): self.wallet_names = [] def run_test (self): + import sys;sys.exit("fatal error") # Make sure wallet is really disabled assert_raises_rpc_error(-32601, 'Method not found', self.nodes[0].getwalletinfo) x = self.nodes[0].validateaddress('3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy') ``` Previously, the test would pass. With this patch, it would fail. ACKs for top commit: enirox001: Looks good to me—ACK faa3e68 stickies-v: re-ACK faa3e684118bffa7a98cf76eeeb59243219df900 pablomartin4btc: tACK faa3e684118bffa7a98cf76eeeb59243219df900 Tree-SHA512: 11ecd5201982e2c776e48d98834b17c15a415306a95524bc702daeba20a316aac797748e9592be8db575597804f149ee7ef104416037cc9e5891758625810e2d
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -3817,7 +3817,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                 // should not call LookupBlockIndex below.
                 RemoveBlockRequest(resp.blockhash, pfrom.GetId());
                 Misbehaving(pfrom.GetId(), 100, "previous compact block reconstruction attempt failed");
-                LogPrint(BCLog::NET, "Peer %d sent compact block transactions multiple times", pfrom.GetId());
+                LogPrint(BCLog::NET, "Peer %d sent compact block transactions multiple times\n", pfrom.GetId());
                 return;
             }
 
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
@@ -130,26 +130,14 @@ def main(self):
         try:
             self.setup()
             self.run_test()
-        except JSONRPCException:
-            self.log.exception("JSONRPC error")
-            self.success = TestStatus.FAILED
         except SkipTest as e:
             self.log.warning("Test Skipped: %s" % e.message)
             self.success = TestStatus.SKIPPED
-        except AssertionError:
-            self.log.exception("Assertion failed")
-            self.success = TestStatus.FAILED
-        except KeyError:
-            self.log.exception("Key error")
-            self.success = TestStatus.FAILED
         except subprocess.CalledProcessError as e:
-            self.log.exception("Called Process failed with '{}'".format(e.output))
-            self.success = TestStatus.FAILED
-        except Exception:
-            self.log.exception("Unexpected exception caught during testing")
+            self.log.exception(f"Called Process failed with stdout='{e.stdout}'; stderr='{e.stderr}';")
             self.success = TestStatus.FAILED
-        except KeyboardInterrupt:
-            self.log.warning("Exiting after keyboard interrupt")
+        except BaseException:
+            self.log.exception("Unexpected exception")
             self.success = TestStatus.FAILED
         finally:
             exit_code = self.shutdown()

Original file line number	Diff line number	Diff line change
`@@ -3817,7 +3817,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,`
`3817`	`3817`	`// should not call LookupBlockIndex below.`
`3818`	`3818`	`RemoveBlockRequest(resp.blockhash, pfrom.GetId());`
`3819`	`3819`	`Misbehaving(pfrom.GetId(), 100, "previous compact block reconstruction attempt failed");`
`3820`		`- LogPrint(BCLog::NET, "Peer %d sent compact block transactions multiple times", pfrom.GetId());`
	`3820`	`+ LogPrint(BCLog::NET, "Peer %d sent compact block transactions multiple times\n", pfrom.GetId());`
`3821`	`3821`	`return;`
`3822`	`3822`	`}`
`3823`	`3823`