HomePhabricator

test: tool wallet test coverage for unexpected writes to wallet

Description

test: tool wallet test coverage for unexpected writes to wallet

Summary:

  • test: Add log messages to test/functional/tool_wallet.py

and update code comments as per Python PEP 8 style guide.

  • test: Split tool_wallet.py test into subtests

as per Marco Falke review suggestion.

  • test: Tool wallet test coverage for unexpected writes to wallet

This commit adds test coverage in test/functional/tool_wallet.py to reproduce unexpected writes to the wallet as described in https://github.com/bitcoin/bitcoin/issues/15608:

  • wallet tool info unexpectedly writes to the wallet file if the wallet file permissions are read/write.
  • wallet tool info raises with "Error loading . Is wallet being used by another process?" if the wallet file permissions are read-only.
  1. Reproduce the reported issue, define the current unexpected behavior, and add test coverage to guide a future fix in the form of commented-out assertions to be uncommented when testing/fixing.
  1. Provisionally extend the same coverage to the wallet tool create test and the getwalletinfo test as regression tests while fixing the issue.
  1. Add some logging for sanity checking.

Changes after rebase:

  1. Make wallet_path an instance method instead of a function in tool_wallet.py as per Marco Falke review suggestion.
  1. Assert wallet permissions instead of logging them in tool_wallet.py. This ran into an issue with Appveyor keeping permissions at 666 so allowed for 666 as a workaround.
  1. Change the added logging from info to debug level.
  1. More helpful assertions order in tool_wallet.py#assert_tool_output. This change makes #assert_tool_output raise "Error loading wallet.dat. Is wallet being used by another process?" rather than a less-helpful message when debugging the read-only wallet permissions issue.

This is a backport of Core PR15687

Test Plan:

ninja all check-all

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D7706

Details

Provenance
Jon Atack <jon@atack.com>Authored on Mar 27 2019, 15:34
deadalnixCommitted on Oct 1 2020, 14:15
deadalnixPushed on Oct 1 2020, 14:15
Reviewer
Restricted Project
Differential Revision
D7706: test: tool wallet test coverage for unexpected writes to wallet
Parents
rABCfd902a90a6a2: wallet: Avoid translating RPC errors when creating txs
Branches
Unknown
Tags
Unknown