Consolidate redundant unit and e2e tests (cbor, http, logger, receipts)

CMakeLists.txt

@@ -637,11 +637,6 @@ if(BUILD_TESTS)
     )
     target_link_libraries(openapi_test PRIVATE http_parser)
 
-    add_unit_test(
-      logger_json_test
-      ${CMAKE_CURRENT_SOURCE_DIR}/src/ds/test/logger_json_test.cpp
-    )
-
     add_unit_test(
       kv_test
       ${CMAKE_CURRENT_SOURCE_DIR}/src/kv/test/kv_test.cpp

src/crypto/test/cbor.cpp

@@ -5,6 +5,7 @@
 
 #include "ccf/ds/hex.h"
 
+#include <algorithm>
 #include <cstdint>
 #include <doctest/doctest.h>
 #include <iostream>
@@ -1646,28 +1647,41 @@ TEST_CASE("CBOR: tagged array Tag(20000, [{'x': 1}, {'y': 2}])")
   REQUIRE(result == expected_repr);
 }
 
-TEST_CASE("CBOR: helper function make_signed")
+// See
+// https://github.com/doctest/doctest/blob/master/doc/markdown/parameterized-tests.md
+#define DOCTEST_VALUE_PARAMETERIZED_DATA(data, data_container) \
+  static size_t _doctest_subcase_idx = 0; \
+  std::for_each( \
+    data_container.begin(), data_container.end(), [&](const auto& in) { \
+      DOCTEST_SUBCASE((std::string(#data_container "[") + \
+                       std::to_string(_doctest_subcase_idx++) + "]") \
+                        .c_str()) \
+      { \
+        data = in; \
+      } \
+    }); \
+  _doctest_subcase_idx = 0
+
+TEST_CASE("CBOR: helper function make_signed with positive and negative values")
 {
-  auto value = make_signed(42);
-  REQUIRE(value != nullptr);
-  REQUIRE(value->as_signed() == 42);
+  std::vector<std::pair<int64_t, std::string>> signed_cases{
+    {42, "Signed: 42"}, {-42, "Signed: -42"}};
 
-  const std::string expected_repr = "Signed: 42";
-  const std::string result = to_string(value);
-  REQUIRE(result == expected_repr);
-}
+  std::pair<int64_t, std::string> test_case;
+  DOCTEST_VALUE_PARAMETERIZED_DATA(test_case, signed_cases);
 
-TEST_CASE("CBOR: helper function make_signed")
-{
-  auto value = make_signed(-42);
+  const auto& [input_value, expected_repr] = test_case;
+
+  auto value = make_signed(input_value);
   REQUIRE(value != nullptr);
-  REQUIRE(value->as_signed() == -42);
+  REQUIRE(value->as_signed() == input_value);
 
-  const std::string expected_repr = "Signed: -42";
   const std::string result = to_string(value);
   REQUIRE(result == expected_repr);
 }
 
+#undef DOCTEST_VALUE_PARAMETERIZED_DATA
+
 TEST_CASE("CBOR: helper function make_string")
 {
   auto value = make_string("hello");

src/ds/test/logger.cpp

@@ -5,6 +5,10 @@
 
 #define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
 #include <doctest/doctest.h>
+#include <filesystem>
+#include <fstream>
+#include <nlohmann/json.hpp>
+#include <unistd.h>
 
 template <typename Base>
 class TestLogger : public Base
@@ -178,4 +182,77 @@ TEST_CASE("Custom logging macros")
   }
 
   ccf::logger::config::loggers().clear();
-}
+}
+
+TEST_CASE("Test custom log format")
+{
+  auto test_log_file =
+    (std::filesystem::temp_directory_path() /
+     ("test_json_logger_" + std::to_string(::getpid()) + ".txt"))
+      .string();
+  std::error_code ec;
+  std::filesystem::remove(test_log_file, ec);
+
+  struct LoggerConfigGuard
+  {
+    ccf::LoggerLevel old_level = ccf::logger::config::level();
+    ~LoggerConfigGuard()
+    {
+      ccf::logger::config::loggers().clear();
+      ccf::logger::config::level() = old_level;
+    }
+  };
+  LoggerConfigGuard logger_config_guard;
+
+  // Start from a clean logger set so this test does not depend on loggers
+  // registered by earlier test cases sharing this binary.
+  ccf::logger::config::loggers().clear();
+  ccf::logger::config::add_json_console_logger();
+  ccf::logger::config::level() = ccf::LoggerLevel::DEBUG;
+  std::string log_msg_debug = "log_msg_debug";
+  std::string log_msg_trace = "log_msg_trace";
+
+  struct CoutRdbufGuard
+  {
+    std::streambuf* old_buf = nullptr;
+    explicit CoutRdbufGuard(std::streambuf* new_buf) :
+      old_buf(std::cout.rdbuf(new_buf))
+    {}
+    ~CoutRdbufGuard()
+    {
+      std::cout.rdbuf(old_buf);
+    }
+  };
+
+  {
+    std::ofstream out(test_log_file);
+    REQUIRE(out.is_open());
+    CoutRdbufGuard cout_guard(out.rdbuf());
+
+    LOG_DEBUG_FMT("{}", log_msg_debug);
+    LOG_TRACE_FMT("{}", log_msg_trace);
+    LOG_DEBUG_FMT("{}", log_msg_debug);
+    LOG_TRACE_FMT("{}", log_msg_trace);
+    LOG_DEBUG_FMT("{}", log_msg_debug);
+
+    std::cout.flush();
+  }
+  std::ifstream f(test_log_file);
+  std::string line;
+  size_t line_count = 0;
+  while (std::getline(f, line))
+  {
+    line_count++;
+    auto j = nlohmann::json::parse(line);
+    auto host_ts = j.find("h_ts");
+    REQUIRE(host_ts != j.end());
+    REQUIRE(j["msg"] == log_msg_debug);
+    REQUIRE(j["file"] == __FILE__);
+    auto line_number = j.find("number");
+    REQUIRE(line_number != j.end());
+    REQUIRE(j["level"] == "debug");
+  }
+  f.close();
+  std::filesystem::remove(test_log_file, ec);
+  REQUIRE(line_count == 3);
+}

src/http/test/http_test.cpp

@@ -703,136 +703,68 @@ DOCTEST_TEST_CASE("Query parser getters")
   }
 }
 
-DOCTEST_TEST_CASE("parse_want_repr_digest - single supported algorithm")
+DOCTEST_TEST_CASE("parse_want_repr_digest")
 {
-  {
-    auto [algo, md] = ccf::http::parse_want_repr_digest("sha-256=1");
-    DOCTEST_CHECK(algo == "sha-256");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA256);
-  }
-
-  {
-    auto [algo, md] = ccf::http::parse_want_repr_digest("sha-384=5");
-    DOCTEST_CHECK(algo == "sha-384");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA384);
-  }
-
-  {
-    auto [algo, md] = ccf::http::parse_want_repr_digest("sha-512=10");
-    DOCTEST_CHECK(algo == "sha-512");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA512);
-  }
-}
-
-DOCTEST_TEST_CASE(
-  "parse_want_repr_digest - multiple algorithms with priorities")
-{
-  {
-    auto [algo, md] =
-      ccf::http::parse_want_repr_digest("sha-256=1, sha-512=10");
-    DOCTEST_CHECK(algo == "sha-512");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA512);
-  }
+  auto check = [](
+                 const std::string& header,
+                 const std::string& expected_algo,
+                 ccf::crypto::MDType expected_md) {
+    auto [algo, md] = ccf::http::parse_want_repr_digest(header);
+    DOCTEST_CHECK(algo == expected_algo);
+    DOCTEST_CHECK(md == expected_md);
+  };
 
+  DOCTEST_SUBCASE("single supported algorithm")
   {
-    auto [algo, md] =
-      ccf::http::parse_want_repr_digest("sha-512=3, sha-256=7, sha-384=5");
-    DOCTEST_CHECK(algo == "sha-256");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA256);
+    check("sha-256=1", "sha-256", ccf::crypto::MDType::SHA256);
+    check("sha-384=5", "sha-384", ccf::crypto::MDType::SHA384);
+    check("sha-512=10", "sha-512", ccf::crypto::MDType::SHA512);
   }
 
+  DOCTEST_SUBCASE("multiple algorithms with priorities")
   {
-    auto [algo, md] =
-      ccf::http::parse_want_repr_digest("sha-384=10, sha-256=10");
+    check("sha-256=1, sha-512=10", "sha-512", ccf::crypto::MDType::SHA512);
+    check(
+      "sha-512=3, sha-256=7, sha-384=5",
+      "sha-256",
+      ccf::crypto::MDType::SHA256);
     // Equal preference - first one wins
-    DOCTEST_CHECK(algo == "sha-384");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA384);
-  }
-}
-
-DOCTEST_TEST_CASE("parse_want_repr_digest - unknown algorithms are ignored")
-{
-  {
-    auto [algo, md] = ccf::http::parse_want_repr_digest("md5=10, sha-256=1");
-    DOCTEST_CHECK(algo == "sha-256");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA256);
+    check("sha-384=10, sha-256=10", "sha-384", ccf::crypto::MDType::SHA384);
   }
 
+  DOCTEST_SUBCASE("unknown algorithms are ignored")
   {
-    auto [algo, md] =
-      ccf::http::parse_want_repr_digest("crc32=5, sha-384=3, unknown=10");
-    DOCTEST_CHECK(algo == "sha-384");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA384);
+    check("md5=10, sha-256=1", "sha-256", ccf::crypto::MDType::SHA256);
+    check(
+      "crc32=5, sha-384=3, unknown=10", "sha-384", ccf::crypto::MDType::SHA384);
   }
-}
 
-DOCTEST_TEST_CASE("parse_want_repr_digest - defaults to sha-256 when no match")
-{
+  DOCTEST_SUBCASE("defaults to sha-256 when no match")
   {
-    auto [algo, md] = ccf::http::parse_want_repr_digest("md5=10");
-    DOCTEST_CHECK(algo == "sha-256");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA256);
+    check("md5=10", "sha-256", ccf::crypto::MDType::SHA256);
+    check("unknown=5", "sha-256", ccf::crypto::MDType::SHA256);
+    check("", "sha-256", ccf::crypto::MDType::SHA256);
   }
 
-  {
-    auto [algo, md] = ccf::http::parse_want_repr_digest("unknown=5");
-    DOCTEST_CHECK(algo == "sha-256");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA256);
-  }
-
-  {
-    auto [algo, md] = ccf::http::parse_want_repr_digest("");
-    DOCTEST_CHECK(algo == "sha-256");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA256);
-  }
-}
-
-DOCTEST_TEST_CASE("parse_want_repr_digest - malformed entries are skipped")
-{
+  DOCTEST_SUBCASE("malformed entries are skipped")
   {
     // Preference of 0 is invalid (must be >= 1)
-    auto [algo, md] = ccf::http::parse_want_repr_digest("sha-256=0");
-    DOCTEST_CHECK(algo == "sha-256");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA256);
-  }
-
-  {
+    check("sha-256=0", "sha-256", ccf::crypto::MDType::SHA256);
     // Negative preference is invalid
-    auto [algo, md] = ccf::http::parse_want_repr_digest("sha-512=-1");
-    DOCTEST_CHECK(algo == "sha-256");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA256);
-  }
-
-  {
+    check("sha-512=-1", "sha-256", ccf::crypto::MDType::SHA256);
     // Non-numeric preference is skipped, but valid entry is used
-    auto [algo, md] =
-      ccf::http::parse_want_repr_digest("sha-256=abc, sha-384=5");
-    DOCTEST_CHECK(algo == "sha-384");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA384);
+    check("sha-256=abc, sha-384=5", "sha-384", ccf::crypto::MDType::SHA384);
   }
-}
 
-DOCTEST_TEST_CASE("parse_want_repr_digest - whitespace handling")
-{
+  DOCTEST_SUBCASE("whitespace handling")
   {
-    auto [algo, md] = ccf::http::parse_want_repr_digest("  sha-256 = 1  ");
-    DOCTEST_CHECK(algo == "sha-256");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA256);
+    check("  sha-256 = 1  ", "sha-256", ccf::crypto::MDType::SHA256);
+    check("sha-256=1 , sha-512=10", "sha-512", ccf::crypto::MDType::SHA512);
   }
 
+  DOCTEST_SUBCASE("algorithm without explicit preference")
   {
-    auto [algo, md] =
-      ccf::http::parse_want_repr_digest("sha-256=1 , sha-512=10");
-    DOCTEST_CHECK(algo == "sha-512");
-    DOCTEST_CHECK(md == ccf::crypto::MDType::SHA512);
+    // No "=" means preference defaults to 1
+    check("sha-512", "sha-512", ccf::crypto::MDType::SHA512);
   }
-}
-
-DOCTEST_TEST_CASE(
-  "parse_want_repr_digest - algorithm without explicit preference")
-{
-  // No "=" means preference defaults to 1
-  auto [algo, md] = ccf::http::parse_want_repr_digest("sha-512");
-  DOCTEST_CHECK(algo == "sha-512");
-  DOCTEST_CHECK(md == ccf::crypto::MDType::SHA512);
 }