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

functional tests: fix race in restore wallet refresh #9720

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

j-berman
Copy link
Collaborator

@j-berman j-berman commented Jan 23, 2025

Needed to disable auto refresh before restoring the wallet, so refresh doesn't run in the idle loop upon restoring.

Fixes this test failure highlighted by @tobtoht (apologies for missing it).

@selsta
Copy link
Collaborator

selsta commented Jan 23, 2025

Just to confirm, this was purely a test issue and this case can not happen inside a real wallet?

@j-berman
Copy link
Collaborator Author

Yep, correct

@@ -57,6 +57,15 @@ def diff_incoming_transfers(actual_transfers, expected_transfers):
# wallet2 m_transfers container is ordered and order should be the same across rescans
diff_transfers(actual_transfers, expected_transfers, ignore_order = False)

def restore_wallet(wallet, seed, restore_height = 0, filename = '', password = '', ):

Choose a reason for hiding this comment

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

The trailing , after the last param, though allowed, seems to break with the style elsewhere in this file, as well as python convention.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@iamamyth
Copy link

The style comment doesn't impact approval (I saw the issue prior to signing off), as every part of the code style in these tests violates most linter rule sets, so one extra line doesn't matter.

@tobtoht tobtoht merged commit d42db81 into monero-project:master Jan 24, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants