test: Run non-wallet tests only once #24635

1. 
   MarcoFalke commented at 7:03 am on March 22, 2022: member

   I don’t see why non-wallet tests should run for two wallet configs, even though they never use a wallet.
2. test: Run non-wallet tests only once fa7a576391
3. in test/functional/test_runner.py:311 in fa50577a84 outdated

   306@@ -309,8 +307,7 @@
   307     'feature_txindex_compatibility.py',
   308     'feature_logging.py',
   309     'feature_anchors.py',
   310-    'feature_coinstatsindex.py --legacy-wallet',
   311-    'feature_coinstatsindex.py --descriptors',
   312+    'feature_coinstatsindex.py',
   

   ---

   ---

   

   MarcoFalke commented at 7:07 am on March 22, 2022:

   This reverts commit 61fb410c0d9bc9e5fe4e3c52b4c519d49faf15f6 as a follow-up to commit e3206c9445f8d8e2bdb36d98af609985e8809908
4. in test/functional/test_runner.py:191 in fa50577a84 outdated

   186@@ -188,8 +187,7 @@
   187     'rpc_decodescript.py',
   188     'rpc_blockchain.py',
   189     'rpc_deprecated.py',
   190-    'wallet_disable.py --legacy-wallet',
   191-    'wallet_disable.py --descriptors',
   192+    'wallet_disable.py',
   

   ---

   ---

   

   MarcoFalke commented at 7:07 am on March 22, 2022:

   This partially reverts commit 242aed7cc1d003e8fed574bbebd19c7e54e23402
5. MarcoFalke force-pushed on Mar 22, 2022
6. 
   ayush933 commented at 7:59 am on March 22, 2022: contributor

   Maybe this would also apply to rpc_createmultisig.py if #24587 is merged?
7. 
   MarcoFalke commented at 8:07 am on March 22, 2022: member

   Good point. I think you can just do it right there in that pull request.
8. DrahtBot added the label Tests on Mar 22, 2022
9. 
   michaelfolkson commented at 11:52 am on March 22, 2022: contributor

   Concept ACK

   Doesn’t make sense for wallet_disable.py to be run twice with both --legacy-wallet and --descriptors given the wallet is disabled for this test.

   The wallet isn’t necessarily disabled for feature_coinstatsindex.py? I’m wondering whether that should be included in this PR…

   edit: Looking through the test_runner.py it appears there are a number of non-wallet tests that are being run twice (if the criteria is that it isn’t testing the Core wallet rather than the Core wallet is explicitly disabled like in wallet_disable.py)

10. 
    michaelfolkson commented at 12:11 pm on March 22, 2022: contributor

    Maybe this would also apply to rpc_createmultisig.py if #24587 is merged? @ayush933: I wouldn’t make any changes to your PR yet until this PR is merged. It is possible your PR could be merged first (hard to know at this point). But yeah if this PR is merged first you can edit/update your PR.

11. 
    MarcoFalke commented at 12:33 pm on March 22, 2022: member

    Starting with commit https://github.com/bitcoin/bitcoin/commit/e3206c9445f8d8e2bdb36d98af609985e8809908 the test doesn’t use any Bitcoin Core wallet at all, even if one is compiled. So telling the test to use one or the other doesn’t make sense.
12. 
    michaelfolkson commented at 9:33 am on March 23, 2022: contributor

    edit: Looking through the test_runner.py it appears there are a number of non-wallet tests that are being run twice (if the criteria is that it isn’t testing the Core wallet rather than the Core wallet is explicitly disabled like in wallet_disable.py)

    So I think this PR (or a follow up PR) should ensure all the non-wallet tests (files beginning with wallet_) should run only once right? There’s no particular reason that feature_coinstatsindex.py is included in this PR but say rpc_signrawtransaction.py (or indeed rpc_createmultisig.py) isn’t?

    I guess individuals could open PRs for each non-wallet test that not only uses MiniWallet but also cleans up the --legacy-wallet and --descriptors double tests.

13. 
    MarcoFalke commented at 9:37 am on March 23, 2022: member

    I have no opinion about tests that are not wallet specific, but happen to use the wallet. Maybe this can be discussed/done in a follow-up?

    The goal of this pull request is to run tests that don’t use the wallet at all to run only once.

14. 
    fanquake commented at 2:06 pm on March 23, 2022: member

    cc @achow101
15. 
    achow101 commented at 5:20 pm on March 23, 2022: member

    ACK fa7a576391ad5d61af937dd62496ded44105c671
16. fanquake merged this on Mar 23, 2022
17. fanquake closed this on Mar 23, 2022

    ---

18. sidhujag referenced this in commit d44cbb3f5f on Mar 24, 2022
19. MarcoFalke deleted the branch on Mar 24, 2022
20. DrahtBot locked this on Mar 24, 2023

Contributors
MarcoFalke ayush933 michaelfolkson fanquake achow101

Labels
Tests