Parcourir la source

Merge branch 'master' into predefs

* master:
  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.
  Convert calling pattern on appender/layout pairing, and make it so appender contains the layout information. Old: 	message := format(pkt) 	write(message) -> os << message New: 	write(pkt) -> format(os, pkt) -> os << message
  Fix header search paths by importing submodules and using directives in the project config

# Conflicts:
#	src/loggers/pattern_layout.cxx
Sam Jaffe il y a 6 ans
Parent
commit
49092d3e35

+ 12 - 0
.gitmodules

@@ -0,0 +1,12 @@
+[submodule "extern/expect"]
+	path = extern/expect
+	url = freenas:utility/expect.git
+[submodule "extern/resource_factory"]
+	path = extern/resource_factory
+	url = freenas:utility/resource_factory.git
+[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

+ 1 - 0
extern/expect

@@ -0,0 +1 @@
+Subproject commit 5d08b8fcbfc385310666c55b5d070a99fab2c2c0

+ 1 - 0
extern/resource_factory

@@ -0,0 +1 @@
+Subproject commit 1a2b2209461a0bb046e69b183e3ac491495fb1e4

+ 1 - 0
extern/scoped_buffer_capture

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

+ 1 - 0
extern/variant

@@ -0,0 +1 @@
+Subproject commit caba555bb2dac91535e786d84ee71a29c93118df

+ 3 - 1
include/logger/detail/appender.h

@@ -3,16 +3,18 @@
 #include "logger/logger_fwd.h"
 
 namespace logging {
+  class layout;
   struct appender {
     appender(level min = level::debug) : min_log_level(min) {}
     virtual ~appender() = default;
-    virtual void write(std::string const & msg) = 0;
+    virtual void write(logpacket const & pkt) = 0;
     virtual void flush() = 0;
     
     bool should_log(level ll) const {
       return ll >= min_log_level;
     }
     
+    std::shared_ptr<layout> layout;
     level min_log_level;
   };
 }

+ 1 - 1
include/logger/detail/layout.h

@@ -7,6 +7,6 @@
 namespace logging {
   struct layout {
     virtual ~layout() = default;
-    virtual std::string format(logpacket const & pkt) const = 0;
+    virtual void format(std::ostream & os, logpacket const & pkt) const = 0;
   };
 }

+ 1 - 5
include/logger/detail/logger_impl.h

@@ -8,17 +8,13 @@
 
 namespace logging {
   class appender;
-  class layout;
 
   struct logger_impl {
-    using p_appender = std::shared_ptr<appender>;
-    using p_layout = std::shared_ptr<layout>;
-
     bool should_log(level ll) const;
     void write(logpacket const & pkt);
     void flush();
 
-    std::vector<std::pair<p_appender, p_layout>> impls;
+    std::vector<std::shared_ptr<appender>> impls;
     level min_log_level;
   };
 }

+ 1 - 1
include/logger/properties.h

@@ -12,7 +12,7 @@
 #include <string>
 #include <vector>
 
-#include "../../../types/variant/variant.hpp"
+#include "variant/variant.hpp"
 
 namespace logging {
   struct properties;

+ 14 - 2
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 */; };
@@ -91,6 +94,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>"; };
@@ -175,6 +181,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 */,
@@ -369,8 +378,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 */,
@@ -420,7 +432,7 @@
 				GCC_WARN_UNUSED_VARIABLE = YES;
 				ONLY_ACTIVE_ARCH = YES;
 				SDKROOT = macosx;
-				USER_HEADER_SEARCH_PATHS = "$(PROJECT_DIR)/include/ $(PROJECT_DIR)/../../paradigm/declarative/expect/include/";
+				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;
 		};
@@ -454,7 +466,7 @@
 				GCC_WARN_UNUSED_FUNCTION = YES;
 				GCC_WARN_UNUSED_VARIABLE = YES;
 				SDKROOT = macosx;
-				USER_HEADER_SEARCH_PATHS = "$(PROJECT_DIR)/include/ $(PROJECT_DIR)/../../paradigm/declarative/expect/include/";
+				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;
 		};

+ 10 - 0
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">

+ 7 - 7
src/log_manager.cxx

@@ -10,8 +10,8 @@
 #include <memory>
 #include <unordered_map>
 
-#include "../../../paradigm/declarative/expect/include/expect/expect.hpp"
-#include "../../../types/resource_factory/include/resource_factory/prototype_factory.hpp"
+#include "expect/expect.hpp"
+#include "resource_factory/prototype_factory.hpp"
 
 #include "common.h"
 #include "logger/c_logger.h"
@@ -39,7 +39,7 @@ struct logging::manager_impl {
   void configure_loggers(properties const & props);
 
   p_logger default_logger;
-  cache<std::pair<p_appender, p_layout>> appenders;
+  cache<p_appender> appenders;
   cache<p_logger> loggers;
 };
 
@@ -93,10 +93,10 @@ void inject_log_level(properties const & props, T & val) {
 void manager_impl::configure_appenders(properties const & props) {
   // TODO: Support multiple File loggers, etc.
   for (auto & app : props["appenders"].object()) {
-    auto pair = std::make_pair(load_appender(app.first, app.second),
-                               load_layout(app.first, app.second));
-    inject_log_level(app.second, *pair.first);
-    appenders[app.first] = pair;
+    auto appender = load_appender(app.first, app.second);
+    appender->layout = load_layout(app.first, app.second);
+    inject_log_level(app.second, *appender);
+    appenders[app.first] = appender;
   }
 }
 

+ 5 - 5
src/logger_impl.cxx

@@ -19,15 +19,15 @@ bool logger_impl::should_log(level ll) const {
 
 void logger_impl::write(logpacket const & pkt) {
   if (!should_log(pkt.level)) return;
-  for (auto & pair : impls) {
-    if (pair.first->should_log(pkt.level)) {
-      pair.first->write(pair.second->format(pkt));
+  for (auto & appender : impls) {
+    if (appender->should_log(pkt.level)) {
+      appender->write(pkt);
     }
   }
 }
 
 void logger_impl::flush() {
-  for (auto & pair : impls) {
-    pair.first->flush();
+  for (auto & appender : impls) {
+    appender->flush();
   }
 }

+ 6 - 5
src/loggers/console_appender.cxx

@@ -7,10 +7,11 @@
 
 #include <iostream>
 
-#include "../../../../paradigm/declarative/expect/include/expect/expect.hpp"
-#include "../../../../types/resource_factory/include/resource_factory/prototype_factory.hpp"
+#include "expect/expect.hpp"
+#include "resource_factory/prototype_factory.hpp"
 
 #include "logger/detail/appender.h"
+#include "logger/detail/layout.h"
 #include "logger/log_manager.h"
 #include "logger/properties.h"
 
@@ -22,7 +23,7 @@ public:
   
   explicit console_appender(std::ostream& os);
   
-  void write(std::string const & msg) override;
+  void write(logpacket const & pkt) override;
   
   void flush() override;
 private:
@@ -42,8 +43,8 @@ std::shared_ptr<appender> console_appender::create(properties const& props) {
 
 console_appender::console_appender(std::ostream& os) : out_(os) {}
 
-void console_appender::write(std::string const & msg) {
-  out_ << msg;
+void console_appender::write(logpacket const & pkt) {
+  layout->format(out_, pkt);
 }
 
 void console_appender::flush() {

+ 4 - 4
src/loggers/default_layout.cxx

@@ -5,7 +5,7 @@
 //  Created by Sam Jaffe on 4/3/19.
 //
 
-#include "../../../../types/resource_factory/include/resource_factory/prototype_factory.hpp"
+#include "resource_factory/prototype_factory.hpp"
 
 #include "logger/detail/layout.h"
 #include "logger/log_manager.h"
@@ -15,15 +15,15 @@ using namespace logging;
 
 struct default_layout : public layout {
   static std::shared_ptr<layout> create(properties const &);
-  std::string format(logpacket const & pkt) const override;
+  void format(std::ostream & os, logpacket const & pkt) const override;
 };
 
 std::shared_ptr<layout> default_layout::create(properties const &) {
   return std::make_shared<default_layout>();
 }
 
-std::string default_layout::format(logpacket const & pkt) const {
-  return pkt.message.str() + "\n";
+void default_layout::format(std::ostream & os, logpacket const & pkt) const {
+  os << pkt.message.str() << "\n";
 }
 
 namespace {

+ 6 - 5
src/loggers/file_appender.cxx

@@ -7,10 +7,11 @@
 
 #include <fstream>
 
-#include "../../../../paradigm/declarative/expect/include/expect/expect.hpp"
-#include "../../../../types/resource_factory/include/resource_factory/prototype_factory.hpp"
+#include "expect/expect.hpp"
+#include "resource_factory/prototype_factory.hpp"
 
 #include "logger/detail/appender.h"
+#include "logger/detail/layout.h"
 #include "logger/log_manager.h"
 #include "logger/properties.h"
 
@@ -31,7 +32,7 @@ public:
   
   explicit file_appender(properties const & props);
   
-  void write(std::string const & msg) override;
+  void write(logpacket const & pkt) override;
   void flush() override;
 private:
   bool flush_immediately_{false};
@@ -81,8 +82,8 @@ file_appender::file_appender(properties const & props)
   }
 }
 
-void file_appender::write(std::string const & msg) {
-  out_ << msg;
+void file_appender::write(logpacket const & pkt) {
+  layout->format(out_, pkt);
   if (flush_immediately_) { flush(); }
 }
 

+ 4 - 4
src/loggers/pattern_layout.cxx

@@ -5,7 +5,7 @@
 //  Created by Sam Jaffe on 4/4/19.
 //
 
-#include "../../../../types/resource_factory/include/resource_factory/prototype_factory.hpp"
+#include "resource_factory/prototype_factory.hpp"
 
 #include "logger/detail/layout.h"
 #include "logger/format.h"
@@ -18,7 +18,7 @@ struct pattern_layout : public layout {
   static std::shared_ptr<layout> create(properties const &);
   
   pattern_layout(properties const & props);
-  std::string format(logpacket const & pkt) const override;
+  void format(std::ostream & os, logpacket const & pkt) const override;
   
   class format formatter;
 };
@@ -43,8 +43,8 @@ std::shared_ptr<layout> pattern_layout::create(properties const & props) {
 pattern_layout::pattern_layout(properties const & props) :
   formatter(format::parse_format_string(props["pattern"])) {}
 
-std::string pattern_layout::format(logpacket const & pkt) const {
-  return formatter.process(pkt);
+void pattern_layout::format(std::ostream & os, logpacket const & pkt) const {
+  os << formatter.process(pkt);
 }
 
 namespace {

+ 2 - 2
src/loggers/properties.cxx

@@ -7,7 +7,7 @@
 
 #include "logger/properties.h"
 
-#include "../../../../paradigm/declarative/expect/include/expect/expect.hpp"
+#include "expect/expect.hpp"
 
 #include "logger/exception.h"
 
@@ -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);

+ 5 - 8
test/c_logger_test.cxx

@@ -31,30 +31,27 @@ TEST_F(CLoggerTest, FlushesOnFlushCall) {
 }
 
 TEST_F(CLoggerTest, LogsWithFmtCode) {
-  using testing::_;
-  EXPECT_CALL(*layout, format(MessageEq("5"))).Times(1);
+  EXPECT_CALL(*appender, write(MessageEq("5"))).Times(1);
   t_logger("", pimpl).errorf("%d", 5);
 }
 
 // TODO: This is wrong
 TEST_F(CLoggerTest, FmtLogHasNameInHeader) {
-  using testing::_;
   using testing::Field;
-  EXPECT_CALL(*layout, format(Field(&logpacket::logger, "TEST"))).Times(1);
+  EXPECT_CALL(*appender, write(Field(&logpacket::logger, "TEST"))).Times(1);
   t_logger("TEST", pimpl).errorf("%d", 5);
 }
 
 // TODO: This is wrong
 TEST_F(CLoggerTest, FmtLogHasLevelInHeader) {
-  using testing::_;
   using testing::Field;
-  EXPECT_CALL(*layout, format(Field(&logpacket::level, level::error))).Times(1);
+  auto IsError = Field(&logpacket::level, level::error);
+  EXPECT_CALL(*appender, write(IsError)).Times(1);
   t_logger("TEST", pimpl).errorf("%d", 5);
 }
 
 TEST_F(CLoggerTest, LogsRawData) {
-  using testing::_;
-  EXPECT_CALL(*layout, format(MessageEq("5"))).Times(1);
+  EXPECT_CALL(*appender, write(MessageEq("5"))).Times(1);
   t_logger("", pimpl).error("5");
 }
 

+ 87 - 0
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"));
+}

+ 109 - 0
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"));
+}

+ 3 - 1
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;

+ 6 - 10
test/log_manager_test.cxx

@@ -5,7 +5,7 @@
 //  Created by Sam Jaffe on 4/2/19.
 //
 
-#include "../../../types/resource_factory/include/resource_factory/prototype_factory.hpp"
+#include "resource_factory/prototype_factory.hpp"
 
 #include "logger/c_logger.h"
 #include "logger/log_manager.h"
@@ -64,8 +64,7 @@ TEST_F(LogManagerTest, CanFetchInjectedMock) {
   mgr.configure(MIN_PROPERTY_SCHEMA);
   
   EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-  EXPECT_CALL(*appender, write("TEST MESSAGE"));
-  EXPECT_CALL(*layout, format(_)).WillRepeatedly(ReturnMessage());
+  EXPECT_CALL(*appender, write(MessageEq("TEST MESSAGE")));
 
   c_logger l = mgr.c_get();
   l.error("TEST MESSAGE");
@@ -76,8 +75,7 @@ TEST_F(LogManagerTest, MultiplexMockLogsToMultipleImpls) {
   mgr.configure(MULTIPLEX_PROPERTY_SCHEMA);
   
   EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-  EXPECT_CALL(*appender, write("TEST MESSAGE")).Times(2);
-  EXPECT_CALL(*layout, format(_)).WillRepeatedly(ReturnMessage());
+  EXPECT_CALL(*appender, write(MessageEq("TEST MESSAGE"))).Times(2);
 
   c_logger l = mgr.c_get();
   l.error("TEST MESSAGE");
@@ -88,9 +86,8 @@ TEST_F(LogManagerTest, LevelCanBeBakedIntoAppenderProperties) {
   mgr.configure(APPENDER_LEVEL_PROPERTY_SCHEMA);
   
   EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-  EXPECT_CALL(*appender, write("TEST MESSAGE")).Times(1);
-  EXPECT_CALL(*appender, write("LOWER MESSAGE")).Times(0);
-  EXPECT_CALL(*layout, format(_)).WillRepeatedly(ReturnMessage());
+  EXPECT_CALL(*appender, write(MessageEq("TEST MESSAGE"))).Times(1);
+  EXPECT_CALL(*appender, write(MessageEq("LOWER MESSAGE"))).Times(0);
   
   c_logger l = mgr.c_get();
   l.warn("TEST MESSAGE");
@@ -102,8 +99,7 @@ TEST_F(LogManagerTest, LevelCanBeBakedIntoLoggerProperties) {
   mgr.configure(LOGGER_LEVEL_PROPERTY_SCHEMA);
   
   EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-  EXPECT_CALL(*appender, write("TEST MESSAGE")).Times(0);
-  EXPECT_CALL(*layout, format(_)).Times(0);
+  EXPECT_CALL(*appender, write(_)).Times(0);
   
   c_logger l = mgr.c_get();
   l.warn("TEST MESSAGE");

+ 4 - 2
test/logger_test.cxx

@@ -29,7 +29,8 @@ TEST_F(LoggerTest, FlushesOnFlushCall) {
 }
 
 TEST_F(LoggerTest, LogsWithBraceFmtCode) {
-  EXPECT_CALL(*layout, format(MessageEq("5"))).Times(1);
+  using testing::_;
+  EXPECT_CALL(*appender, write(MessageEq("5"))).Times(1);
   t_logger("", pimpl).log(level::error, "{}", 5);
 }
 
@@ -41,6 +42,7 @@ TEST_F(LoggerTest, DoesNotLogAboveLevel) {
 }
 
 TEST_F(LoggerTest, LogCurlyBraceLiteralByDoubling) {
-  EXPECT_CALL(*layout, format(MessageEq("{}"))).Times(1);
+  using testing::_;
+  EXPECT_CALL(*appender, write(MessageEq("{}"))).Times(1);
   t_logger("", pimpl).log(level::error, "{{}}", 5);
 }

+ 23 - 9
test/mock_logger.h

@@ -10,6 +10,8 @@
 
 #include <gmock/gmock.h>
 
+#include <sstream>
+
 #include "logger/detail/appender.h"
 #include "logger/detail/layout.h"
 #include "logger/detail/logger_impl.h"
@@ -36,15 +38,31 @@ struct MockAppender : public logging::appender {
   MockAppender() { SetLogLevel(logging::level::trace); }
   void SetLogLevel(logging::level ll) { min_log_level = ll; }
   MOCK_METHOD0(flush, void());
-  MOCK_METHOD1(write, void(std::string const &));
+  MOCK_METHOD1(write, void(logging::logpacket const &));
+};
+
+struct StubAppender : public logging::appender {
+  StubAppender() { SetLogLevel(logging::level::trace); }
+  void SetLogLevel(logging::level ll) { min_log_level = ll; }
+  void flush() override {}
+  void write(logging::logpacket const & pkt) override {
+    layout->format(stream, pkt);
+  }
+  std::stringstream stream;
 };
 
 struct MockLayout : public logging::layout {
-  MOCK_CONST_METHOD1(format, std::string(logging::logpacket const &));
+  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(ReturnMessage) {
-  return arg0.message.str();
+ACTION(LogMessage) {
+  arg0 << arg1.message.str();
 }
 
 MATCHER_P(MessageEq, value, "") {
@@ -54,25 +72,21 @@ MATCHER_P(MessageEq, value, "") {
 struct LoggerTest : public testing::Test {
   void SetUp() override {
     appender.reset(new MockAppender);
-    layout.reset(new MockLayout);
     pimpl = std::make_shared<logging::logger_impl>();
-    pimpl->impls.push_back({appender, layout});
+    pimpl->impls.push_back(appender);
     
     using testing::_;
     using testing::AnyNumber;
 
     EXPECT_CALL(*appender, write(_)).Times(AnyNumber());
     EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-    ON_CALL(*layout, format(_)).WillByDefault(ReturnMessage());
   }
   void TearDown() override {
     pimpl.reset();
-    layout.reset();
     appender.reset();
   }
   
   std::shared_ptr<MockAppender> appender;
-  std::shared_ptr<MockLayout> layout;
   std::shared_ptr<logging::logger_impl> pimpl;
 };
 

+ 37 - 0
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"));
+}