Merge #565: tests: Put device specific parameters in Emulator classes

2c59ad3 tests: Explicitly specify multisig types (Andrew Chow)
44314c2 tests: Add strict_bip48 (Andrew Chow)
8299b08 test: Refactor _make_multisigs to make just one multisig (Andrew Chow)
2166793 tests: Use random port for bitcoind (Andrew Chow)
1226752 tests: use Bitcoind class to manage bitcoind (Andrew Chow)
57c2c20 tests: Add -Wno-array-parameter to BitBox01 build (Andrew Chow)
63a9334 tests: Reduce Jade simulator output (Andrew Chow)
9e1ee3f test: remove full_type (Andrew Chow)
0ec28f4 tests: Move device params into emulator object (Andrew Chow)
12d7b59 tests: Make signtx_cases a required argument for TestSignTx only (Andrew Chow)
9926382 tests: Pass in detection type explicitly to TestDeviceConnect (Andrew Chow)
ed16b8e tests: Allow DeviceTestcase.parameterize to take arbitrary parameters (Andrew Chow)
c86222f tests: Always run superclass start and stop functions (Andrew Chow)
530ed9f tests, ledger: use emulator per-test (Andrew Chow)
5c2e843 tests: Move device specific tests out of ledger_test_suite (Andrew Chow)
649bd9a test, jade: Use emulator per-test (Andrew Chow)
f719fd0 tests: Move device specific tests out of jade_test_suite (Andrew Chow)
c6b767a tests: Use per-test BitBox01Emulator (Andrew Chow)
019efbb tests: Use per-test ColdcardSimulator (Andrew Chow)

Pull request description:

  Currently we have a bunch of device specific parameters passed into the tests when they are created. However the parameters are almost always the same across test cases, so passing them in individually results in long mostly duplicated lines. Since most tests have a `Emulator` class which manages the emulator, we can instead pack those parameters into the `Emulator` object. This makes the code easier to read, and it allows us to also adjust those parameters at runtime when there are different modes for the emulators. This is particularly important as we implement Taproot as the different combinations of things to test gets much more complicated than can be handled with lookups to global dictionaries. Instead of querying a dictionary and doing comparisons of device type strings, we can directly query the emulator.

  There were a few test parameters which are test specific. The way that test parameters are passed in have been updated to allow for test specific parameters instead of having those parameters be available to all tests.

  The Coldcard, Jade, and Ledger have also been updated to use a per-test emulator.

Top commit has no ACKs.

Tree-SHA512: 955692d1662496063bbca7093ec28e8f551495618a75c319e903bc78a2ea3041e78901ed526d7c7cdace77d2df7e601a7adbda4423b4e6b74b28217ca66fac5a

test/run_tests.py

@@ -9,7 +9,7 @@
 from test_bip32 import TestBIP32
 from test_coldcard import coldcard_test_suite
 from test_descriptor import TestDescriptor
-from test_device import start_bitcoind
+from test_device import Bitcoind
 from test_psbt import TestPSBT
 from test_trezor import trezor_test_suite
 from test_ledger import ledger_test_suite
@@ -99,21 +99,21 @@
 
 if args.trezor_1 or args.trezor_t or args.coldcard or args.ledger or args.keepkey or args.bitbox01 or args.jade:
     # Start bitcoind
-    rpc, userpass = start_bitcoind(args.bitcoind)
+    bitcoind = Bitcoind.create(args.bitcoind)
 
     if success and args.bitbox01:
-        success &= digitalbitbox_test_suite(args.bitbox01_path, rpc, userpass, args.interface)
+        success &= digitalbitbox_test_suite(args.bitbox01_path, bitcoind, args.interface)
     if success and args.coldcard:
-        success &= coldcard_test_suite(args.coldcard_path, rpc, userpass, args.interface)
+        success &= coldcard_test_suite(args.coldcard_path, bitcoind, args.interface)
     if success and args.trezor_1:
-        success &= trezor_test_suite(args.trezor_1_path, rpc, userpass, args.interface, '1')
+        success &= trezor_test_suite(args.trezor_1_path, bitcoind, args.interface, '1')
     if success and args.trezor_t:
-        success &= trezor_test_suite(args.trezor_t_path, rpc, userpass, args.interface, 't')
+        success &= trezor_test_suite(args.trezor_t_path, bitcoind, args.interface, 't')
     if success and args.keepkey:
-        success &= keepkey_test_suite(args.keepkey_path, rpc, userpass, args.interface)
+        success &= keepkey_test_suite(args.keepkey_path, bitcoind, args.interface)
     if success and args.ledger:
-        success &= ledger_test_suite(args.ledger_path, rpc, userpass, args.interface)
+        success &= ledger_test_suite(args.ledger_path, bitcoind, args.interface)
     if success and args.jade:
-        success &= jade_test_suite(args.jade_path, rpc, userpass, args.interface)
+        success &= jade_test_suite(args.jade_path, bitcoind, args.interface)
 
 sys.exit(not success)

test/setup_environment.sh

@@ -181,7 +181,7 @@ if [[ -n ${build_bitbox01} ]]; then
 
     # Build the simulator. This is cached, but it is also fast
     mkdir -p build && cd build
-    cmake .. -DBUILD_TYPE=simulator -DCMAKE_C_FLAGS="-Wno-format-truncation"
+    cmake .. -DBUILD_TYPE=simulator -DCMAKE_C_FLAGS="-Wno-format-truncation -Wno-array-parameter"
     make
     cd ../..
 fi
@@ -304,7 +304,6 @@ if [[ -n ${build_jade} ]]; then
         ./configure \
             --target-list=xtensa-softmmu \
             --enable-gcrypt \
-            --enable-sanitizers \
             --disable-user \
             --disable-opengl \
             --disable-curses \

test/test_coldcard.py

@@ -11,118 +11,151 @@
 import unittest
 
 from hwilib._cli import process_commands
-from test_device import DeviceTestCase, start_bitcoind, TestDeviceConnect, TestDisplayAddress, TestGetKeypool, TestGetDescriptors, TestSignMessage, TestSignTx
-
-def coldcard_test_suite(simulator, rpc, userpass, interface):
-    try:
-        os.unlink('coldcard-emulator.stdout')
-    except FileNotFoundError:
-        pass
-    coldcard_log = open('coldcard-emulator.stdout', 'a')
-    # Start the Coldcard simulator
-    coldcard_proc = subprocess.Popen(['python3', os.path.basename(simulator), '--ms'], cwd=os.path.dirname(simulator), stdout=coldcard_log, preexec_fn=os.setsid)
-    # Wait for simulator to be up
-    while True:
+from test_device import (
+    Bitcoind,
+    DeviceEmulator,
+    DeviceTestCase,
+    TestDeviceConnect,
+    TestDisplayAddress,
+    TestGetKeypool,
+    TestGetDescriptors,
+    TestSignMessage,
+    TestSignTx,
+)
+
+class ColdcardSimulator(DeviceEmulator):
+    def __init__(self, simulator):
         try:
-            enum_res = process_commands(['enumerate'])
-            found = False
-            for dev in enum_res:
-                if dev['type'] == 'coldcard' and 'error' not in dev:
-                    found = True
-                    break
-            if found:
-                break
-        except Exception:
+            os.unlink("coldcard-emulator.stdout")
+        except FileNotFoundError:
             pass
-        time.sleep(0.5)
-    # Cleanup
-
-    def cleanup_simulator():
-        if coldcard_proc.poll() is None:
-            os.killpg(os.getpgid(coldcard_proc.pid), signal.SIGTERM)
-            os.waitpid(os.getpgid(coldcard_proc.pid), 0)
-        coldcard_log.close()
-    atexit.register(cleanup_simulator)
-
-    # Coldcard specific management command tests
-    class TestColdcardManCommands(DeviceTestCase):
-        def test_setup(self):
-            result = self.do_command(self.dev_args + ['-i', 'setup'])
-            self.assertIn('error', result)
-            self.assertIn('code', result)
-            self.assertEqual(result['error'], 'The Coldcard does not support software setup')
-            self.assertEqual(result['code'], -9)
-
-        def test_wipe(self):
-            result = self.do_command(self.dev_args + ['wipe'])
-            self.assertIn('error', result)
-            self.assertIn('code', result)
-            self.assertEqual(result['error'], 'The Coldcard does not support wiping via software')
-            self.assertEqual(result['code'], -9)
-
-        def test_restore(self):
-            result = self.do_command(self.dev_args + ['-i', 'restore'])
-            self.assertIn('error', result)
-            self.assertIn('code', result)
-            self.assertEqual(result['error'], 'The Coldcard does not support restoring via software')
-            self.assertEqual(result['code'], -9)
-
-        def test_backup(self):
-            result = self.do_command(self.dev_args + ['backup'])
-            self.assertTrue(result['success'])
-            for filename in glob.glob("backup-*.7z"):
-                os.remove(filename)
-
-        def test_pin(self):
-            result = self.do_command(self.dev_args + ['promptpin'])
-            self.assertIn('error', result)
-            self.assertIn('code', result)
-            self.assertEqual(result['error'], 'The Coldcard does not need a PIN sent from the host')
-            self.assertEqual(result['code'], -9)
-
-            result = self.do_command(self.dev_args + ['sendpin', '1234'])
-            self.assertIn('error', result)
-            self.assertIn('code', result)
-            self.assertEqual(result['error'], 'The Coldcard does not need a PIN sent from the host')
-            self.assertEqual(result['code'], -9)
-
-    class TestColdcardGetXpub(DeviceTestCase):
-        def test_getxpub(self):
-            result = self.do_command(self.dev_args + ['--expert', 'getxpub', 'm/44h/0h/0h/3'])
-            self.assertEqual(result['xpub'], 'tpubDFHiBJDeNvqPWNJbzzxqDVXmJZoNn2GEtoVcFhMjXipQiorGUmps3e5ieDGbRrBPTFTh9TXEKJCwbAGW9uZnfrVPbMxxbFohuFzfT6VThty')
-            self.assertTrue(result['testnet'])
-            self.assertFalse(result['private'])
-            self.assertEqual(result['depth'], 4)
-            self.assertEqual(result['parent_fingerprint'], 'bc123c3e')
-            self.assertEqual(result['child_num'], 3)
-            self.assertEqual(result['chaincode'], '806b26507824f73bc331494afe122f428ef30dde80b2c1ce025d2d03aff411e7')
-            self.assertEqual(result['pubkey'], '0368000bdff5e0b71421c37b8514de8acd4d98ba9908d183d9da56d02ca4fcfd08')
-
-    dev_type = "coldcard"
-    sim_path = "/tmp/ckcc-simulator.sock"
-    fpr = "0f056943"
-    xpub = "tpubDDpWvmUrPZrhSPmUzCMBHffvC3HyMAPnWDSAQNBTnj1iZeJa7BZQEttFiP4DS4GCcXQHezdXhn86Hj6LHX5EDstXPWrMaSneRWM8yUf6NFd"
+        self.simulator = simulator
+        self.coldcard_log = None
+        self.coldcard_proc = None
+        self.type = "coldcard"
+        self.path = "/tmp/ckcc-simulator.sock"
+        self.fingerprint = "0f056943"
+        self.master_xpub = "tpubDDpWvmUrPZrhSPmUzCMBHffvC3HyMAPnWDSAQNBTnj1iZeJa7BZQEttFiP4DS4GCcXQHezdXhn86Hj6LHX5EDstXPWrMaSneRWM8yUf6NFd"
+        self.password = ""
+        self.supports_ms_display = True
+        self.supports_xpub_ms_display = False
+        self.supports_unsorted_ms = False
+        self.supports_taproot = False
+        self.strict_bip48 = False
+
+    def start(self):
+        super().start()
+        self.coldcard_log = open("coldcard-emulator.stdout", "a")
+        # Start the Coldcard simulator
+        self.coldcard_proc = subprocess.Popen(
+            [
+                "python3",
+                os.path.basename(self.simulator), "--ms"
+            ],
+            cwd=os.path.dirname(self.simulator),
+            stdout=self.coldcard_log,
+            preexec_fn=os.setsid
+        )
+        # Wait for simulator to be up
+        while True:
+            try:
+                enum_res = process_commands(["enumerate"])
+                found = False
+                for dev in enum_res:
+                    if dev["type"] == "coldcard" and "error" not in dev:
+                        found = True
+                        break
+                if found:
+                    break
+            except Exception:
+                pass
+            time.sleep(0.5)
+        atexit.register(self.stop)
+
+    def stop(self):
+        super().stop()
+        if self.coldcard_proc.poll() is None:
+            os.killpg(os.getpgid(self.coldcard_proc.pid), signal.SIGTERM)
+            os.waitpid(os.getpgid(self.coldcard_proc.pid), 0)
+        self.coldcard_log.close()
+        atexit.unregister(self.stop)
+
+# Coldcard specific management command tests
+class TestColdcardManCommands(DeviceTestCase):
+    def test_setup(self):
+        result = self.do_command(self.dev_args + ['-i', 'setup'])
+        self.assertIn('error', result)
+        self.assertIn('code', result)
+        self.assertEqual(result['error'], 'The Coldcard does not support software setup')
+        self.assertEqual(result['code'], -9)
+
+    def test_wipe(self):
+        result = self.do_command(self.dev_args + ['wipe'])
+        self.assertIn('error', result)
+        self.assertIn('code', result)
+        self.assertEqual(result['error'], 'The Coldcard does not support wiping via software')
+        self.assertEqual(result['code'], -9)
+
+    def test_restore(self):
+        result = self.do_command(self.dev_args + ['-i', 'restore'])
+        self.assertIn('error', result)
+        self.assertIn('code', result)
+        self.assertEqual(result['error'], 'The Coldcard does not support restoring via software')
+        self.assertEqual(result['code'], -9)
+
+    def test_backup(self):
+        result = self.do_command(self.dev_args + ['backup'])
+        self.assertTrue(result['success'])
+        for filename in glob.glob("backup-*.7z"):
+            os.remove(filename)
+
+    def test_pin(self):
+        result = self.do_command(self.dev_args + ['promptpin'])
+        self.assertIn('error', result)
+        self.assertIn('code', result)
+        self.assertEqual(result['error'], 'The Coldcard does not need a PIN sent from the host')
+        self.assertEqual(result['code'], -9)
+
+        result = self.do_command(self.dev_args + ['sendpin', '1234'])
+        self.assertIn('error', result)
+        self.assertIn('code', result)
+        self.assertEqual(result['error'], 'The Coldcard does not need a PIN sent from the host')
+        self.assertEqual(result['code'], -9)
+
+class TestColdcardGetXpub(DeviceTestCase):
+    def test_getxpub(self):
+        result = self.do_command(self.dev_args + ['--expert', 'getxpub', 'm/44h/0h/0h/3'])
+        self.assertEqual(result['xpub'], 'tpubDFHiBJDeNvqPWNJbzzxqDVXmJZoNn2GEtoVcFhMjXipQiorGUmps3e5ieDGbRrBPTFTh9TXEKJCwbAGW9uZnfrVPbMxxbFohuFzfT6VThty')
+        self.assertTrue(result['testnet'])
+        self.assertFalse(result['private'])
+        self.assertEqual(result['depth'], 4)
+        self.assertEqual(result['parent_fingerprint'], 'bc123c3e')
+        self.assertEqual(result['child_num'], 3)
+        self.assertEqual(result['chaincode'], '806b26507824f73bc331494afe122f428ef30dde80b2c1ce025d2d03aff411e7')
+        self.assertEqual(result['pubkey'], '0368000bdff5e0b71421c37b8514de8acd4d98ba9908d183d9da56d02ca4fcfd08')
+
+def coldcard_test_suite(simulator, bitcoind, interface):
+    dev_emulator = ColdcardSimulator(simulator)
+
     signtx_cases = [
-        (["legacy"], True, True, False),
-        (["segwit"], True, True, False),
-        (["legacy", "segwit"], True, True, False),
+        (["legacy"], ["legacy"], True, False),
+        (["segwit"], ["segwit"], True, False),
+        (["legacy", "segwit"], ["legacy", "segwit"], True, False),
     ]
 
     # Generic device tests
     suite = unittest.TestSuite()
-    suite.addTest(DeviceTestCase.parameterize(TestColdcardManCommands, rpc, userpass, dev_type, dev_type, sim_path, fpr, '', interface=interface))
-    suite.addTest(DeviceTestCase.parameterize(TestColdcardGetXpub, rpc, userpass, dev_type, dev_type, sim_path, fpr, xpub, interface=interface))
-    suite.addTest(DeviceTestCase.parameterize(TestDeviceConnect, rpc, userpass, dev_type, dev_type, sim_path, fpr, xpub, interface=interface))
-    suite.addTest(DeviceTestCase.parameterize(TestDeviceConnect, rpc, userpass, 'coldcard_simulator', dev_type, sim_path, fpr, xpub, interface=interface))
-    suite.addTest(DeviceTestCase.parameterize(TestGetDescriptors, rpc, userpass, dev_type, dev_type, sim_path, fpr, xpub, interface=interface))
-    suite.addTest(DeviceTestCase.parameterize(TestGetKeypool, rpc, userpass, dev_type, dev_type, sim_path, fpr, xpub, interface=interface))
-    suite.addTest(DeviceTestCase.parameterize(TestDisplayAddress, rpc, userpass, dev_type, dev_type, sim_path, fpr, xpub, interface=interface))
-    suite.addTest(DeviceTestCase.parameterize(TestSignMessage, rpc, userpass, dev_type, dev_type, sim_path, fpr, xpub, interface=interface))
-    suite.addTest(DeviceTestCase.parameterize(TestSignTx, rpc, userpass, dev_type, dev_type, sim_path, fpr, xpub, interface=interface, signtx_cases=signtx_cases))
+    suite.addTest(DeviceTestCase.parameterize(TestColdcardManCommands, bitcoind, emulator=dev_emulator, interface=interface))
+    suite.addTest(DeviceTestCase.parameterize(TestColdcardGetXpub, bitcoind, emulator=dev_emulator, interface=interface))
+    suite.addTest(DeviceTestCase.parameterize(TestDeviceConnect, bitcoind, emulator=dev_emulator, interface=interface, detect_type="coldcard"))
+    suite.addTest(DeviceTestCase.parameterize(TestDeviceConnect, bitcoind, emulator=dev_emulator, interface=interface, detect_type="coldcard"))
+    suite.addTest(DeviceTestCase.parameterize(TestGetDescriptors, bitcoind, emulator=dev_emulator, interface=interface))
+    suite.addTest(DeviceTestCase.parameterize(TestGetKeypool, bitcoind, emulator=dev_emulator, interface=interface))
+    suite.addTest(DeviceTestCase.parameterize(TestDisplayAddress, bitcoind, emulator=dev_emulator, interface=interface))
+    suite.addTest(DeviceTestCase.parameterize(TestSignMessage, bitcoind, emulator=dev_emulator, interface=interface))
+    suite.addTest(DeviceTestCase.parameterize(TestSignTx, bitcoind, emulator=dev_emulator, interface=interface, signtx_cases=signtx_cases))
 
     result = unittest.TextTestRunner(stream=sys.stdout, verbosity=2).run(suite)
-    cleanup_simulator()
-    atexit.unregister(cleanup_simulator)
     return result.wasSuccessful()
 
 if __name__ == '__main__':
@@ -133,6 +166,6 @@ def test_getxpub(self):
     args = parser.parse_args()
 
     # Start bitcoind
-    rpc, userpass = start_bitcoind(args.bitcoind)
+    bitcoind = Bitcoind.create(args.bitcoind)
 
-    sys.exit(not coldcard_test_suite(args.simulator, rpc, userpass, args.interface))
+    sys.exit(not coldcard_test_suite(args.simulator, bitcoind, args.interface))