Skip to content

Commit

Permalink
Fix --log-output=file support for relative paths and add a test
Browse files Browse the repository at this point in the history
  • Loading branch information
na-- committed Mar 7, 2022
1 parent 9536a68 commit 7574bd2
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 13 deletions.
29 changes: 27 additions & 2 deletions cmd/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

const (
noopDefaultFunc = `export default function() {};`
fooLogDefaultFunc = `export default function() { console.log('foo'); };`
noopHandleSummary = `
export function handleSummary(data) {
return {}; // silence the end of test summary
Expand Down Expand Up @@ -61,10 +62,10 @@ func TestStdoutAndStderrAreEmptyWithQuietAndLogsForwarded(t *testing.T) {
"k6", "--quiet", "--log-output", "file=" + logFilePath,
"--log-format", "raw", "run", "--no-summary", "-",
}
ts.stdIn = bytes.NewBufferString(`export default function() { console.log('foo'); };`)
ts.stdIn = bytes.NewBufferString(fooLogDefaultFunc)
newRootCommand(ts.globalState).execute()

// The our test state hook still catches this message
// The test state hook still catches this message
assert.True(t, testutils.LogContains(ts.loggerHook.Drain(), logrus.InfoLevel, `foo`))

// But it's not shown on stderr or stdout
Expand All @@ -77,6 +78,30 @@ func TestStdoutAndStderrAreEmptyWithQuietAndLogsForwarded(t *testing.T) {
assert.Equal(t, "foo\n", string(logContents))
}

func TestRelativeLogPathWithSetupAndTeardown(t *testing.T) {
t.Parallel()

ts := newGlobalTestState(t)

ts.args = []string{"k6", "--log-output", "file=test.log", "--log-format", "raw", "run", "-i", "2", "-"}
ts.stdIn = bytes.NewBufferString(fooLogDefaultFunc + `
export function setup() { console.log('bar'); };
export function teardown() { console.log('baz'); };
`)
newRootCommand(ts.globalState).execute()

// The test state hook still catches these messages
logEntries := ts.loggerHook.Drain()
assert.True(t, testutils.LogContains(logEntries, logrus.InfoLevel, `foo`))
assert.True(t, testutils.LogContains(logEntries, logrus.InfoLevel, `bar`))
assert.True(t, testutils.LogContains(logEntries, logrus.InfoLevel, `baz`))

// And check that the log file also contains everything
logContents, err := afero.ReadFile(ts.fs, filepath.Join(ts.cwd, "test.log"))
require.NoError(t, err)
assert.Equal(t, "bar\nfoo\nfoo\nbaz\n", string(logContents))
}

func TestWrongCliFlagIterations(t *testing.T) {
t.Parallel()

Expand Down
5 changes: 4 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,10 @@ func (c *rootCommand) setupLoggers() (<-chan struct{}, error) {

case strings.HasPrefix(line, "file"):
ch = make(chan struct{}) // TODO: refactor, get it from the constructor
hook, err := log.FileHookFromConfigLine(c.globalState.ctx, c.globalState.fs, c.globalState.fallbackLogger, line, ch)
hook, err := log.FileHookFromConfigLine(
c.globalState.ctx, c.globalState.fs, c.globalState.getwd,
c.globalState.fallbackLogger, line, ch,
)
if err != nil {
return nil, err
}
Expand Down
26 changes: 17 additions & 9 deletions log/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,9 @@ type fileHook struct {

// FileHookFromConfigLine returns new fileHook hook.
func FileHookFromConfigLine(
ctx context.Context, fs afero.Fs, fallbackLogger logrus.FieldLogger, line string, done chan struct{},
ctx context.Context, fs afero.Fs, getCwd func() (string, error),
fallbackLogger logrus.FieldLogger, line string, done chan struct{},
) (logrus.Hook, error) {
// TODO: fix this so it works correctly with relative paths from the CWD

hook := &fileHook{
fs: fs,
fallbackLogger: fallbackLogger,
Expand All @@ -71,7 +70,7 @@ func FileHookFromConfigLine(
return nil, err
}

if err := hook.openFile(); err != nil {
if err := hook.openFile(getCwd); err != nil {
return nil, err
}

Expand Down Expand Up @@ -107,14 +106,23 @@ func (h *fileHook) parseArgs(line string) error {
}

// openFile opens logfile and initializes writers.
func (h *fileHook) openFile() error {
if _, err := h.fs.Stat(filepath.Dir(h.path)); os.IsNotExist(err) {
return fmt.Errorf("provided directory '%s' does not exist", filepath.Dir(h.path))
func (h *fileHook) openFile(getCwd func() (string, error)) error {
path := h.path
if !filepath.IsAbs(path) {
cwd, err := getCwd()
if err != nil {
return fmt.Errorf("'%s' is a relative path but could not determine CWD: %w", path, err)
}
path = filepath.Join(cwd, path)
}

if _, err := h.fs.Stat(filepath.Dir(path)); os.IsNotExist(err) {
return fmt.Errorf("provided directory '%s' does not exist", filepath.Dir(path))
}

file, err := h.fs.OpenFile(h.path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o600)
file, err := h.fs.OpenFile(path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o600)
if err != nil {
return fmt.Errorf("failed to open logfile %s: %w", h.path, err)
return fmt.Errorf("failed to open logfile %s: %w", path, err)
}

h.w = file
Expand Down
2 changes: 1 addition & 1 deletion log/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func TestFileHookFromConfigLine(t *testing.T) {
t.Parallel()

res, err := FileHookFromConfigLine(
context.Background(), afero.NewMemMapFs(), logrus.New(), test.line, make(chan struct{}),
context.Background(), afero.NewMemMapFs(), nil, logrus.New(), test.line, make(chan struct{}),
)

if test.err {
Expand Down

0 comments on commit 7574bd2

Please sign in to comment.