test: Don't include torcontrol.cpp into the test file #13291

pull Empact wants to merge 1 commits into bitcoin:master from Empact:tor-reply changing 2 files +11 −4
  1. Empact commented at 6:10 AM on May 21, 2018: member

    These methods are standalone string parsing methods which were included into test via an include of torcontrol.cpp, which is bad practice.

    Splitting them out reveals that they were the only torcontrol.cpp methods under test, so the test file is renamed tor_reply_tests.cpp.

    Introduced in #10408

  2. Empact renamed this:
    [moveonly] Extract tor_reply.h from tor_control.cpp
    [moveonly] Extract tor_reply.h from torcontrol.cpp
    on May 21, 2018
  3. Empact force-pushed on May 21, 2018
  4. fanquake added the label Refactoring on May 21, 2018
  5. Empact force-pushed on May 21, 2018
  6. Empact force-pushed on May 21, 2018
  7. Empact force-pushed on May 21, 2018
  8. practicalswift commented at 7:19 AM on May 21, 2018: contributor

    Concept ACK

  9. jonasschnelli commented at 2:06 PM on May 21, 2018: contributor

    Wouldn't an extern definition of the function in test source not be sufficient and simpler?

  10. MarcoFalke commented at 3:46 PM on May 21, 2018: member

    Agree with @jonasschnelli. I think just declaring the function in the test file should be sufficient and does indeed compile:

    diff --git a/src/test/torcontrol_tests.cpp b/src/test/torcontrol_tests.cpp
    index 8bd5ce1222..9ea085eaa7 100644
    --- a/src/test/torcontrol_tests.cpp
    +++ b/src/test/torcontrol_tests.cpp
    @@ -3,11 +3,15 @@
     // file COPYING or http://www.opensource.org/licenses/mit-license.php.
     //
     #include <test/test_bitcoin.h>
    -#include <torcontrol.cpp>
    +#include <torcontrol.h>
     
     #include <boost/test/unit_test.hpp>
     
     
    +std::pair<std::string, std::string> SplitTorReplyLine(const std::string& s);
    +std::map<std::string, std::string> ParseTorReplyMapping(const std::string& s);
    +
    +
     BOOST_FIXTURE_TEST_SUITE(torcontrol_tests, BasicTestingSetup)
     
     static void CheckSplitTorReplyLine(std::string input, std::string command, std::string args)
    diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
    index 717d1cf7e5..a4952a3336 100644
    --- a/src/torcontrol.cpp
    +++ b/src/torcontrol.cpp
    @@ -251,7 +251,7 @@ bool TorControlConnection::Command(const std::string &cmd, const ReplyHandlerCB&
      * Grammar is implicitly defined in https://spec.torproject.org/control-spec by
      * the server reply formats for PROTOCOLINFO (S3.21) and AUTHCHALLENGE (S3.24).
      */
    -static std::pair<std::string,std::string> SplitTorReplyLine(const std::string &s)
    +std::pair<std::string, std::string> SplitTorReplyLine(const std::string& s)
     {
         size_t ptr=0;
         std::string type;
    @@ -270,7 +270,7 @@ static std::pair<std::string,std::string> SplitTorReplyLine(const std::string &s
      * the server reply formats for PROTOCOLINFO (S3.21), AUTHCHALLENGE (S3.24),
      * and ADD_ONION (S3.27). See also sections 2.1 and 2.3.
      */
    -static std::map<std::string,std::string> ParseTorReplyMapping(const std::string &s)
    +std::map<std::string, std::string> ParseTorReplyMapping(const std::string& s)
     {
         std::map<std::string,std::string> mapping;
         size_t ptr=0;
    
  11. Declare TorReply parsing functions in torcontrol_tests
    Rather than including the implementation file into the test,
    which is bad practice.
    97c112d4ca
  12. Empact force-pushed on May 21, 2018
  13. Empact commented at 5:56 PM on May 21, 2018: member

    SGTM, updated.

  14. Empact renamed this:
    [moveonly] Extract tor_reply.h from torcontrol.cpp
    refactoring: Don't include torcontrol.cpp into the test file
    on May 21, 2018
  15. MarcoFalke added the label Tests on May 21, 2018
  16. MarcoFalke commented at 9:36 PM on May 21, 2018: member

    utaCK 97c112d4ca42caf0668af2b8e71a46de72b23def

  17. practicalswift commented at 9:43 PM on May 21, 2018: contributor

    utACK 97c112d4ca42caf0668af2b8e71a46de72b23def

  18. kallewoof approved
  19. kallewoof commented at 3:03 AM on May 22, 2018: member

    utACK 97c112d4ca42caf0668af2b8e71a46de72b23def

  20. MarcoFalke renamed this:
    refactoring: Don't include torcontrol.cpp into the test file
    test: Don't include torcontrol.cpp into the test file
    on May 24, 2018
  21. MarcoFalke merged this on May 24, 2018
  22. MarcoFalke closed this on May 24, 2018

  23. MarcoFalke referenced this in commit 536120ec39 on May 24, 2018
  24. Empact deleted the branch on Jul 2, 2018
  25. jasonbcox referenced this in commit be756d0c7f on Jun 28, 2019
  26. jtoomim referenced this in commit 5f44f14f0b on Jun 29, 2019
  27. jonspock referenced this in commit e567c64a94 on Jul 6, 2019
  28. jonspock referenced this in commit 85c483cec9 on Jul 7, 2019
  29. jonspock referenced this in commit 4f51d749b6 on Jul 7, 2019
  30. proteanx referenced this in commit 75f2a3fca4 on Jul 7, 2019
  31. jonspock referenced this in commit d0f7c152c3 on Jul 9, 2019
  32. PastaPastaPasta referenced this in commit 92aeba497d on Jun 17, 2020
  33. PastaPastaPasta referenced this in commit baede72c4e on Jun 27, 2020
  34. PastaPastaPasta referenced this in commit eecc792fb9 on Jun 28, 2020
  35. PastaPastaPasta referenced this in commit 443ceee1e5 on Jun 29, 2020
  36. PastaPastaPasta referenced this in commit 9b8f67eb04 on Jul 1, 2020
  37. MarcoFalke locked this on Sep 8, 2021

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: 2026-04-13 15:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me