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

Jump to bottom

added device tests for shamir dry run and warnings #358

Merged
merged 1 commit into from Jul 29, 2019
Merged

added device tests for shamir dry run and warnings #358

merged 1 commit into from Jul 29, 2019

Conversation

ciny
Copy link
Contributor

@ciny ciny commented Jul 24, 2019

this PR implements #342 except the "reboot" tests

@ciny ciny added tests Automated integration tests Shamir labels Jul 24, 2019
@ciny ciny added this to the 2019-08 milestone Jul 24, 2019
@ciny ciny requested a review from tsusanka July 24, 2019 16:31
@ciny ciny added this to In progress in Shamir via automation Jul 24, 2019
@ciny ciny force-pushed the ciny/iss342-device-tests branch from 6a9edd6 to 8d422f6 Compare July 24, 2019 16:58
@matejcik
Copy link
Contributor

new tests should be written with the input_flow style

i have a rewrite of recoverydevice_shamir in progress, will finish and submit it tomorrow. but please don't add the "dryrun" test in the old style

@ciny ciny force-pushed the ciny/iss342-device-tests branch from 8d422f6 to 0943624 Compare July 24, 2019 17:14
@ciny
Copy link
Contributor Author

ciny commented Jul 24, 2019

The idea was to have at least some tests in master before the freeze. I have added refactor to new style to the task and was planning on doing that on Friday. Not having them in master is not a blocker for the freeze IMHO so we can wait until the refactor is done.

@tsusanka
Copy link
Contributor

Yes, let's merge this after the new style refactor then.

tsusanka
tsusanka requested changes Jul 24, 2019
View reviewed changes
Copy link
Contributor

@tsusanka tsusanka left a comment •
edited

Choose a reason for hiding this comment

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

Consider this more of a prelimary check with just a few minor issues. Let's refactor this to the style and fix those few nit-picks.

python/trezorlib/tests/device_tests/test_msg_recoverydevice_shamir.py Outdated Show resolved Hide resolved
python/trezorlib/tests/device_tests/test_msg_recoverydevice_shamir.py Outdated Show resolved Hide resolved
python/trezorlib/tests/device_tests/test_msg_recoverydevice_shamir.py Outdated Show resolved Hide resolved
python/trezorlib/tests/device_tests/test_msg_recoverydevice_shamir.py Outdated Show resolved Hide resolved
python/trezorlib/tests/device_tests/test_msg_recoverydevice_shamir_dryrun.py Outdated Show resolved Hide resolved
Shamir automation moved this from In progress to Review in progress Jul 24, 2019
@tsusanka
Copy link
Contributor

@ciny matejcik fixed the shamir test, do you want to fix the dry run accordingly?

@ciny
Copy link
Contributor Author

ciny commented Jul 26, 2019

@tsusanka yup, it's in progress.

@matejcik matejcik mentioned this pull request Jul 26, 2019
Merged
Shamir automation moved this from Review in progress to Reviewer approved Jul 28, 2019
@tsusanka
Copy link
Contributor

@matejcik could you check @ciny's work? It LGTM

Shamir automation moved this from Reviewer approved to Review in progress Jul 29, 2019
@matejcik
Copy link
Contributor

one minor nitpick otherwise LGTM.
actually if you need this, feel free to merge it now, i'll be going back to this later in the afternoon anyway

@ciny ciny force-pushed the ciny/iss342-device-tests branch from 87d0174 to db35a11 Compare July 29, 2019 08:23
@tsusanka tsusanka merged commit 5584939 into master Jul 29, 2019
Shamir automation moved this from Review in progress to Done Jul 29, 2019
@tsusanka tsusanka deleted the ciny/iss342-device-tests branch July 29, 2019 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matejcik matejcik matejcik requested changes

@tsusanka tsusanka tsusanka approved these changes

Assignees
No one assigned
Labels
tests Automated integration tests
Projects
No open projects
Shamir
  
Done
Milestone
2019-08
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ciny @matejcik @tsusanka