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.

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

test: Fix label tests #536

Merged
merged 2 commits into from
Oct 25, 2021
Merged

Conversation

achow101
Copy link
Member

@achow101 achow101 commented Oct 24, 2021

These tests weren't working. Also fixed another keepkey test bug.

@achow101 achow101 force-pushed the fix-trezor-label-test branch from b2ab5ae to a39a741 Compare October 24, 2021 03:52
@achow101 achow101 force-pushed the fix-trezor-label-test branch from a39a741 to 6b043d3 Compare October 24, 2021 04:23
result = self.do_command(self.dev_args + ['enumerate'])
for dev in result:
if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:11044':
self.assertEqual(result['label'], 'HWI Keepkey')
if dev['type'] == 'keepkey' and dev['path'] == 'udp:127.0.0.1:11044':
Copy link
Collaborator

@prusnak prusnak Oct 24, 2021

Choose a reason for hiding this comment

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

Shouldn't this stay as "trezor"?

Other occurences in this file use it like this:

if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:11044':

if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:11044':

if dev['type'] == 'trezor' and dev['path'] == 'udp:127.0.0.1:11044':

Copy link
Member Author

Choose a reason for hiding this comment

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

No, those are actually incorrect! I've added a commit that fixes them and will cause the tests to fail if these are incorrect.

There are a number of trezor and keepkey tests which rely on enumerate
and then looking at the enumerated results. However some are looking for
the wrong thing and inadverdently doing nothing. So extra checks are
added to make sure they are actually doing something.
@achow101 achow101 merged commit 8c6763b into bitcoin-core:master Oct 25, 2021
@achow101
Copy link
Member Author

Merging as CI passed

@@ -285,20 +297,30 @@ def test_passphrase(self):

# A passphrase will need to be sent
result = self.do_command(self.dev_args + ['enumerate'])
print(result)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you forgot to remove this debug line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants