Merge branch 'test_impls'

* test_impls:
  Add scoped_buffer_capture as a submodule.
  Limit coverage reporting.
  IsValid check was dumb.
  Add tests for file and console appenders, fixing two errors identified in FileAppenderTest. - Fix null property testing in operator[]. - Fix merging properties.
  Add test that shows that pattern_layout calls into logging::format.

.gitmodules

@@ -7,3 +7,6 @@
 [submodule "extern/variant"]
 	path = extern/variant
 	url = freenas:utility/variant.git
+[submodule "extern/scoped_buffer_capture"]
+	path = extern/scoped_buffer_capture
+	url = freenas:utility/scoped_buffer_capture

extern/scoped_buffer_capture

@@ -0,0 +1 @@
+Subproject commit 577f821510f014127ed2a58e6bed0fe14b49f6c0

logger.xcodeproj/project.pbxproj

@@ -23,6 +23,9 @@
 		CD6F7406225187F40081ED74 /* liblogging.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = 0ECAC4AF1BC00AC500FDAE14 /* liblogging.dylib */; };
 		CD6F740C225187FD0081ED74 /* logger_test.cxx in Sources */ = {isa = PBXBuildFile; fileRef = CD6F73FC225187E10081ED74 /* logger_test.cxx */; };
 		CD6F746C22518A2C0081ED74 /* GoogleMock.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = CD6F746B22518A2C0081ED74 /* GoogleMock.framework */; };
+		CD760CB922621776008A62DE /* pattern_layout_test.cxx in Sources */ = {isa = PBXBuildFile; fileRef = CD760CB822621776008A62DE /* pattern_layout_test.cxx */; };
+		CD760CBF226221F6008A62DE /* console_appender_test.cxx in Sources */ = {isa = PBXBuildFile; fileRef = CD760CBE226221F6008A62DE /* console_appender_test.cxx */; };
+		CD760CC1226226CC008A62DE /* file_appender_test.cxx in Sources */ = {isa = PBXBuildFile; fileRef = CD760CC0226226CC008A62DE /* file_appender_test.cxx */; };
 		CD88E9572252BDFC00927F40 /* log_manager.cxx in Sources */ = {isa = PBXBuildFile; fileRef = CD88E9552252BDFC00927F40 /* log_manager.cxx */; };
 		CD88E95F2252D3EF00927F40 /* c_logger.cxx in Sources */ = {isa = PBXBuildFile; fileRef = CD88E95D2252D3EF00927F40 /* c_logger.cxx */; };
 		CD88E9632252D67A00927F40 /* common.cxx in Sources */ = {isa = PBXBuildFile; fileRef = CD88E9612252D67A00927F40 /* common.cxx */; };
@@ -90,6 +93,9 @@
 		CD6F742D225189290081ED74 /* libcfmt_logger.dylib */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.dylib"; path = libcfmt_logger.dylib; sourceTree = BUILT_PRODUCTS_DIR; };
 		CD6F742F225189470081ED74 /* GoogleMock.xcodeproj */ = {isa = PBXFileReference; lastKnownFileType = "wrapper.pb-project"; name = GoogleMock.xcodeproj; path = "../../../gmock-xcode-master/GoogleMock.xcodeproj"; sourceTree = "<group>"; };
 		CD6F746B22518A2C0081ED74 /* GoogleMock.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = GoogleMock.framework; path = "../../../gmock-xcode-master/build/Release/GoogleMock.framework"; sourceTree = "<group>"; };
+		CD760CB822621776008A62DE /* pattern_layout_test.cxx */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = pattern_layout_test.cxx; sourceTree = "<group>"; };
+		CD760CBE226221F6008A62DE /* console_appender_test.cxx */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = console_appender_test.cxx; sourceTree = "<group>"; };
+		CD760CC0226226CC008A62DE /* file_appender_test.cxx */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = file_appender_test.cxx; sourceTree = "<group>"; };
 		CD88E9552252BDFC00927F40 /* log_manager.cxx */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = log_manager.cxx; sourceTree = "<group>"; };
 		CD88E95D2252D3EF00927F40 /* c_logger.cxx */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = c_logger.cxx; sourceTree = "<group>"; };
 		CD88E9612252D67A00927F40 /* common.cxx */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = common.cxx; sourceTree = "<group>"; };
@@ -172,6 +178,9 @@
 			children = (
 				CD6F73FC225187E10081ED74 /* logger_test.cxx */,
 				CD1CDEB22256B04600E5B6B2 /* format_test.cxx */,
+				CD760CB822621776008A62DE /* pattern_layout_test.cxx */,
+				CD760CBE226221F6008A62DE /* console_appender_test.cxx */,
+				CD760CC0226226CC008A62DE /* file_appender_test.cxx */,
 				CD1CDE8C22540D9B00E5B6B2 /* c_logger_test.cxx */,
 				CD1CDE8F22542CC500E5B6B2 /* log_manager_test.cxx */,
 				CD1CDE9122543E7E00E5B6B2 /* test_properties.cxx */,
@@ -365,8 +374,11 @@
 			isa = PBXSourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
+				CD760CBF226221F6008A62DE /* console_appender_test.cxx in Sources */,
 				CD1CDE9022542CC500E5B6B2 /* log_manager_test.cxx in Sources */,
 				CD6F740C225187FD0081ED74 /* logger_test.cxx in Sources */,
+				CD760CB922621776008A62DE /* pattern_layout_test.cxx in Sources */,
+				CD760CC1226226CC008A62DE /* file_appender_test.cxx in Sources */,
 				CD1CDE9222543E7E00E5B6B2 /* test_properties.cxx in Sources */,
 				CD1CDEB32256B04600E5B6B2 /* format_test.cxx in Sources */,
 				CD1CDE8D22540D9B00E5B6B2 /* c_logger_test.cxx in Sources */,
@@ -416,7 +428,7 @@
 				GCC_WARN_UNUSED_VARIABLE = YES;
 				ONLY_ACTIVE_ARCH = YES;
 				SDKROOT = macosx;
-				USER_HEADER_SEARCH_PATHS = "$(PROJECT_DIR)/include/ $(PROJECT_DIR)/extern/expect/include/ $(PROJECT_DIR)/extern/resource_factory/include/ $(PROJECT_DIR)/extern/";
+				USER_HEADER_SEARCH_PATHS = "$(PROJECT_DIR)/include/ $(PROJECT_DIR)/extern/expect/include/ $(PROJECT_DIR)/extern/resource_factory/include/ $(PROJECT_DIR)/extern/scoped_buffer_capture/include/ $(PROJECT_DIR)/extern/";
 			};
 			name = Debug;
 		};
@@ -450,7 +462,7 @@
 				GCC_WARN_UNUSED_FUNCTION = YES;
 				GCC_WARN_UNUSED_VARIABLE = YES;
 				SDKROOT = macosx;
-				USER_HEADER_SEARCH_PATHS = "$(PROJECT_DIR)/include/ $(PROJECT_DIR)/extern/expect/include/ $(PROJECT_DIR)/extern/resource_factory/include/ $(PROJECT_DIR)/extern/";
+				USER_HEADER_SEARCH_PATHS = "$(PROJECT_DIR)/include/ $(PROJECT_DIR)/extern/expect/include/ $(PROJECT_DIR)/extern/resource_factory/include/ $(PROJECT_DIR)/extern/scoped_buffer_capture/include/ $(PROJECT_DIR)/extern/";
 			};
 			name = Release;
 		};

logger.xcodeproj/xcshareddata/xcschemes/logger_test.xcscheme

@@ -11,7 +11,17 @@
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
       codeCoverageEnabled = "YES"
+      onlyGenerateCoverageForSpecifiedTargets = "YES"
       shouldUseLaunchSchemeArgsEnv = "YES">
+      <CodeCoverageTargets>
+         <BuildableReference
+            BuildableIdentifier = "primary"
+            BlueprintIdentifier = "0ECAC4AE1BC00AC500FDAE14"
+            BuildableName = "liblogging.dylib"
+            BlueprintName = "logging"
+            ReferencedContainer = "container:logger.xcodeproj">
+         </BuildableReference>
+      </CodeCoverageTargets>
       <Testables>
          <TestableReference
             skipped = "NO">

src/loggers/properties.cxx

@@ -65,7 +65,7 @@ namespace logging {
   
   properties properties::mergedWith(properties const & other) const {
     if (data.is<object_t>()) {
-      properties out;
+      properties out = property::_obj({});
       for (auto & pair : object()) {
         auto & to = out.data.get<object_t>()[pair.first];
         to = mergeKey(pair.first, pair.second, other);

test/console_appender_test.cxx

@@ -0,0 +1,87 @@
+//
+//  console_appender_test.cxx
+//  logger_test
+//
+//  Created by Sam Jaffe on 4/13/19.
+//
+
+#include <gmock/gmock.h>
+
+#include "resource_factory/prototype_factory.hpp"
+#include "scoped_buffer_capture.h"
+
+#include "logger/log_manager.h"
+#include "logger/properties.h"
+#include "mock_logger.h"
+
+class ConsoleAppenderTest : public testing::Test {
+protected:
+  void SetUp() override;
+  void TearDown() override;
+  
+  std::string cout() const { return cout_->str(); }
+  std::string cerr() const { return cerr_->str(); }
+  std::shared_ptr<logging::appender> get(logging::properties const &) const;
+private:
+  std::shared_ptr<StubLayout> playout;
+  std::unique_ptr<scoped_buffer_capture_t> cout_, cerr_;
+};
+
+void ConsoleAppenderTest::SetUp() {
+  playout.reset(new StubLayout);
+  cout_.reset(new scoped_buffer_capture_t(std::cout));
+  cerr_.reset(new scoped_buffer_capture_t(std::cerr));
+}
+
+void ConsoleAppenderTest::TearDown() {
+  cerr_.reset();
+  cout_.reset();
+  playout.reset();
+}
+
+std::shared_ptr<logging::appender>
+ConsoleAppenderTest::get(logging::properties const & props) const {
+  auto pappender = logging::appenders::instance().get("Console", props);
+  pappender->layout = playout;
+  return pappender;
+}
+
+TEST_F(ConsoleAppenderTest, ErrorOnUnknownTarget) {
+  using namespace logging::property;
+  logging::properties props{_obj({
+    {"target", _v("COUT")}
+  })};
+  
+  EXPECT_THROW(logging::appenders::instance().get("Console", props),
+               std::logic_error);
+}
+
+TEST_F(ConsoleAppenderTest, LogsToStdOut) {
+  using namespace logging;
+  using namespace logging::property;
+  properties props{_obj({
+    {"target", _v("SYSTEM_OUT")}
+  })};
+  
+  using testing::Eq;
+  using testing::IsEmpty;
+  logpacket pkt{{}, level::error, {}, {}, "This is a test message"};
+  EXPECT_NO_THROW(get(props)->write(pkt));
+  EXPECT_THAT(cout(), Eq("This is a test message"));
+  EXPECT_THAT(cerr(), IsEmpty());
+}
+
+TEST_F(ConsoleAppenderTest, LogsToStdErr) {
+  using namespace logging;
+  using namespace logging::property;
+  properties props{_obj({
+    {"target", _v("SYSTEM_ERR")}
+  })};
+  
+  using testing::Eq;
+  using testing::IsEmpty;
+  logpacket pkt{{}, level::error, {}, {}, "This is a test message"};
+  EXPECT_NO_THROW(get(props)->write(pkt));
+  EXPECT_THAT(cout(), IsEmpty());
+  EXPECT_THAT(cerr(), Eq("This is a test message"));
+}

test/file_appender_test.cxx

@@ -0,0 +1,109 @@
+//
+//  file_appender_test.cxx
+//  logger_test
+//
+//  Created by Sam Jaffe on 4/13/19.
+//
+
+#include <cstdlib>
+#include <fstream>
+#include <gmock/gmock.h>
+
+#include "resource_factory/prototype_factory.hpp"
+
+#include "logger/exception.h"
+#include "logger/log_manager.h"
+#include "logger/properties.h"
+#include "mock_logger.h"
+
+class FileAppenderTest : public testing::Test {
+protected:
+  void SetUp() override;
+  void TearDown() override;
+  
+  std::string filename() const { return data; }
+  std::shared_ptr<logging::appender> get(logging::properties const &) const;
+private:
+  std::shared_ptr<StubLayout> playout;
+  char data[24];
+};
+
+void FileAppenderTest::SetUp() {
+  strncpy(data, "test_log_fileXXXXXX.log", sizeof(data));
+  int fd = -1;
+  if ((fd = mkstemps(data, 4)) != -1) {
+    close(fd); // We'll open this elsewhere
+  } else {
+    throw std::runtime_error(strerror(errno));
+  }
+  playout.reset(new StubLayout);
+}
+
+void FileAppenderTest::TearDown() {
+  playout.reset();
+  remove(data);
+  memset(data, 0, sizeof(data));
+}
+
+std::shared_ptr<logging::appender>
+FileAppenderTest::get(logging::properties const & props) const {
+  auto pappender = logging::appenders::instance().get("File", props);
+  pappender->layout = playout;
+  return pappender;
+}
+
+TEST_F(FileAppenderTest, ThrowsIfNoFilename) {
+  EXPECT_THROW(logging::appenders::instance().get("File", {}),
+               logging::invalid_property_type);
+}
+
+std::string slurp(std::string const & filename) {
+  std::ifstream in(filename);
+  std::stringstream ss;
+  ss << in.rdbuf();
+  return ss.str();
+}
+
+TEST_F(FileAppenderTest, WritesFile) {
+  using namespace logging;
+  using namespace logging::property;
+  properties props{_obj({
+    {"filename", _v(filename())}
+  })};
+  
+  using testing::Eq;
+  logpacket pkt{{}, level::error, {}, {}, "This is a test message"};
+  EXPECT_NO_THROW(get(props)->write(pkt));
+  EXPECT_THAT(slurp(filename()), Eq("This is a test message"));
+}
+
+TEST_F(FileAppenderTest, AppendsToFile) {
+  using namespace logging;
+  using namespace logging::property;
+  properties props{_obj({
+    {"filename", _v(filename())}
+  })};
+  
+  using testing::Eq;
+  logpacket pkt{{}, level::error, {}, {}, "Test"};
+  for (int i = 0; i < 2; ++i) {
+    EXPECT_NO_THROW(get(props)->write(pkt));
+  }
+  EXPECT_THAT(slurp(filename()), Eq("TestTest"));
+}
+
+TEST_F(FileAppenderTest, OverwritesFileWithSetting) {
+  using namespace logging;
+  using namespace logging::property;
+  properties props{_obj({
+    {"filename", _v(filename())},
+    {"fileAppend", _v(false)}
+  })};
+  
+  using testing::Eq;
+  logpacket pkt{{}, level::error, {}, {}, "Test"};
+  for (int i = 0; i < 2; ++i) {
+    EXPECT_NO_THROW(get(props)->write(pkt));
+  }
+  EXPECT_THAT(slurp(filename()), Eq("Test"));
+}

test/format_test.cxx

@@ -53,7 +53,9 @@ TEST(FormatTest, CatchesRawContentAfterFmt) {
 }
 
 // Thursday, April 4, 2019 6:17:20 PM GMT
-constexpr const int NOW = 1554401840;
+namespace {
+  constexpr const int NOW = 1554401840;
+}
 
 TEST(FormatTest, HandlesDateFormatter) {
   using testing::Eq;

test/mock_logger.h

@@ -55,6 +55,12 @@ struct MockLayout : public logging::layout {
   MOCK_CONST_METHOD2(format, void(std::ostream&, logging::logpacket const&));
 };
 
+struct StubLayout : public logging::layout {
+  void format(std::ostream& os, logging::logpacket const& pkt) const {
+    os << pkt.message.str();
+  }
+};
+
 ACTION(LogMessage) {
   arg0 << arg1.message.str();
 }

test/pattern_layout_test.cxx

@@ -0,0 +1,37 @@
+//
+//  pattern_layout_test.cxx
+//  logger_test
+//
+//  Created by Sam Jaffe on 4/13/19.
+//
+
+#include <gmock/gmock.h>
+
+#include "resource_factory/prototype_factory.hpp"
+
+#include "logger/detail/layout.h"
+#include "logger/log_manager.h"
+#include "logger/logpacket.h"
+#include "logger/properties.h"
+
+// Thursday, April 4, 2019 6:17:20 PM GMT
+namespace {
+  constexpr const int NOW = 1554401840;
+}
+
+TEST(PatternLayoutTest, TestInvokesFormatter) {
+  using namespace logging;
+  using namespace logging::property;
+  properties props{_obj({
+    {"pattern", _v("%d{%I:%M:%S.%_ms} [%%] %-5.5p %.36c - %m")}
+  })};
+  auto playout = layouts::instance().get("PatternLayout", props);
+  
+  std::stringstream ss;
+  playout->format(ss, {{NOW, 0}, level::warning, {}, "UnitTest",
+    "This is a test message"});
+  
+  using testing::Eq;
+  EXPECT_THAT(ss.str(), Eq("06:17:20.000 [%] WARNI UnitTest -"
+                           " This is a test message"));
+}