Skip to content

Commit

Permalink
refactor: change xlog constructor to functional options
Browse files Browse the repository at this point in the history
  • Loading branch information
dmke committed Jan 28, 2024
1 parent 4f53d5f commit 129696c
Show file tree
Hide file tree
Showing 17 changed files with 219 additions and 97 deletions.
19 changes: 7 additions & 12 deletions cmd/texd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,19 +272,14 @@ func printVersion() {
}

func setupLogger() (xlog.Logger, error) {
lvl, err := xlog.ParseLevel(logLevel)
if err != nil {
return nil, err
}

o := &slog.HandlerOptions{
AddSource: true,
// XXX: provide ReplaceAttr callback to normalize Source locations?
Level: lvl,
o := []xlog.Option{
xlog.LeveledString(logLevel),
xlog.WithSource(),
}

if texd.Development() {
return xlog.New(xlog.TypeText, os.Stderr, o)
o = append(o, xlog.WriteTo(os.Stderr), xlog.AsText())
} else {
o = append(o, xlog.WriteTo(os.Stdout), xlog.AsJSON())
}
return xlog.New(xlog.TypeJSON, os.Stdout, o)
return xlog.New(o...)
}
2 changes: 1 addition & 1 deletion exec/docker_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func NewDockerClient(log xlog.Logger, baseDir string) (h *DockerClient, err erro
}

if log == nil {
log = xlog.NewNop()
log = xlog.NewDiscard()
}
dc := &DockerClient{
log: log,
Expand Down
2 changes: 1 addition & 1 deletion exec/docker_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (s *dockerClientSuite) SetupTest() {
s.cli = &apiMock{}
s.subject = &DockerClient{
cli: s.cli,
log: xlog.NewNop(),
log: xlog.NewDiscard(),
}
}

Expand Down
8 changes: 4 additions & 4 deletions exec/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func (m *dockerClientMock) Run(ctx context.Context, tag, wd string, cmd []string

func TestDockerClient_Executor(t *testing.T) {
subject := (&DockerClient{
log: xlog.NewNop(),
log: xlog.NewDiscard(),
cli: &apiMock{},
}).Executor(&mockDocument{})
require.NotNil(t, subject)
Expand All @@ -37,7 +37,7 @@ func TestDockerExec_invalidDocument(t *testing.T) {
cli: nil, // not accessed
}

err := exec.Run(bg, xlog.NewNop())
err := exec.Run(bg, xlog.NewDiscard())
require.EqualError(t, err, "invalid document: "+io.ErrClosedPipe.Error())
}

Expand All @@ -55,7 +55,7 @@ func TestDockerExec_latexmkFailed(t *testing.T) {
cli: cli,
}

err := exec.Run(bg, xlog.NewNop())
err := exec.Run(bg, xlog.NewDiscard())
require.EqualError(t, err, "compilation failed: "+errStart.Error())
assert.True(t, tex.IsCompilationError(err))

Expand All @@ -78,6 +78,6 @@ func TestDockerExec_success(t *testing.T) {
cli: cli,
}

err := exec.Run(bg, xlog.NewNop())
err := exec.Run(bg, xlog.NewDiscard())
require.NoError(t, err)
}
2 changes: 1 addition & 1 deletion exec/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestLocalExec_Run(t *testing.T) {
// create local exec
exec := LocalExec(tt.doc).(*localExec)
exec.path = tt.path
err := exec.Run(context.Background(), xlog.NewNop())
err := exec.Run(context.Background(), xlog.NewDiscard())

if tt.expectedErr == "" {
assert.NoError(t, err)
Expand Down
12 changes: 6 additions & 6 deletions exec/mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestMock_Run_extractError(t *testing.T) {
doc := &mockDocument{"/", io.ErrClosedPipe, "a", nil}
subject := Mock(true, "content")(doc)

err := subject.Run(bg, xlog.NewNop())
err := subject.Run(bg, xlog.NewDiscard())
require.EqualError(t, err, "invalid document: "+io.ErrClosedPipe.Error())
}

Expand All @@ -34,15 +34,15 @@ func TestMock_Run_invalidMainfilePanics(t *testing.T) {
subject := Mock(true, "content")(doc)

require.PanicsWithValue(t, "malformed input file: missing extension",
func() { _ = subject.Run(bg, xlog.NewNop()) })
func() { _ = subject.Run(bg, xlog.NewDiscard()) })
}

func TestMock_Run_noAddFilePanics(t *testing.T) {
doc := &mockDocument{"/", nil, "a.tex", nil} // doesn't implement AddFile
subject := Mock(true, "content")(doc)

require.PanicsWithValue(t, "can't add files to document",
func() { _ = subject.Run(bg, xlog.NewNop()) })
func() { _ = subject.Run(bg, xlog.NewDiscard()) })
}

type mockDockumentWithAddFile struct {
Expand All @@ -64,7 +64,7 @@ func TestMock_Run_errorOnAddFilePanics(t *testing.T) {
doc.On("AddFile", "a.log", "content").Return(io.ErrClosedPipe)

require.PanicsWithError(t, "failed to store result file: "+io.ErrClosedPipe.Error(),
func() { _ = subject.Run(bg, xlog.NewNop()) })
func() { _ = subject.Run(bg, xlog.NewDiscard()) })
}

func TestMock_Run_shouldFailCapturesLog(t *testing.T) {
Expand All @@ -75,7 +75,7 @@ func TestMock_Run_shouldFailCapturesLog(t *testing.T) {

doc.On("AddFile", "a.log", "content").Return(nil)

err := subject.Run(bg, xlog.NewNop())
err := subject.Run(bg, xlog.NewDiscard())
require.EqualError(t, err, "compilation failed")
}

Expand All @@ -87,6 +87,6 @@ func TestMock_Run_shouldFailCapturesResult(t *testing.T) {

doc.On("AddFile", "a.pdf", "%PDF/1.5").Return(nil)

err := subject.Run(bg, xlog.NewNop())
err := subject.Run(bg, xlog.NewDiscard())
require.NoError(t, err)
}
4 changes: 2 additions & 2 deletions refstore/dir/dir_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestNew_dirNotWritable(t *testing.T) {

func TestDirAdapter_keepFiles(t *testing.T) {
require := require.New(t)
log := xlog.NewNop()
log := xlog.NewDiscard()

subject, err := NewMemory(nil, &refstore.KeepForever{})
require.NoError(err)
Expand Down Expand Up @@ -121,7 +121,7 @@ func TestDirAdapter_purgeFiles(t *testing.T) {

func TestDirAdapter_accessMap(t *testing.T) {
require := require.New(t)
log := xlog.NewNop()
log := xlog.NewDiscard()
f := dummyFile("01234567890")

for _, q := range []struct{ n, sz int }{
Expand Down
28 changes: 17 additions & 11 deletions service/middleware/logging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package middleware

import (
"bytes"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"

"github.com/digineo/texd/xlog"
"github.com/stretchr/testify/assert"
Expand All @@ -23,20 +24,25 @@ func TestLogging(t *testing.T) {
h = RequestID(h)

var buf bytes.Buffer
log, err := xlog.New(xlog.TypeText, &buf, &slog.HandlerOptions{
ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr {
if a.Key == slog.TimeKey {
return slog.Attr{}
}
return a
},
})
log, err := xlog.New(
xlog.AsText(),
xlog.WriteTo(&buf),
xlog.MockClock(time.Unix(1650000000, 0)),
)
require.NoError(t, err)

w := httptest.NewRecorder()
WithLogging(log)(h).ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/", nil))
require.Equal(t, http.StatusOK, w.Code)

msg := `level=INFO msg="" method=GET status=200 bytes=0 host=192.0.2.1 url=/` + "\n"
assert.Equal(t, msg, buf.String())
assert.Equal(t, strings.Join([]string{
"time=2022-04-15T07:20:00.000+02:00",
"level=INFO",
`msg=""`,
"method=GET",
"status=200",
"bytes=0",
"host=192.0.2.1",
"url=/",
}, " ")+"\n", buf.String())
}
2 changes: 1 addition & 1 deletion service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func Start(opts Options, log xlog.Logger) (func(context.Context) error, error) {
return newService(opts, log).start(opts.Addr)
}

var discardlog = xlog.NewNop()
var discardlog = xlog.NewDiscard()

func (svc *service) Logger() xlog.Logger {
if svc.log == nil {
Expand Down
4 changes: 2 additions & 2 deletions service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestSuite(t *testing.T) {
func (suite *testSuite) SetupSuite() {
require := suite.Require()

logger, err := xlog.New(xlog.TypeText, os.Stderr, nil)
logger, err := xlog.New(xlog.AsText(), xlog.WriteTo(os.Stderr))
suite.Require().NoError(err)
suite.logger = logger

Expand Down Expand Up @@ -260,7 +260,7 @@ func (suite *testSuite) TestService_refstore_useKnownRef() {
if err != nil {
panic(err)
}
if err = refs.Store(xlog.NewNop(), contents); err != nil {
if err = refs.Store(xlog.NewDiscard(), contents); err != nil {
panic(err)
}
contents.Close()
Expand Down
26 changes: 13 additions & 13 deletions service/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import (
"bytes"
"encoding/json"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"

Expand All @@ -29,7 +29,7 @@ func TestHandleStatus(t *testing.T) {
mode: "local",
compileTimeout: 3 * time.Second,
jobs: make(chan struct{}, 2),
log: xlog.NewNop(),
log: xlog.NewDiscard(),
}

req := httptest.NewRequest(http.MethodGet, "/status", nil)
Expand Down Expand Up @@ -60,14 +60,11 @@ func TestHandleStatus(t *testing.T) {

func TestHandleStatus_withFailIO(t *testing.T) {
var buf bytes.Buffer
log, err := xlog.New(xlog.TypeText, &buf, &slog.HandlerOptions{
ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr {
if a.Key == slog.TimeKey {
return slog.Attr{}
}
return a
},
})
log, err := xlog.New(
xlog.AsText(),
xlog.WriteTo(&buf),
xlog.MockClock(time.Unix(1650000000, 0)),
)
require.NoError(t, err)

svc := &service{
Expand All @@ -87,7 +84,10 @@ func TestHandleStatus_withFailIO(t *testing.T) {

assert.Equal(t, http.StatusOK, rec.code)
assert.Equal(t, mimeTypeJSON, rec.h.Get("Content-Type"))

msg := `level=ERROR msg="failed to write response" error="io: read/write on closed pipe"` + "\n"
assert.Equal(t, msg, buf.String())
assert.Equal(t, strings.Join([]string{
"time=2022-04-15T07:20:00.000+02:00",
"level=ERROR",
`msg="failed to write response"`,
`error="io: read/write on closed pipe"`,
}, " ")+"\n", buf.String())
}
8 changes: 4 additions & 4 deletions tex/document_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func testFileWriter(t *testing.T, s string, candidate bool) {
}

subject := fileWriter{
log: xlog.NewNop(),
log: xlog.NewDiscard(),
file: f,
wc: &nopCloser{Writer: &buf},
buf: make([]byte, 4),
Expand Down Expand Up @@ -224,7 +224,7 @@ func TestDocument(t *testing.T) {
subject := documentHelper{ //nolint:forcetypeassert
t: t,
fs: afero.Afero{Fs: afero.NewMemMapFs()},
document: NewDocument(xlog.NewNop(), DefaultEngine, "").(*document),
document: NewDocument(xlog.NewDiscard(), DefaultEngine, "").(*document),
}
subject.document.fs = subject.fs

Expand Down Expand Up @@ -279,7 +279,7 @@ func TestDocument_MainInput(t *testing.T) {
subject := documentHelper{ //nolint:forcetypeassert
t: t,
fs: afero.Afero{Fs: afero.NewMemMapFs()},
document: NewDocument(xlog.NewNop(), DefaultEngine, "").(*document),
document: NewDocument(xlog.NewDiscard(), DefaultEngine, "").(*document),
}
subject.document.fs = subject.fs

Expand Down Expand Up @@ -335,7 +335,7 @@ func TestNewDocument(t *testing.T) {
engine := NewEngine("foo")
image := "bar"

subject := NewDocument(xlog.NewNop(), engine, image)
subject := NewDocument(xlog.NewDiscard(), engine, image)
require.NotNil(t, subject)

assert.Equal(t, engine, subject.Engine())
Expand Down
2 changes: 1 addition & 1 deletion tex/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestFileCategory_String(t *testing.T) {
func TestMetrics(t *testing.T) {
assert, require := assert.New(t), require.New(t)

doc := NewDocument(xlog.NewNop(), DefaultEngine, "")
doc := NewDocument(xlog.NewDiscard(), DefaultEngine, "")
doc.(*document).fs = afero.NewMemMapFs()
for name, size := range map[string]int{
"input.tex": 10,
Expand Down
21 changes: 21 additions & 0 deletions xlog/discard.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package xlog

import "os"

type discard struct{}

// NewDiscard produces basically the same logger as
//
// xlog.New(xlog.Discard(), otheroptions...)
//
// but without the option overhead.
func NewDiscard() Logger {
return &discard{}
}

func (*discard) Debug(msg string, args ...any) {}
func (*discard) Info(msg string, args ...any) {}
func (*discard) Warn(msg string, args ...any) {}
func (*discard) Error(msg string, args ...any) {}
func (*discard) Fatal(msg string, args ...any) { os.Exit(1) }
func (d *discard) With(args ...any) Logger { return d }
Loading

0 comments on commit 129696c

Please sign in to comment.