mysql icon indicating copy to clipboard operation
mysql copied to clipboard

Logger interface compatible with testing package

Open drewwells opened this issue 7 years ago • 4 comments

Issue description

It would be convenient if the Logger interface was compatible with the testing logger (Logf).

Example code

func TestMyTest(t *testing.T) {
// start mysql in a test with a container for instance
// test polls mysql waiting for the connection to come up
// meanwhile go-sql-driver/mysql dumps errors into the testing package
  someCodeSucceeds
// testing does not report log statements
}

Error log

mysql dumps messages into the terminal regardless of testing success or failure
[mysql] 2017/05/16 15:46:20 packets.go:33: unexpected EOF
[mysql] 2017/05/16 15:46:20 packets.go:33: unexpected EOF
[mysql] 2017/05/16 15:46:20 packets.go:33: unexpected EOF
[mysql] 2017/05/16 15:46:21 packets.go:33: unexpected EOF
[mysql] 2017/05/16 15:46:21 packets.go:33: unexpected EOF
[mysql] 2017/05/16 15:46:21 packets.go:33: unexpected EOF
[mysql] 2017/05/16 15:46:22 packets.go:33: unexpected EOF
[mysql] 2017/05/16 15:46:22 packets.go:33: unexpected EOF
[mysql] 2017/05/16 15:46:22 packets.go:33: unexpected EOF

Configuration

Driver version (or git SHA): 1.3.0 Go version: run go version in your console 1.8.1 Server version: E.g. MySQL 5.6, MariaDB 10.0.20 MySQL 5.6 Server OS: E.g. Debian 8.1 (Jessie), Windows 10 Oracle Linux 7

drewwells avatar May 16 '17 15:05 drewwells

Probably the best way to implement this without breaking backwards compat is to change the Logger to an empty interface. Then use type inference to choose different loggers

type Logger interface {}

type printer interface {
  Print(v ...interface{})
}

type logfer interface {
  Logf(fmt string, args ...interface{})
}

type errLog struct {
  logger Logger
}

func (el errLog) Print(args ...interface{}) {
    switch el.logger.(type) {
       case logger:
         el.logger.Logf("%v", args...)
       case printer:
         el.logger.Print(args...)
    }
}

drewwells avatar May 16 '17 15:05 drewwells

Workaround

type sqlLogger struct {
	*testing.T
}

func (s sqlLogger) Print(args ...interface{}) {
	s.T.Logf("%v", args...)
}

func TestA(t *testing.T) {
  mysql.SetLogger(sqlLogger{t})
}

drewwells avatar May 16 '17 16:05 drewwells

Although I cannot argue the convenience argument (mostly because it's subjective), I think what you labeled as "Workaround" is actually the solution. Interfaces are there to be defined by the consumer, not by other code we want it to be compatible with.

Changing to an empty interface is a bad idea IMHO, defeats the whole purpose of interfaces.

I would just leave things as they are. If that 6 extra LOC is really bothering you, I would just try to contribute it as an optional implementation.

sagikazarmark avatar Jan 10 '19 03:01 sagikazarmark

My pet peeve with the logger is that is Global on package level. This means that is not possible to associate errors with contexts, requests or anything like it.

So wouldn't t be leaked and overwritten across parallel tests?

josesa avatar Nov 29 '21 11:11 josesa