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

chore: update trezor_tests binary #2860

Merged
merged 1 commit into from
Mar 13, 2023
Merged

chore: update trezor_tests binary #2860

merged 1 commit into from
Mar 13, 2023

Conversation

ph4r05
Copy link
Contributor

@ph4r05 ph4r05 commented Mar 2, 2023 •
edited
Loading

Improved Monero-Trezor test suite as occasionally passphrase-related bug reports have surfaced.

Tests revamp, always use passphrase flow with transaction signing, more passphrase-related tests, pin test, invalid passphrase test. Chain generation optimized.

Using release: https://github.com/ph4r05/monero/releases/tag/v0.18.1.1-dev-tests-u18.04-02
Related to this PR: monero-project/monero#8752

@ph4r05 ph4r05 requested a review from matejcik as a code owner March 2, 2023 15:58
@ph4r05 ph4r05 marked this pull request as draft March 9, 2023 11:47
@matejcik
Copy link
Contributor

matejcik commented Mar 9, 2023

you marked this as draft, any reason not to merge?

@ph4r05
Copy link
Contributor Author

ph4r05 commented Mar 9, 2023

Thanks for approval @matejcik! I've just noticed there is an issue I need to investigate with passphrase tests. Will ping you once this is done, thanks!

@ph4r05
chore: update trezor_tests binary
6395612 
- tests revamp, always use passphrase flow with transaction signing, more passphrase-related tests, pin test, invalid passphrase test. Chain generation optimized
@ph4r05
Copy link
Contributor Author

ph4r05 commented Mar 10, 2023

@matejcik I had to implement few fixes, can you pls try it now? Thanks!

@ph4r05 ph4r05 marked this pull request as ready for review March 10, 2023 11:21
@ph4r05
Copy link
Contributor Author

ph4r05 commented Mar 10, 2023

btw should I add a changelog to this PR? Is it relevant?

@ph4r05 ph4r05 requested a review from matejcik March 10, 2023 16:49
@matejcik
Copy link
Contributor

no reason to add a changelog. the commit should have been tagged with [no changelog], but the changelog check is not merge-blocking so no point in doing it now.

i'll merge this when the full CI pipeline passes

@ph4r05
Copy link
Contributor Author

ph4r05 commented Mar 13, 2023 •
edited
Loading

ok cool! New monero tests passed, https://gitlab.com/satoshilabs/trezor/trezor-firmware/-/jobs/3922204537 🎉
Tests times are 251 s, compared to original 328 s it is 23% faster with 4 more tests added (and new chain was generated). If the chain file is cached, it should be even faster.

@matejcik matejcik merged commit eacd153 into trezor:master Mar 13, 2023
@ph4r05 ph4r05 deleted the pr/xmr-tests branch March 13, 2023 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matejcik matejcik Awaiting requested review from matejcik

Assignees

@ph4r05 ph4r05

Labels
None yet
Projects
Firmware
Status: ✅ Approved
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ph4r05 @matejcik