qa: Fix wallet_multiwallet.py #31410

pull hebasto wants to merge 3 commits into bitcoin:master from hebasto:241203-multiwallet changing 2 files +29 −13
  1. hebasto commented at 1:37 pm on December 3, 2024: member

    Fixes #31409.

    The first commit prevents possible false positives by ensuring that each condition potentially causing the “Error scanning” log message is tested separately.

    The second commit disables a problematic check on Windows.

  2. hebasto added the label Tests on Dec 3, 2024
  3. DrahtBot commented at 1:37 pm on December 3, 2024: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31410.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Approach ACK stickies-v
    Stale ACK maflcko

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    No conflicts as of last run.

  4. hebasto force-pushed on Dec 3, 2024
  5. DrahtBot added the label CI failed on Dec 3, 2024
  6. in test/functional/wallet_multiwallet.py:132 in b58c8fe005 outdated 
    125@@ -129,15 +126,23 @@ def wallet_file(name):
126         for wallet_name in to_load:
127             self.nodes[0].loadwallet(wallet_name)
128 
129+        # Tests for possible scanning errors:
130+        # 1. "Permission denied" error.
131         os.mkdir(wallet_dir('no_access'))
132         os.chmod(wallet_dir('no_access'), 0)
    maflcko commented at 2:36 pm on December 3, 2024:
    Would it be possible to achieve the same with icacls on Windows?
    hebasto commented at 2:48 pm on December 3, 2024:
    Via subprocess?
    hebasto commented at 9:12 pm on December 3, 2024:
    I’m not sure if testing the scenario with the modified ACL on Windows is worth the added complexity of the test script.
  7. hebasto force-pushed on Dec 3, 2024
  8. hebasto marked this as ready for review on Dec 3, 2024
  9. hebasto added the label Wallet on Dec 3, 2024
  10. maflcko removed the label CI failed on Dec 4, 2024
  11. in test/functional/wallet_multiwallet.py:132 in fd8d7c367f outdated 
    148+            assert_equal(sorted(map(lambda w: w['name'], walletlist)), sorted(in_wallet_dir))
149+        # 2. "Too many levels of symbolic links" error.
150+        os.mkdir(wallet_dir('self_walletdat_symlink'))
151+        os.symlink('wallet.dat', wallet_dir('self_walletdat_symlink/wallet.dat'))
152+        with self.nodes[0].assert_debug_log(expected_msgs=['Error scanning']):
153+            walletlist = self.nodes[0].listwalletdir()['wallets']
    maflcko commented at 7:30 am on December 4, 2024:
    Could do a third listwalletdir without the symlink to check that the test is working by checking for absence of error scanning?
    hebasto commented at 9:41 am on December 4, 2024:
    Thanks! Done.
  12. hebasto force-pushed on Dec 4, 2024
  13. maflcko commented at 8:48 pm on December 5, 2024: member

    review ACK c0127439597ee9d09b8e117be7d6226a68b45721 🎑

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK c0127439597ee9d09b8e117be7d6226a68b45721 🎑
3tf/up7OMkfp/8WkQfPI/JfkrY0jnqtfRAg2T09eD2yTw/BRFey4pUZPMlJu+Y93FYLisAf28Ld2wEeMnGisIBQ==
  14. maflcko added this to the milestone 29.0 on Dec 18, 2024
  15. hebasto commented at 11:55 am on December 20, 2024: member
    Friendly ping a Python connoisseur @stickies-v :)
  16. Countrymom71 approved
  17. in test/functional/wallet_multiwallet.py:144 in 3f53cc8b8c outdated 
    147+                with self.nodes[0].assert_debug_log(expected_msgs=['Error scanning']):
148+                    walletlist = self.nodes[0].listwalletdir()['wallets']
149+            finally:
150+                # Need to ensure access is restored for cleanup
151+                os.chmod(wallet_dir('no_access'), stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
152+            assert_equal(sorted(map(lambda w: w['name'], walletlist)), sorted(in_wallet_dir))
    stickies-v commented at 11:22 am on January 16, 2025:
    Making this check conditional on platform is undocumented in the commit message, and seems orthogonal to the goal of testing errors individually. I think it would be best to split this out in a separate commit and document it?
    hebasto commented at 11:20 am on January 21, 2025:

    I don’t think these changes can be meaningfully separated. Applying either change independently would break the test or render it incomplete.

    The commit message has been updated though.

  18. stickies-v commented at 11:50 am on January 16, 2025: contributor

    Approach ACK c0127439597ee9d09b8e117be7d6226a68b45721

    I’m not familiar enough with symlinks and build toolchains to have an opinion on c0127439597ee9d09b8e117be7d6226a68b45721, but it seems reasonable.

  19. in test/functional/wallet_multiwallet.py:150 in c012743959 outdated 
    145+                # Need to ensure access is restored for cleanup
146+                os.chmod(wallet_dir('no_access'), stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
147+            assert_equal(sorted(map(lambda w: w['name'], walletlist)), sorted(in_wallet_dir))
148+        # 2. "Too many levels of symbolic links" error.
149+        # This test cannot be conducted robustly on Windows
150+        # because it depends on the build toolchain:
    fanquake commented at 12:05 pm on January 16, 2025:

    In c0127439597ee9d09b8e117be7d6226a68b45721:

    because it depends on the build toolchain:

    Even if this is the case, is the best option to just to do nothing? Shouldn’t we atleast test the known behaviour for our release build, so we’d catch if this changes (is fixed?) in future? Otherwise my understanding is if this changes again, it’s just going to be hidden/ignored.

    maflcko commented at 12:16 pm on January 16, 2025:
    I guess the check could be limited to exclude the msvc build?
    hebasto commented at 11:26 am on January 21, 2025:

    Shouldn’t we atleast test the known behaviour for our release build…?

    As documented in the code comment:

    A cross-compiled bitcoind.exe parses self_walletdat_symlink without errors.

    Therefore, the release build won’t trigger the “Error scanning” message, and this test would fail on the master branch.

    fanquake commented at 11:26 am on January 21, 2025:

    I guess the check could be limited to exclude the msvc build?

    That would make the most sense to me.

    fanquake commented at 11:28 am on January 21, 2025:

    Therefore, the release build won’t trigger the “Error scanning” message, and this test would fail on the master branch.

    Sure, so you can check that that is happening when testing on Windows.

    hebasto commented at 4:06 pm on January 29, 2025:

    I guess the check could be limited to exclude the msvc build?

    Unfortunately, I can’t find a simple way to detect whether binaries were built with MSVC without adding extra complexity to the Python code.

    maflcko commented at 9:23 am on February 4, 2025:
    I’d be fine with a hacky check that only runs on windows and checks for a magic msvc binary string in open(exe_path, "rb").read(), but anything is fine and this is not a blocker from my side.
  20. hebasto force-pushed on Jan 21, 2025
  21. maflcko commented at 11:24 am on January 21, 2025: member

    Only change is the commit msg

    re-ACK e717fb5a429d9d00e008fa1eb2b85179050be8fe 🍣

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: re-ACK e717fb5a429d9d00e008fa1eb2b85179050be8fe 🍣
370Rky7BeBYMJwMIgU5Z47QQaP7eIFFxizwukIZAlazkuNY6DYE5ckOBE/wtHkl58rSQdK2QEsSU14lp+IPPVAA==
  22. DrahtBot requested review from stickies-v on Jan 21, 2025
  23. Countrymom71 commented at 3:04 pm on January 29, 2025: none
    I need help on Cloudflare seeing my wallet. I have tried setting up all the domains and things to keep it secure and reroute things and until I get it right I can’t see my account with crypto. I only want to use to to keep my crypto secure and send and receive. I have no interest in anything else on the computer I am old and don’t understand all the computer lingo I just need some simple easy way to get to my account and use it pay my fee and that’s it. The other stuff on here makes me want to scream It’s confusing. I even got a younger girl to come help me and paid her. She wasn’t much help you can see. I believe if I had a number to call and someone could talk me thru a lil I would figure it out and life would be grand. I have done a lot and so did the girl I hired but something must be wrong somewhere. Please help me or tell me who to call thanks
  24. hebasto commented at 8:57 pm on February 1, 2025: member

    FWIW, the Windows UCRT-linked cross-compiled builds still need this fix.

    UPD. Further investigation is needed.

  25. hodlinator commented at 11:19 pm on February 8, 2025: contributor

    Code review e717fb5a429d9d00e008fa1eb2b85179050be8fe

    Guix cross-build failure

    Attempted to run Guix build e717fb5a429d9d00e008fa1eb2b85179050be8fe cross-built for Windows, but it fails already on test “0” / line 133:

    https://github.com/bitcoin/bitcoin/blob/e717fb5a429d9d00e008fa1eb2b85179050be8fe/test/functional/wallet_multiwallet.py#L129-L133

     0$ BITCOIND=/c/Users/hodlinator/Downloads/bitcoind-e717fb5a429d.exe build/test/functional/wallet_multiwallet.py
 12025-02-08T22:43:58.813000Z TestFramework (INFO): PRNG seed is: 7271806868081798699
 22025-02-08T22:43:58.862000Z TestFramework (INFO): Initializing test directory C:\Users\HODLIN~1\AppData\Local\Temp\bitcoin_func_test_fj5a7zvd
 32025-02-08T22:44:00.455000Z TestFramework (ERROR): Assertion failed
 4Traceback (most recent call last):
 5  File "C:\Users\hodlinator\bitcoin\build\test\functional\test_framework\test_framework.py", line 135, in main
 6    self.run_test()
 7    ~~~~~~~~~~~~~^^
 8  File "C:\Users\hodlinator\bitcoin\build\test\functional\wallet_multiwallet.py", line 133, in run_test
 9    assert_equal(sorted(map(lambda w: w['name'], walletlist)), sorted(in_wallet_dir))
10    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11  File "C:\Users\hodlinator\bitcoin\build\test\functional\test_framework\util.py", line 77, in assert_equal
12    raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
13AssertionError: not(['default_wallet', 'recursive_dir_symlink\\wallets\\default_wallet',
14'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\default_wallet',
15'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\default_wallet',
16'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\default_wallet',
17'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\sub\\w5',
18'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w',
19'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w1',
20'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w2',
21'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w3',
22'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7',
23'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7_symlink',
24'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\sub\\w5',
25'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w',
26'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w1',
27'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w2',
28'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w3',
29'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7',
30'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7_symlink',
31'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\sub\\w5',
32'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w',
33'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w1',
34'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w2',
35'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w3',
36'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7',
37'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7_symlink',
38'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\sub\\w5',
39'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w',
40'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w1',
41'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w2',
42'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w3',
43'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7',
44'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7_symlink',
45'recursive_dir_symlink\\wallets\\sub\\w5',
46'recursive_dir_symlink\\wallets\\w', 'recursive_dir_symlink\\wallets\\w1', 'recursive_dir_symlink\\wallets\\w2',
47'recursive_dir_symlink\\wallets\\w3', 'recursive_dir_symlink\\wallets\\w7', 'recursive_dir_symlink\\wallets\\w7_symlink',
48'sub\\w5', 'w', 'w1', 'w2', 'w3', 'w7', 'w7_symlink']
49== ['default_wallet', 'sub\\w5', 'w', 'w1', 'w2', 'w3', 'w7', 'w7_symlink'])
502025-02-08T22:44:00.507000Z TestFramework (INFO): Stopping nodes

    Should that not be working?

    C++ behavior difference when iterating directories?

    I managed to come up with 584e2fcd943ede052aa3641befae121454ae091e, which adds extra error checking that appears to be missing from the wallet/db.cpp code.

    After adding that, I was able to re-implement permission checking for Windows in 0f67bd65ec3536ba618e6ae79931a68a2517f375 (for native builds, cross compiled seem to fail earlier as noted above). It does use ACLs since I hadn’t seen #31410 (review) until I was pretty much done with the implementation (didn’t want to bias my review). Might be something for a follow-up PR.

    Windows Developer Mode parenthesis

    First thing I attempted on Windows was to run the wallet_multiwallet.py test on the current parent of this PR (ebe4cac38bf6c510b00b8e080acab079c54016d6):

     0$ BITCOIND=/c/Users/hodlinator/bitcoin/build/src/Release/bitcoind.exe build/test/functional/wallet_multiwallet.py
 12025-02-08T12:32:02.546000Z TestFramework (INFO): PRNG seed is: 1953116584645958224
 22025-02-08T12:32:02.566000Z TestFramework (INFO): Initializing test directory C:\Users\HODLIN~1\AppData\Local\Temp\bitcoin_func_test_tqrp64pv
 32025-02-08T12:32:03.168000Z TestFramework (ERROR): Unexpected exception caught during testing
 4Traceback (most recent call last):
 5  File "C:\Users\hodlinator\bitcoin\build\test\functional\test_framework\test_framework.py", line 135, in main
 6    self.run_test()
 7    ~~~~~~~~~~~~~^^
 8  File "C:\Users\hodlinator\bitcoin\build\test\functional\wallet_multiwallet.py", line 85, in run_test
 9    os.symlink('w7', wallet_dir('w7_symlink'))
10    ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11OSError: [WinError 1314] A required privilege is not held by the client: 'w7' -> 'C:\\Users\\HODLIN~1\\AppData\\Local\\Temp\\bitcoin_func_test_tqrp64pv\\node0\\regtest\\wallets\\w7_symlink'

    Was able to find a solution on StackOverflow - activated Developer Mode in Windows settings to avoid it. After that the test runs fine using native-built Windows binaries (without changes from this PR). Later I changed my user to be Administrator, might have solved it too.

  26. in test/functional/wallet_multiwallet.py:135 in e717fb5a42 outdated 
    133+        # 0. Baseline, no errors.
134+        with self.nodes[0].assert_debug_log(expected_msgs=[], unexpected_msgs=['Error scanning']):
135+            walletlist = self.nodes[0].listwalletdir()['wallets']
136+        assert_equal(sorted(map(lambda w: w['name'], walletlist)), sorted(in_wallet_dir))
137+        # 1. "Permission denied" error.
138+        if platform.system() != 'Windows' and os.geteuid() != 0:
    hodlinator commented at 10:00 am on February 10, 2025:

    nit: Could add a warning when skipping the test because of user permissions:

    0        if platform.system() != 'Windows':
1            if os.geteuid() == 0:
2                self.log.warning('Skipping "permission denied"-test requiring non-root user.')
3            else:

    (Ran into something similar on my own branch as sanitizer-tests apparently run with root permissions (CI log)).

    hebasto commented at 1:25 pm on February 10, 2025:
    Thanks! Applied.
  27. hebasto force-pushed on Feb 10, 2025
  28. hebasto commented at 3:33 pm on February 10, 2025: member

    Attempted to run Guix build e717fb5 cross-built for Windows, but it fails already on test “0” / line 133:

    Is this failure introduced by this PR, or does it occur on the master branch as well?

    Was able to find a solution on StackOverflow - activated Developer Mode in Windows settings to avoid it.

    There are many ways to enable the “Create symbolic links” privilege (SeCreateSymbolicLinkPrivilege).

  29. hodlinator commented at 10:52 pm on February 10, 2025: contributor

    Attempted to run Guix build e717fb5 cross-built for Windows, but it fails already on test “0” / line 133:

    Is this failure introduced by this PR, or does it occur on the master branch as well?

    I think the way cross-built binaries fail was new thanks to your introduced “0. Baseline, no errors."-test.

    With the same executable (e717fb5a429d = this PR which only includes Python changes) on master (ebe4cac38bf6c510b00b8e080acab079c54016d6 Python scripts), I get a different error, since os.chmod() has no effect on Windows, the expected scanning error is not emitted:

     0$ BITCOIND=/c/Users/hodlinator/Downloads/bitcoind-e717fb5a429d.exe build/test/functional/wallet_multiwallet.py
 12025-02-10T21:40:56.073000Z TestFramework (INFO): PRNG seed is: 4150940621330997535
 22025-02-10T21:40:56.093000Z TestFramework (INFO): Initializing test directory C:\Users\HODLIN~1\AppData\Local\Temp\bitcoin_func_test_pszwxbng
 32025-02-10T21:40:59.598000Z TestFramework (ERROR): Assertion failed
 4Traceback (most recent call last):
 5  File "C:\Users\hodlinator\bitcoin\build\test\functional\test_framework\test_framework.py", line 135, in main
 6    self.run_test()
 7    ~~~~~~~~~~~~~^^
 8  File "C:\Users\hodlinator\bitcoin\build\test\functional\wallet_multiwallet.py", line 135, in run_test
 9    with self.nodes[0].assert_debug_log(expected_msgs=['Error scanning']):
10         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.13_3.13.752.0_x64__qbz5n2kfra8p0\Lib\contextlib.py", line 148, in __exit__
12    next(self.gen)
13    ~~~~^^^^^^^^^^
14  File "C:\Users\hodlinator\bitcoin\build\test\functional\test_framework\test_node.py", line 520, in assert_debug_log
15    self._raise_assertion_error('Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(str(expected_msgs), print_log))
16    ~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
17  File "C:\Users\hodlinator\bitcoin\build\test\functional\test_framework\test_node.py", line 198, in _raise_assertion_error
18    raise AssertionError(self._node_msg(msg))
19AssertionError: [node 0] Expected messages "['Error scanning']" does not partially match log:
  30. maflcko commented at 7:23 am on February 11, 2025: member

    re-ACK 9025657baaf99fcf630cc1a37baec11b072196fa only change is adding a warning log for the skipped test 🥋

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: re-ACK 9025657baaf99fcf630cc1a37baec11b072196fa only change is adding a warning log for the skipped test 🥋
38JwK2lTF2PaJReWtjE7KYXRP5D3CeQdUUGaK/bgJ/HSQX1wVVCUIdRe+nyLSRUJ7rivC1UC0wOT6Hh8/2Y3tDw==
  31. hodlinator commented at 7:52 pm on February 11, 2025: contributor

    Just reproduced the same failure using binaries produced by CI cross-compilation setup from #31176 as I had before using the Guix cross-compiled build. Good to see that the two have the same behavior… but worrying that nobody else is getting this.

     0$ BITCOINCLI=/c/Users/hodlinator/Downloads/18274b6-bins/bitcoin-cli.exe BITCOIND=/c/Users/hodlinator/Downloads/18274b6-bins/bitcoind.exe build/test/functional/wallet_multiwallet.py
 12025-02-11T19:42:46.959000Z TestFramework (INFO): PRNG seed is: 4624503638077972153
 22025-02-11T19:42:47.013000Z TestFramework (INFO): Initializing test directory C:\Users\HODLIN~1\AppData\Local\Temp\bitcoin_func_test_14_2asnv
 32025-02-11T19:42:48.521000Z TestFramework (ERROR): Assertion failed
 4Traceback (most recent call last):
 5  File "C:\Users\hodlinator\bitcoin\build\test\functional\test_framework\test_framework.py", line 135, in main
 6    self.run_test()
 7    ~~~~~~~~~~~~~^^
 8  File "C:\Users\hodlinator\bitcoin\build\test\functional\wallet_multiwallet.py", line 133, in run_test
 9    assert_equal(sorted(map(lambda w: w['name'], walletlist)), sorted(in_wallet_dir))
10    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11  File "C:\Users\hodlinator\bitcoin\build\test\functional\test_framework\util.py", line 77, in assert_equal
12    raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
13AssertionError: not(['default_wallet', 'recursive_dir_symlink\\wallets\\default_wallet', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\default_wallet', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\default_wallet', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\default_wallet', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\sub\\w5', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w1', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w2', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w3', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7_symlink', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\sub\\w5', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w1', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w2', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w3', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7_symlink', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\sub\\w5', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w1', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w2', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w3', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7_symlink', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\sub\\w5', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w1', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w2', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w3', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7', 'recursive_dir_symlink\\wallets\\recursive_dir_symlink\\wallets\\w7_symlink', 'recursive_dir_symlink\\wallets\\sub\\w5', 'recursive_dir_symlink\\wallets\\w', 'recursive_dir_symlink\\wallets\\w1', 'recursive_dir_symlink\\wallets\\w2', 'recursive_dir_symlink\\wallets\\w3', 'recursive_dir_symlink\\wallets\\w7', 'recursive_dir_symlink\\wallets\\w7_symlink', 'sub\\w5', 'w', 'w1', 'w2', 'w3', 'w7', 'w7_symlink'] == ['default_wallet', 'sub\\w5', 'w', 'w1', 'w2', 'w3', 'w7', 'w7_symlink'])
142025-02-11T19:42:48.573000Z TestFramework (INFO): Not stopping nodes as test failed. The dangling processes will be cleaned up later.
  32. glozow requested review from achow101 on Feb 13, 2025
  33. glozow assigned achow101 on Feb 13, 2025
  34. achow101 commented at 9:03 pm on February 13, 2025: member
    I am able to replicate @hodlinator’s errors with cross built binaries.
  35. hebasto commented at 9:14 pm on February 13, 2025: member

    Is this failure introduced by this PR, or does it occur on the master branch as well?

    I think the way cross-built binaries fail was new thanks to your introduced “0. Baseline, no errors."-test.

    Right.

    Even with an additional commit that fixes another check in this test, some of the subsequent checks reveal their flaws. Fixing these issues will require removing non-portable behaviour in the wallet code. I’m testing a branch with additional fixes.

    In the meantime, converting this PR to a draft.

  36. hebasto marked this as a draft on Feb 13, 2025
  37. hebasto removed this from the milestone 29.0 on Feb 13, 2025
  38. fanquake referenced this in commit 3358b1d105 on Mar 31, 2025
  39. maflcko commented at 10:57 am on April 1, 2025: member
    Needs rebase to remove the TODO?
  40. qa: Test scanning errors individually 
    This change ensures that each condition potentially triggering the
"Error scanning" log message is tested independently, avoiding false
positives.

Additionally, the "Permission denied" error test is now performed
conditionally, skipping scenarios where it is not applicable (e.g., when
running as root or on Windows).
    c748305ac6
  41. qa: Disable self-symlink test on Windows and document it 5fca92d175
  42. hebasto force-pushed on Apr 2, 2025
  43. ci: Enable `wallet_multiwallet.py` in "Windows, test cross-built" job 80034e76c8


hebasto DrahtBot maflcko Countrymom71 stickies-v fanquake hodlinator achow101


achow101 stickies-v

Labels
Wallet Tests

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-04-27 18:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me