From b81a4659950a6c4e22316f66b55cae8afc4f4d9a Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Fri, 6 Dec 2024 21:45:18 +0100 Subject: [PATCH 1/4] refactor test: Profit from using namespace + using detail function Also adds BOOST_CHECK_NO_THROW() while touching that line, clarifying part of what we are checking for. Also removed redundant inline from template functions in .cpp file. --- src/test/util_string_tests.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp index fbab9e7abac..eaf179f6400 100644 --- a/src/test/util_string_tests.cpp +++ b/src/test/util_string_tests.cpp @@ -8,21 +8,22 @@ #include using namespace util; +using util::detail::CheckNumFormatSpecifiers; BOOST_AUTO_TEST_SUITE(util_string_tests) // Helper to allow compile-time sanity checks while providing the number of // args directly. Normally PassFmt would be used. template -inline void PassFmt(util::ConstevalFormatString fmt) +void PassFmt(ConstevalFormatString fmt) { // Execute compile-time check again at run-time to get code coverage stats - util::detail::CheckNumFormatSpecifiers(fmt.fmt); + BOOST_CHECK_NO_THROW(CheckNumFormatSpecifiers(fmt.fmt)); } template -inline void FailFmtWithError(const char* wrong_fmt, std::string_view error) +void FailFmtWithError(const char* wrong_fmt, std::string_view error) { - BOOST_CHECK_EXCEPTION(util::detail::CheckNumFormatSpecifiers(wrong_fmt), const char*, HasReason{error}); + BOOST_CHECK_EXCEPTION(CheckNumFormatSpecifiers(wrong_fmt), const char*, HasReason{error}); } BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec) From 533013cba20664e3581a8e4633cc188d5be3175a Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Fri, 6 Dec 2024 16:08:34 +0100 Subject: [PATCH 2/4] test: Prove+document ConstevalFormatString/tinyformat parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Lőrinc Co-Authored-By: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Co-Authored-By: Ryan Ofsky --- src/test/util_string_tests.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp index eaf179f6400..d90539af47a 100644 --- a/src/test/util_string_tests.cpp +++ b/src/test/util_string_tests.cpp @@ -12,6 +12,14 @@ using util::detail::CheckNumFormatSpecifiers; BOOST_AUTO_TEST_SUITE(util_string_tests) +template +void TfmFormatZeroes(const std::string& fmt) +{ + std::apply([&](auto... args) { + (void)tfm::format(fmt, args...); + }, std::array{}); +} + // Helper to allow compile-time sanity checks while providing the number of // args directly. Normally PassFmt would be used. template @@ -19,6 +27,12 @@ void PassFmt(ConstevalFormatString fmt) { // Execute compile-time check again at run-time to get code coverage stats BOOST_CHECK_NO_THROW(CheckNumFormatSpecifiers(fmt.fmt)); + + // If ConstevalFormatString didn't throw above, make sure tinyformat doesn't + // throw either for the same format string and parameter count combination. + // Proves that we have some extent of protection from runtime errors + // (tinyformat may still throw for some type mismatches). + BOOST_CHECK_NO_THROW(TfmFormatZeroes(fmt.fmt)); } template void FailFmtWithError(const char* wrong_fmt, std::string_view error) @@ -111,6 +125,15 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec) FailFmtWithError<2>("%1$*2$", err_term); FailFmtWithError<2>("%1$.*2$", err_term); FailFmtWithError<2>("%1$9.*2$", err_term); + + // Ensure that tinyformat throws if format string contains wrong number + // of specifiers. PassFmt relies on this to verify tinyformat successfully + // formats the strings, and will need to be updated if tinyformat is changed + // not to throw on failure. + BOOST_CHECK_EXCEPTION(TfmFormatZeroes<2>("%s"), tfm::format_error, + HasReason{"tinyformat: Not enough conversion specifiers in format string"}); + BOOST_CHECK_EXCEPTION(TfmFormatZeroes<1>("%s %s"), tfm::format_error, + HasReason{"tinyformat: Too many conversion specifiers in format string"}); } BOOST_AUTO_TEST_SUITE_END() From 76cca4aa6fcd363dee821c837b1b627ea137b7a4 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Fri, 6 Dec 2024 21:56:16 +0100 Subject: [PATCH 3/4] test: Document non-parity between tinyformat and ConstevalFormatstring - For "%n", which is supposed to write to the argument for printf. - For string/integer mismatches of width/precision specifiers. Co-Authored-By: Ryan Ofsky --- src/test/util_string_tests.cpp | 9 +++++++++ test/lint/run-lint-format-strings.py | 2 ++ 2 files changed, 11 insertions(+) diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp index d90539af47a..e40bef63790 100644 --- a/src/test/util_string_tests.cpp +++ b/src/test/util_string_tests.cpp @@ -126,6 +126,15 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec) FailFmtWithError<2>("%1$.*2$", err_term); FailFmtWithError<2>("%1$9.*2$", err_term); + // Non-parity between tinyformat and ConstevalFormatString. + // tinyformat throws but ConstevalFormatString does not. + BOOST_CHECK_EXCEPTION(tfm::format(ConstevalFormatString<1>{"%n"}, 0), tfm::format_error, + HasReason{"tinyformat: %n conversion spec not supported"}); + BOOST_CHECK_EXCEPTION(tfm::format(ConstevalFormatString<2>{"%*s"}, "hi", "hi"), tfm::format_error, + HasReason{"tinyformat: Cannot convert from argument type to integer for use as variable width or precision"}); + BOOST_CHECK_EXCEPTION(tfm::format(ConstevalFormatString<2>{"%.*s"}, "hi", "hi"), tfm::format_error, + HasReason{"tinyformat: Cannot convert from argument type to integer for use as variable width or precision"}); + // Ensure that tinyformat throws if format string contains wrong number // of specifiers. PassFmt relies on this to verify tinyformat successfully // formats the strings, and will need to be updated if tinyformat is changed diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py index 4402ec2d57f..0e08c9f974e 100755 --- a/test/lint/run-lint-format-strings.py +++ b/test/lint/run-lint-format-strings.py @@ -15,6 +15,8 @@ import sys FALSE_POSITIVES = [ ("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS), COPYRIGHT_HOLDERS_SUBSTITUTION)"), ("src/test/translation_tests.cpp", "strprintf(format, arg)"), + ("src/test/util_string_tests.cpp", 'tfm::format(ConstevalFormatString<2>{"%*s"}, "hi", "hi")'), + ("src/test/util_string_tests.cpp", 'tfm::format(ConstevalFormatString<2>{"%.*s"}, "hi", "hi")'), ] From c93bf0e6e2cd0e9dc979ed996e41cc1034bc1ad1 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Thu, 5 Dec 2024 14:50:06 +0100 Subject: [PATCH 4/4] test: Add missing %c character test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Proves tinyformat doesn't trigger an exception for \0 characters. Co-Authored-By: Lőrinc --- src/test/util_string_tests.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp index e40bef63790..340c650fe36 100644 --- a/src/test/util_string_tests.cpp +++ b/src/test/util_string_tests.cpp @@ -45,6 +45,7 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec) PassFmt<0>(""); PassFmt<0>("%%"); PassFmt<1>("%s"); + PassFmt<1>("%c"); PassFmt<0>("%%s"); PassFmt<0>("s%%"); PassFmt<1>("%%%s");