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

qa: Broken wallet_multiwallet.py #31409

Open
hebasto opened this issue Dec 3, 2024 · 1 comment · May be fixed by #31410
Open

qa: Broken wallet_multiwallet.py #31409

hebasto opened this issue Dec 3, 2024 · 1 comment · May be fixed by #31410
Labels

Comments

@hebasto
Copy link
Member

hebasto commented Dec 3, 2024

On the master branch @ ebe4cac, the wallet_multiwallet.py test has several issues:

  1. This code:

    os.mkdir(wallet_dir('no_access'))
    os.chmod(wallet_dir('no_access'), 0)
    try:
    with self.nodes[0].assert_debug_log(expected_msgs=['Error scanning']):
    walletlist = self.nodes[0].listwalletdir()['wallets']
    finally:
    # Need to ensure access is restored for cleanup
    os.chmod(wallet_dir('no_access'), stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
    assert_equal(sorted(map(lambda w: w['name'], walletlist)), sorted(in_wallet_dir))
    checks for the "Error scanning" message in the debug.log caused by processing the no_access directory. However, the same message can also be generated when parsing the self_walletdat_symlink directory. As a result, the current implementation is prone to producing false-positive results.

  2. Parsing the self_walletdat_symlink directory with bitcoind.exe depends on how it was built. When cross-compiling, the parsing completes without system errors. On the other hand, when building natively, it raises an "unknown error" exception and logs the "Error scanning" message.

  3. This code:

    os.mkdir(wallet_dir('no_access'))
    os.chmod(wallet_dir('no_access'), 0)
    try:
    with self.nodes[0].assert_debug_log(expected_msgs=['Error scanning']):
    walletlist = self.nodes[0].listwalletdir()['wallets']
    finally:
    # Need to ensure access is restored for cleanup
    os.chmod(wallet_dir('no_access'), stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
    is not portable due to its use of os.chmod:

Note: Although Windows supports chmod(), you can only set the file’s read-only flag with it (via the stat.S_IWRITE and stat.S_IREAD constants or a corresponding integer value).

@hebasto hebasto added the Tests label Dec 3, 2024
@maflcko
Copy link
Member

maflcko commented Dec 3, 2024

I guess it would be good to separate the no_access and symlink unit tests into two test cases, not combine them into one. This would allow to skip just one of them, if needed on Windows.

@hebasto hebasto linked a pull request Dec 3, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants