From 7574bd2c2488a7f6a3bc804a8f12aede47383b17 Mon Sep 17 00:00:00 2001
From: Nedyalko Andreev <n@andreev.sh>
Date: Mon, 7 Mar 2022 14:56:14 +0200
Subject: [PATCH] Fix --log-output=file support for relative paths and add a
 test

---
 cmd/integration_test.go | 29 +++++++++++++++++++++++++++--
 cmd/root.go             |  5 ++++-
 log/file.go             | 26 +++++++++++++++++---------
 log/file_test.go        |  2 +-
 4 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/cmd/integration_test.go b/cmd/integration_test.go
index 846673c79863..ae60195d6ce6 100644
--- a/cmd/integration_test.go
+++ b/cmd/integration_test.go
@@ -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
@@ -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
@@ -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()
 
diff --git a/cmd/root.go b/cmd/root.go
index 23c623bb7677..2f10af801fc8 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -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
 		}
diff --git a/log/file.go b/log/file.go
index 2901c3f367c0..0cd87376c049 100644
--- a/log/file.go
+++ b/log/file.go
@@ -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,
@@ -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
 	}
 
@@ -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
diff --git a/log/file_test.go b/log/file_test.go
index ca4417f2bc5e..f7c4cde43792 100644
--- a/log/file_test.go
+++ b/log/file_test.go
@@ -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 {