From 4e4c4b679898ad845513a25500fbcb158765164e Mon Sep 17 00:00:00 2001 From: Alex Bramley Date: Sun, 13 Nov 2011 14:02:12 +0000 Subject: [PATCH] Migrate to split logging pkg. --- client/connection.go | 2 +- client/connection_test.go | 2 +- logging/logging.go | 235 -------------------------------------- logging/logging_test.go | 60 ---------- logging/mock_logging.go | 93 --------------- logging/mock_test.go | 127 -------------------- state/channel.go | 2 +- state/nick.go | 2 +- state/tracker.go | 2 +- state/tracker_test.go | 2 +- 10 files changed, 6 insertions(+), 521 deletions(-) delete mode 100644 logging/logging.go delete mode 100644 logging/logging_test.go delete mode 100644 logging/mock_logging.go delete mode 100644 logging/mock_test.go diff --git a/client/connection.go b/client/connection.go index 26dcf79..aaf3f14 100644 --- a/client/connection.go +++ b/client/connection.go @@ -4,7 +4,7 @@ import ( "bufio" "crypto/tls" "github.com/fluffle/goirc/event" - "github.com/fluffle/goirc/logging" + "github.com/fluffle/golog/logging" "github.com/fluffle/goirc/state" "fmt" "net" diff --git a/client/connection_test.go b/client/connection_test.go index 479d9dd..8f22e41 100644 --- a/client/connection_test.go +++ b/client/connection_test.go @@ -3,7 +3,7 @@ package client import ( "bufio" "github.com/fluffle/goirc/event" - "github.com/fluffle/goirc/logging" + "github.com/fluffle/golog/logging" "github.com/fluffle/goirc/state" "gomock.googlecode.com/hg/gomock" "testing" diff --git a/logging/logging.go b/logging/logging.go deleted file mode 100644 index bc19bcb..0000000 --- a/logging/logging.go +++ /dev/null @@ -1,235 +0,0 @@ -package logging - -import ( - "flag" - "fmt" - "io" - "log" - "os" - "sync" -) - -// A simple level-based logging system. - -// Note that higher levels of logging are still usable via Log(). They will be -// output to the debug log in split mode if --log.level is set high enough. - -// Also, remember to call flag.Parse() near the start of your func main()! - -// The enforced singleton style of the standard "log" pkg is very nice, but -// it encourages people to write less testable code, and while logging is one -// of the few places where a singleton is not necessarily bad practise, it's -// not *that* hard to propagate your logging to where it needs to be. -// Alternatively you can create your own damn singleton with this package ;-) - -type LogLevel int -type LogMap map[LogLevel]*log.Logger - -const ( - Fatal LogLevel = iota - 1 - Error - Warn - Info - Debug -) - -var logString map[LogLevel]string = map[LogLevel]string{ - Fatal: "FATAL", - Error: "ERROR", - Warn: "WARN", - Info: "INFO", - Debug: "DEBUG", -} -func LogString(lv LogLevel) string { - if s, ok := logString[lv]; ok { - return s - } - return fmt.Sprintf("LOG(%d)", lv) -} - -var ( - file = flag.String("log.file", "", - "Log to this file rather than STDERR") - level = flag.Int("log.level", int(Error), - "Level of logging to be output") - only = flag.Bool("log.only", false, - "Only log output at the selected level") - split = flag.Bool("log.split", false, - "Log to one file per log level Error/Warn/Info/Debug.") - - // Shortcut flags for great justice - quiet = flag.Bool("log.quiet", false, - "Only fatal output (equivalent to -v -1)") - warn = flag.Bool("log.warn", false, - "Warning output (equivalent to -v 1)") - info = flag.Bool("log.info", false, - "Info output (equivalent to -v 2)") - debug = flag.Bool("log.debug", false, - "Debug output (equivalent to -v 3)") -) - -type Logger interface { - // Log at a given level - Log(LogLevel, string, ...interface{}) - // Log at level 3 - Debug(string, ...interface{}) - // Log at level 2 - Info(string, ...interface{}) - // Log at level 1 - Warn(string, ...interface{}) - // Log at level 0 - Error(string, ...interface{}) - // Log at level -1, to STDERR always, and exit after logging. - Fatal(string, ...interface{}) - // Change the current log display level - SetLogLevel(LogLevel) - // Set the logger to only output the current level - SetOnly(bool) -} - -// A struct to implement the above interface -type logger struct { - // We wrap a set of log.Logger for most of the heavy lifting - // but it can't be anonymous thanks to the conflicting definitions of Fatal - log LogMap - level LogLevel - only bool - *sync.Mutex // to ensure changing levels/flags is atomic -} - -// Helper function for opening log files, causes lots of Fatal :-) -func openLog(fn string) *log.Logger { - fh, err := os.OpenFile(fn, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0644) - if err != nil { - log.Fatalf("Error opening log file: %s", err) - } - return makeLogger(fh) -} - -// Helper function to create log.Loggers out of io.Writers -func makeLogger(w io.Writer) *log.Logger { - return log.New(w, "", log.LstdFlags | log.Lshortfile) -} - -// Creates a new logger object using the flags declared above. -// You MUST call flag.Parse before calling this ;-) -// Calling this more than once is inadvisable, you may get log corruption. -func NewFromFlags() *logger { - // Sanity checks: if log.split is set, must have a log.file. - if *split && *file == "" { - log.Fatalf("You must pass --log.file with --log.split") - } - - lv := Error - logMap := make(LogMap) - - // What are we logging? - // The shortcut flags prioritize by level, but an - // explicit level flag takes first precedence. - // I think the switch looks cleaner than if/else if, meh :-) - switch { - case *level != 0: - lv = LogLevel(*level) - case *quiet: - lv = Fatal - case *warn: - lv = Warn - case *info: - lv = Info - case *debug: - lv = Debug - } - - // Where are we logging to? - if *split { - // Fill in the logger map. - for l := Fatal; l <= Debug; l++ { - logMap[l] = openLog(*file + "." + logString[l]) - } - } else { - var _log *log.Logger - if *file != "" { - _log = openLog(*file) - } else { - _log = makeLogger(os.Stderr) - } - for l := Fatal; l <= Debug; l++ { - logMap[l] = _log - } - } - - return New(logMap, lv, *only) -} - -// You'll have to set up your own loggers for this one... -func New(m LogMap, lv LogLevel, only bool) *logger { - // Sanity check the log map we've been passed. - // We need loggers for all levels in case SetLogLevel is called. - for l := Fatal; l <= Debug; l++ { - if _log, ok := m[l]; !ok || _log == nil { - log.Fatalf("Output log level %s has no logger configured.", - logString[l]) - } - } - return &logger{m, lv, only, &sync.Mutex{}} -} - -// Internal function all others call to ensure identical call depth -func (l *logger) write(lv LogLevel, fm string, v ...interface{}) { - if lv > l.level || (l.only && lv != l.level) { - // Your logs are not important to us, goodnight - return - } - fm = fmt.Sprintf(LogString(lv)+" "+fm, v...) - if lv > Debug || lv < Fatal { - // This is an unrecognised log level, so log it to Debug - lv = Debug - } - - l.Lock() - defer l.Unlock() - // Writing the log is deceptively simple - l.log[lv].Output(3, fm) - if lv == Fatal { - // Always fatal to stderr too. Use panic so (a) we get a backtrace, - // and (b) it's trappable for testing (and maybe other times too). - log.Panic(fm) - } -} - -func (l *logger) Log(lv LogLevel, fm string, v ...interface{}) { - l.write(lv, fm, v...) -} - -// Helper functions for specific levels -func (l *logger) Debug(fm string, v ...interface{}) { - l.write(Debug, fm, v...) -} - -func (l *logger) Info(fm string, v ...interface{}) { - l.write(Info, fm, v...) -} - -func (l *logger) Warn(fm string, v ...interface{}) { - l.write(Warn, fm, v...) -} - -func (l *logger) Error(fm string, v ...interface{}) { - l.write(Error, fm, v...) -} - -func (l *logger) Fatal(fm string, v ...interface{}) { - l.write(Fatal, fm, v...) -} - -func (l *logger) SetLogLevel(lv LogLevel) { - l.Lock() - defer l.Unlock() - l.level = lv -} - -func (l *logger) SetOnly(only bool) { - l.Lock() - defer l.Unlock() - l.only = only -} diff --git a/logging/logging_test.go b/logging/logging_test.go deleted file mode 100644 index 886d06a..0000000 --- a/logging/logging_test.go +++ /dev/null @@ -1,60 +0,0 @@ -package logging - -import ( - "testing" -) - -// Note: the below is deliberately PLACED AT THE TOP OF THIS FILE because -// it is fragile. It ensures the right file:line is logged. Sorry! -func TestLogCorrectLineNumbers(t *testing.T) { - l, m := newMock(t) - l.Log(Error, "Error!") - // This breaks the mock encapsulation a little, but meh. - if s := string(m.m[Error].written); s[20:] != "logging_test.go:11: ERROR Error!\n" { - t.Errorf("Error incorrectly logged (check line numbers!)") - } -} - -func TestStandardLogging(t *testing.T) { - l, m := newMock(t) - l.SetLogLevel(Error) - - l.Log(4, "Nothing should be logged yet") - m.ExpectNothing() - - l.Log(Debug, "or yet...") - m.ExpectNothing() - - l.Log(Info, "or yet...") - m.ExpectNothing() - - l.Log(Warn, "or yet!") - m.ExpectNothing() - - l.Log(Error, "Error!") - m.Expect("Error!") -} - -func TestAllLoggingLevels(t *testing.T) { - l, m := newMock(t) - - l.Log(4, "Log to level 4.") - m.ExpectAt(4, "Log to level 4.") - - l.Debug("Log to debug.") - m.ExpectAt(Debug, "Log to debug.") - - l.Info("Log to info.") - m.ExpectAt(Info, "Log to info.") - - l.Warn("Log to warning.") - m.ExpectAt(Warn, "Log to warning.") - - l.Error("Log to error.") - m.ExpectAt(Error, "Log to error.") - - // recover to track the panic caused by Fatal. - defer func() { recover() }() - l.Fatal("Log to fatal.") - m.ExpectAt(Fatal, "Log to fatal.") -} diff --git a/logging/mock_logging.go b/logging/mock_logging.go deleted file mode 100644 index 1f5da56..0000000 --- a/logging/mock_logging.go +++ /dev/null @@ -1,93 +0,0 @@ -// Automatically generated by MockGen. DO NOT EDIT! -// Source: logging.go - -package logging - -import ( - gomock "gomock.googlecode.com/hg/gomock" -) - -// Mock of Logger interface -type MockLogger struct { - ctrl *gomock.Controller - recorder *_MockLoggerRecorder -} - -// Recorder for MockLogger (not exported) -type _MockLoggerRecorder struct { - mock *MockLogger -} - -func NewMockLogger(ctrl *gomock.Controller) *MockLogger { - mock := &MockLogger{ctrl: ctrl} - mock.recorder = &_MockLoggerRecorder{mock} - return mock -} - -func (m *MockLogger) EXPECT() *_MockLoggerRecorder { - return m.recorder -} - -func (m *MockLogger) Log(_param0 LogLevel, _param1 string, _param2 ...interface{}) { - m.ctrl.Call(m, "Log", _param0, _param1, _param2) -} - -func (mr *_MockLoggerRecorder) Log(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCall(mr.mock, "Log", arg0, arg1, arg2) -} - -func (m *MockLogger) Debug(_param0 string, _param1 ...interface{}) { - m.ctrl.Call(m, "Debug", _param0, _param1) -} - -func (mr *_MockLoggerRecorder) Debug(arg0 interface{}, arg1 ...interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCall(mr.mock, "Debug", arg0, arg1) -} - -func (m *MockLogger) Info(_param0 string, _param1 ...interface{}) { - m.ctrl.Call(m, "Info", _param0, _param1) -} - -func (mr *_MockLoggerRecorder) Info(arg0 interface{}, arg1 ...interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCall(mr.mock, "Info", arg0, arg1) -} - -func (m *MockLogger) Warn(_param0 string, _param1 ...interface{}) { - m.ctrl.Call(m, "Warn", _param0, _param1) -} - -func (mr *_MockLoggerRecorder) Warn(arg0 interface{}, arg1 ...interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCall(mr.mock, "Warn", arg0, arg1) -} - -func (m *MockLogger) Error(_param0 string, _param1 ...interface{}) { - m.ctrl.Call(m, "Error", _param0, _param1) -} - -func (mr *_MockLoggerRecorder) Error(arg0 interface{}, arg1 ...interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCall(mr.mock, "Error", arg0, arg1) -} - -func (m *MockLogger) Fatal(_param0 string, _param1 ...interface{}) { - m.ctrl.Call(m, "Fatal", _param0, _param1) -} - -func (mr *_MockLoggerRecorder) Fatal(arg0 interface{}, arg1 ...interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCall(mr.mock, "Fatal", arg0, arg1) -} - -func (m *MockLogger) SetLogLevel(_param0 LogLevel) { - m.ctrl.Call(m, "SetLogLevel", _param0) -} - -func (mr *_MockLoggerRecorder) SetLogLevel(arg0 interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCall(mr.mock, "SetLogLevel", arg0) -} - -func (m *MockLogger) SetOnly(_param0 bool) { - m.ctrl.Call(m, "SetOnly", _param0) -} - -func (mr *_MockLoggerRecorder) SetOnly(arg0 interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCall(mr.mock, "SetOnly", arg0) -} diff --git a/logging/mock_test.go b/logging/mock_test.go deleted file mode 100644 index 418b996..0000000 --- a/logging/mock_test.go +++ /dev/null @@ -1,127 +0,0 @@ -package logging - -import ( - "os" - "strings" - "testing" -) - -// TODO(fluffle): Assumes at most one logging line will be written -// between calls to Expect*. Change to be Expect(exp []string)? - -type mockWriter struct { - written []byte -} - -func (w *mockWriter) Write(p []byte) (n int, err os.Error) { - w.written = append(w.written, p...) - return len(p), nil -} - -func (w *mockWriter) getLine() string { - // 20 bytes covers the date and time in - // 2011/10/22 10:22:57 :: - if len(w.written) < 20 { - return "" - } - s := string(w.written) - idx := strings.Index(s, "\n") - s = s[20:idx] - w.written = w.written[idx+1:] - // consume ':: ' - idx = strings.Index(s, ":") + 1 - idx += strings.Index(s[idx:], ":") + 2 - return s[idx:] -} - -func (w *mockWriter) reset() { - w.written = w.written[:0] -} - -type writerMap struct { - t *testing.T - m map[LogLevel]*mockWriter -} - -// This doesn't create a mock Logger but a Logger that writes to mock outputs -// for testing purposes. Use the gomock-generated mock_logging package for -// external testing code that needs to mock out a logger. -func newMock(t *testing.T) (*logger, *writerMap) { - wMap := &writerMap{ - t: t, - m: map[LogLevel]*mockWriter{ - Debug: &mockWriter{make([]byte, 0)}, - Info: &mockWriter{make([]byte, 0)}, - Warn: &mockWriter{make([]byte, 0)}, - Error: &mockWriter{make([]byte, 0)}, - Fatal: &mockWriter{make([]byte, 0)}, - }, - } - logMap := make(LogMap) - for lv, w := range wMap.m { - logMap[lv] = makeLogger(w) - } - // Set the default log level high enough that everything will get logged - return New(logMap, (1 << 31) - 1, false), wMap -} - -// When you expect something to be logged but don't care so much what level at. -func (wm *writerMap) Expect(exp string) { - found := false - for lv, w := range wm.m { - if s := w.getLine(); s != "" && !found { - // Since we don't know what log level we're expecting, compare - // exp against the log line with the level stripped. - idx := strings.Index(s, " ") + 1 - if s[idx:] == exp { - found = true - } else { - wm.t.Errorf("Unexpected log message encountered at level %s:", - LogString(lv)) - wm.t.Errorf("exp: %s\ngot: %s", exp, s[idx:]) - } - } - } - wm.ExpectNothing() - if !found { - wm.t.Errorf("Expected log message not encountered:") - wm.t.Errorf("exp: %s", exp) - } -} - - -// When you expect nothing to be logged -func (wm *writerMap) ExpectNothing() { - for lv, w := range wm.m { - if s := w.getLine(); s != "" { - wm.t.Errorf("Unexpected log message at level %s:", - LogString(lv)) - wm.t.Errorf("%s", s) - w.reset() - } - } -} - -// When you expect something to be logged at a specific level. -func (wm *writerMap) ExpectAt(lv LogLevel, exp string) { - var w *mockWriter - if _, ok := wm.m[lv]; !ok { - w = wm.m[Debug] - } else { - w = wm.m[lv] - } - s := w.getLine() - exp = strings.Join([]string{LogString(lv), exp}, " ") - if s == "" { - wm.t.Errorf("Nothing logged at level %s:", LogString(lv)) - wm.t.Errorf("exp: %s", exp) - // Check nothing was written to a different log level here, too. - wm.ExpectNothing() - return - } - if s != exp { - wm.t.Errorf("Log message at level %s differed.", LogString(lv)) - wm.t.Errorf("exp: %s\ngot: %s", exp, s) - } - wm.ExpectNothing() -} diff --git a/state/channel.go b/state/channel.go index d555b22..8d50e52 100644 --- a/state/channel.go +++ b/state/channel.go @@ -2,7 +2,7 @@ package state import ( "fmt" - "github.com/fluffle/goirc/logging" + "github.com/fluffle/golog/logging" "reflect" "strconv" ) diff --git a/state/nick.go b/state/nick.go index 6dc4089..5fed228 100644 --- a/state/nick.go +++ b/state/nick.go @@ -1,7 +1,7 @@ package state import ( - "github.com/fluffle/goirc/logging" + "github.com/fluffle/golog/logging" "reflect" ) diff --git a/state/tracker.go b/state/tracker.go index 2ee8f2e..8ac7e80 100644 --- a/state/tracker.go +++ b/state/tracker.go @@ -1,7 +1,7 @@ package state import ( - "github.com/fluffle/goirc/logging" + "github.com/fluffle/golog/logging" ) // The state manager interface diff --git a/state/tracker_test.go b/state/tracker_test.go index d60da5f..30d7856 100644 --- a/state/tracker_test.go +++ b/state/tracker_test.go @@ -1,7 +1,7 @@ package state import ( - "github.com/fluffle/goirc/logging" + "github.com/fluffle/golog/logging" "gomock.googlecode.com/hg/gomock" "testing" )