test: Fix test by checking the actual exception instance #28989

1. 
   hebasto commented at 9:57 pm on December 2, 2023: member

   The system_tests/run_command test is broken because it passes even with the diff as follows:

    0--- a/src/test/system_tests.cpp
    1+++ b/src/test/system_tests.cpp
    2@@ -90,7 +90,7 @@ BOOST_AUTO_TEST_CASE(run_command)
    3         });
    4     }
    5     {
    6-        BOOST_REQUIRE_THROW(RunCommandParseJSON("echo \"{\""), std::runtime_error); // Unable to parse JSON
    7+        BOOST_REQUIRE_THROW(RunCommandParseJSON("invalid_command \"{\""), std::runtime_error); // Unable to parse JSON
    8     }
    9     // Test std::in, except for Windows
   10 #ifndef WIN32
   

   The reason of such fragility is that the BOOST_REQUIRE_THROW macro passes even if the command raises an exception in the underlying subprocess implementation, which might have a type derived from std::runtime_error.

2. 
   DrahtBot commented at 9:57 pm on December 2, 2023: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	furszy, maflcko, pablomartin4btc, achow101

   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.

3. DrahtBot added the label Tests on Dec 2, 2023
4. hebasto removed the label Tests on Dec 2, 2023
5. DrahtBot added the label Tests on Dec 2, 2023
6. in src/test/system_tests.cpp:103 in 24d978a36d outdated

    99+#else
   100+        const std::string command{"echo {"};
   101+#endif
   102+        BOOST_CHECK_EXCEPTION(RunCommandParseJSON(command), std::runtime_error, [&](const auto& e) {
   103+            return std::strstr(e.what(), "Unable to parse JSON: {") != nullptr;
   104+        });
   

   ---

   ---

   

   furszy commented at 3:38 pm on December 3, 2023:

   Could use HasReason().

   0BOOST_CHECK_EXCEPTION(RunCommandParseJSON(command), std::runtime_error, HasReason("Unable to parse JSON"));
   

   ---

   

   hebasto commented at 4:08 pm on December 3, 2023:

   Thanks! Updated.
7. test: Fix test by checking the actual exception instance

   The BOOST_REQUIRE_THROW passes even if the command raises an exception
   in the underlying subprocess implementation, which might have a type
   derived from std::runtime_error.

   55e3dc3e03
8. hebasto force-pushed on Dec 3, 2023
9. 
   hebasto commented at 4:08 pm on December 3, 2023: member

   Updated 24d978a36d1b405f968161a9d046beffffc0b249 -> 55e3dc3e03510e97caba1547a82e3e022b0bbd42 (pr28989.01 -> pr28989.02, diff):

   • addressed @furszy’s comment
10. 
    furszy commented at 4:12 pm on December 3, 2023: member

    Non-Windows code ACK 55e3dc3e
11. 
    maflcko commented at 7:29 am on December 4, 2023: member

    lgtm ACK 55e3dc3e03510e97caba1547a82e3e022b0bbd42
12. 
    pablomartin4btc commented at 0:05 am on December 6, 2023: member

    ACK 55e3dc3e03510e97caba1547a82e3e022b0bbd42
13. maflcko assigned achow101 on Dec 6, 2023
14. maflcko assigned ryanofsky on Dec 6, 2023
15. 
    hebasto commented at 11:02 am on December 6, 2023: member

    Friendly ping @Sjors :)
16. 
    achow101 commented at 3:27 pm on December 6, 2023: member

    ACK 55e3dc3e03510e97caba1547a82e3e022b0bbd42
17. achow101 merged this on Dec 6, 2023
18. achow101 closed this on Dec 6, 2023

    ---

19. hebasto deleted the branch on Dec 6, 2023
20. 
    Sjors commented at 6:39 pm on December 6, 2023: member

    Looks like a reasonable fix, thanks!
21. PastaPastaPasta referenced this in commit d25cdb6000 on Oct 24, 2024
22. PastaPastaPasta referenced this in commit 285c53633b on Oct 24, 2024
23. PastaPastaPasta referenced this in commit 75e0334866 on Oct 24, 2024
24. PastaPastaPasta referenced this in commit a67319351a on Oct 24, 2024
25. bitcoin locked this on Dec 5, 2024

Contributors
hebasto DrahtBot furszy maflcko pablomartin4btc achow101 Sjors

Labels
Tests