Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ci: add hwi tests for core and legacy builds #2074

Merged
merged 1 commit into from
Feb 8, 2022
Merged

ci: add hwi tests for core and legacy builds #2074

merged 1 commit into from
Feb 8, 2022

Conversation

vdovhanych
Copy link
Member

This pr adds hwi trezor test to our CI. Not sure if this is the best solution for adding the tests to our ci.

Also, If you have any idea how to not directly point to /nix/store for bitcoind binary, use a relative path so it doesn't break with each nix update. Suggestions welcome.

prusnak
prusnak reviewed Jan 20, 2022
View reviewed changes
ci/test.yml Outdated Show resolved Hide resolved
@vdovhanych
Copy link
Member Author

Hi i found interesting outcomes of these tests. If I test it against released firmware everything passes

trezor_1: test_enumerate (test_device.TestDeviceConnect) ... ok
trezor_1: test_fingerprint_autodetect (test_device.TestDeviceConnect) ... ok
trezor_1: test_no_type (test_device.TestDeviceConnect) ... ok
trezor_1: test_path_type (test_device.TestDeviceConnect) ... ok
trezor_1: test_type_only_autodetect (test_device.TestDeviceConnect) ... ok
trezor_1: test_getdescriptors (test_device.TestGetDescriptors) ... ok
trezor_1: test_getkeypool (test_device.TestGetKeypool) ... ok
trezor_1: test_big_tx (test_device.TestSignTx) ... ok
trezor_1: test_signtx (test_device.TestSignTx) ... ok
trezor_1: test_display_address_bad_path (test_device.TestDisplayAddress) ... ok
trezor_1: test_display_address_descriptor (test_device.TestDisplayAddress) ... ok
trezor_1: test_display_address_multisig (test_device.TestDisplayAddress) ... ok
trezor_1: test_display_address_path (test_device.TestDisplayAddress) ... ok
trezor_1: test_bad_path (test_device.TestSignMessage) ... ok
trezor_1: test_sign_msg (test_device.TestSignMessage) ... ok
trezor_1: test_backup (__main__.TestTrezorManCommands) ... ok
trezor_1: test_label (__main__.TestTrezorManCommands) ... ok
trezor_1: test_passphrase (__main__.TestTrezorManCommands) ... ok
trezor_1: test_pins (__main__.TestTrezorManCommands) ... ok
trezor_1: test_setup_wipe (__main__.TestTrezorManCommands) ... ok
trezor_1: test_label (__main__.TestTrezorLabel) ... ok
trezor_1: test_enumerate (test_device.TestDeviceConnect) ... ok
trezor_1: test_fingerprint_autodetect (test_device.TestDeviceConnect) ... ok
trezor_1: test_no_type (test_device.TestDeviceConnect) ... ok
trezor_1: test_path_type (test_device.TestDeviceConnect) ... ok
trezor_1: test_type_only_autodetect (test_device.TestDeviceConnect) ... ok
trezor_1: test_expert_getxpub (__main__.TestTrezorGetxpub) ... ok
trezor_1: test_getxpub (__main__.TestTrezorGetxpub) ... ok

----------------------------------------------------------------------
Ran 28 tests in 66.415s

OK

but if the tests run against latest master builds the test in the ci fails

trezor_1: test_enumerate (test_device.TestDeviceConnect) ... ok
trezor_1: test_fingerprint_autodetect (test_device.TestDeviceConnect) ... ok
trezor_1: test_no_type (test_device.TestDeviceConnect) ... ok
trezor_1: test_path_type (test_device.TestDeviceConnect) ... ok
trezor_1: test_type_only_autodetect (test_device.TestDeviceConnect) ... FAIL
trezor_1: test_getdescriptors (test_device.TestGetDescriptors) ... ok
trezor_1: test_getkeypool (test_device.TestGetKeypool) ... ok
trezor_1: test_big_tx (test_device.TestSignTx) ... ok
trezor_1: test_signtx (test_device.TestSignTx) ... ok
trezor_1: test_display_address_bad_path (test_device.TestDisplayAddress) ... ok
trezor_1: test_display_address_descriptor (test_device.TestDisplayAddress) ... ok
trezor_1: test_display_address_multisig (test_device.TestDisplayAddress) ... ok
trezor_1: test_display_address_path (test_device.TestDisplayAddress) ... ok
trezor_1: test_bad_path (test_device.TestSignMessage) ... ok
trezor_1: test_sign_msg (test_device.TestSignMessage) ... ok
trezor_1: test_backup (__main__.TestTrezorManCommands) ... ok
trezor_1: test_label (__main__.TestTrezorManCommands) ... ok
trezor_1: test_passphrase (__main__.TestTrezorManCommands) ... ok
trezor_1: test_pins (__main__.TestTrezorManCommands) ... ok
trezor_1: test_setup_wipe (__main__.TestTrezorManCommands) ... ok
trezor_1: test_label (__main__.TestTrezorLabel) ... ok
trezor_1: test_enumerate (test_device.TestDeviceConnect) ... ok
trezor_1: test_fingerprint_autodetect (test_device.TestDeviceConnect) ... ok
trezor_1: test_no_type (test_device.TestDeviceConnect) ... ok
trezor_1: test_path_type (test_device.TestDeviceConnect) ... ok
trezor_1: test_type_only_autodetect (test_device.TestDeviceConnect) ... ok
trezor_1: test_expert_getxpub (__main__.TestTrezorGetxpub) ... ok
trezor_1: test_getxpub (__main__.TestTrezorGetxpub) ... ok
======================================================================
FAIL: trezor_1: test_type_only_autodetect (test_device.TestDeviceConnect)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builds/satoshilabs/trezor/trezor-firmware/HWI/test/test_device.py", line 180, in test_type_only_autodetect
    self.assertEqual(gmxp_res['xpub'], self.master_xpub)
AssertionError: 'xpub6BiVtCpG9fQPxnPmHXG8PhtzQdWC2Su4qWu6XW9tpW[61 chars]8wZy' != 'xpub6D1weXBcFAo8CqBbpP4TbH5sxQH8ZkqC5pDEvJ95rN[61 chars]4AFH'
- xpub6BiVtCpG9fQPxnPmHXG8PhtzQdWC2Su4qWu6XW9tpWFYhxydCLJGrWBJZ5H6qTAHdPQ7pQhtpjiYZVZARo14qHiay2fvrX996oEP42u8wZy
+ xpub6D1weXBcFAo8CqBbpP4TbH5sxQH8ZkqC5pDEvJ95rNNBZC9zrKmZP2fXMuve7ZRBe18pWQQsGg68jkq24mZchHwYENd8cCiSb71u3KD4AFH
----------------------------------------------------------------------
Ran 28 tests in 173.919s
FAILED (failures=1)

could this be a valid error or is there something wrong with my ci setup?

@mmilata
Copy link
Member

mmilata commented Jan 21, 2022

Hmm, the test case is using different method to connect to the emulator, it's almost as if it connected to wrong trezor and got wrong xpub. But there should be no other emulators running? Can you add hwi enumerate before the tests to the next run?

https://github.com/bitcoin-core/HWI/blob/master/test/test_device.py#L178-L185

mmilata
mmilata reviewed Jan 21, 2022
View reviewed changes
ci/test.yml Outdated Show resolved Hide resolved
@vdovhanych
Copy link
Member Author

There seems to be an issue in the HWI test since this pr most likely. test_getkeypool (test_device.TestGetKeypool) is related to this. Unfortunately removing privileged access introduced different errors with the following output.

@mmilata
Copy link
Member

mmilata commented Feb 2, 2022 •
edited
Loading

The issue with test_getkeypool is that it requires bitcoind including bitcoin/bitcoin#22558. In 6349e77 I've added it, behind the fullDeps arg to avoid building bitcoind from source when it's not needed.

Now it remains to figure out the LIBUSB_ERROR_NO_DEVICE issue, looks like somebody ran into it before: bitcoin-core/HWI#529.

@vdovhanych vdovhanych changed the title WIP - ci: add hwi tests for core and legacy builds ci: add hwi tests for core and legacy builds Feb 3, 2022
@vdovhanych
Copy link
Member Author

vdovhanych commented Feb 3, 2022 •
edited
Loading

Thanks to @mmilata for debugging the weird issue with the running in containers. This now should be ready, and it looks like running correctly now.

Also @mmilata @matejcik I'm guessing this should be enough if we run it on scheduled pipelines only right? or do we want to have it on all ?

@mmilata
Copy link
Member

mmilata commented Feb 4, 2022 •
edited
Loading

No strong opinion on this but the test itself takes 8 minutes to run on TT (2m on T1) which is not that much so running it every time would be fine.

@vdovhanych
Copy link
Member Author

@matejcik @mmilata let's merge it then like this?

matejcik
matejcik approved these changes Feb 8, 2022
View reviewed changes
Copy link
Contributor

@matejcik matejcik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK from me, i'll leave merging to @mmilata

@mmilata mmilata merged commit 48d7f71 into master Feb 8, 2022
@mmilata mmilata deleted the vdovhanych/hwi-test branch February 8, 2022 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prusnak prusnak prusnak left review comments

@mmilata mmilata mmilata left review comments

@matejcik matejcik matejcik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vdovhanych @mmilata @prusnak @matejcik