qt test: Don’t bind to regtest port #233

1. 
   achow101 commented at 9:25 pm on March 1, 2021: member

   The qt tests don’t need to bind to the regtest port. By not binding, it will no longer conflict with existing regtest instances and the tests will run as normal.

   Fixes #10

2. 
   jarolrod commented at 3:25 am on March 2, 2021: member

   ACK a9d1ddd91dc20332033d09869afee76ff6f281b8

   Confirming the behavior cited in #10 on master. Also confirming that this PR fixes it.

   Tested on macOS 11.2 Qt 5.15.2 using the QT_QPA_PLATFORM=cocoa ./test_bitcoin-qt work around.

   I setup a node running bitcoin-qt -regtest, then setup two instances of the test to run at the same time. Everything ran smoothly and the blocking behavior was not encountered.

3. in src/qt/test/test_main.cpp:57 in a9d1ddd91d outdated

   53@@ -54,6 +54,7 @@ int main(int argc, char* argv[])
   54 
   55     NodeContext node_context;
   56     std::unique_ptr<interfaces::Node> node = interfaces::MakeNode(&node_context);
   57+    gArgs.ForceSetArg("-listen", "0");
   

   ---

   ---

   

   MarcoFalke commented at 10:02 am on March 2, 2021:

   Same for listenonion?

       f.write("listenonion=0\n")
       f.write("discover=0\n")
       f.write("dnsseed=0\n")
       f.write("fixedseeds=0\n")
       f.write("upnp=0\n")
       f.write("natpmp=0\n")
   

   ---

   

   achow101 commented at 8:19 pm on March 6, 2021:

   Added those overrides as well.
4. hebasto added the label Tests on Mar 6, 2021
5. 
   hebasto commented at 7:17 pm on March 6, 2021: member

   I think this change prevents an opportunity to test Qt signals and UI response when new block/tx arrives.

   (that, of course, will require to spin up another node)

6. qt test: Don't bind to regtest port

   The qt tests don't need to bind to the regtest port. By not binding, it
   will no longer conflict with existing regtest instances and the tests
   will run as normal.

   e21276a82a
7. achow101 force-pushed on Mar 6, 2021
8. 
   achow101 commented at 8:21 pm on March 6, 2021: member

   I think this change prevents an opportunity to test Qt signals and UI response when new block/tx arrives.

   (that, of course, will require to spin up another node)

   In that case, -port can be overridden. In any case, I don’t think that we should reject this just because there might be a change in the future that conflicts with it.

9. 
   MarcoFalke commented at 7:56 am on March 7, 2021: contributor

   cr ACK e21276a82a9996c73e43990ccf927397f71399ea

   didn’t test

10. in src/qt/test/test_main.cpp:57 in e21276a82a

    53@@ -54,6 +54,13 @@ int main(int argc, char* argv[])
    54 
    55     NodeContext node_context;
    56     std::unique_ptr<interfaces::Node> node = interfaces::MakeNode(&node_context);
    57+    gArgs.ForceSetArg("-listen", "0");
    

    ---

    ---

    

    jarolrod commented at 4:54 pm on March 8, 2021:

    Passing 0 to port lets the system assign a free port number. This fixes the issue while allowing for future GUI tests that involve interactions with another node.

    0    gArgs.ForceSetArg("-port", "0");
    

    ---

    

    achow101 commented at 5:15 pm on March 8, 2021:

    I don’t think that is necessary at this time. Additionally, I don’t think doing this would be useful. In this hypothetical test, the test framework needs to know what port is being used, so I don’t think this would help.

    ---

    

    jarolrod commented at 5:48 pm on March 8, 2021:

    true
11. 
    jarolrod commented at 6:20 pm on March 8, 2021: member

    re-ACK e21276a82a9996c73e43990ccf927397f71399ea, tested on macOS 11.2
12. MarcoFalke merged this on Mar 8, 2021
13. MarcoFalke closed this on Mar 8, 2021

    ---

14. sidhujag referenced this in commit 4a97844b20 on Mar 8, 2021
15. gwillen referenced this in commit 4e32743e0e on Jun 28, 2022
16. bitcoin-core locked this on Aug 16, 2022

Contributors
achow101 jarolrod MarcoFalke hebasto

Labels
Tests