CSV output: minor refactoring and clean up

output/csv/config.go

@@ -9,7 +9,6 @@ import (
 	"gopkg.in/guregu/null.v3"
 
 	"github.com/mstoykov/envconfig"
-	"github.com/sirupsen/logrus"
 	"go.k6.io/k6/lib/types"
 )
 
@@ -60,7 +59,7 @@ func (c Config) Apply(cfg Config) Config {
 }
 
 // ParseArg takes an arg string and converts it to a config
-func ParseArg(arg string, logger logrus.FieldLogger) (Config, error) {
+func ParseArg(arg string) (Config, error) {
 	c := NewConfig()
 
 	if !strings.Contains(arg, "=") {
@@ -95,7 +94,7 @@ func ParseArg(arg string, logger logrus.FieldLogger) (Config, error) {
 // GetConsolidatedConfig combines {default config values + JSON config +
 // environment vars + arg config values}, and returns the final result.
 func GetConsolidatedConfig(
-	jsonRawConf json.RawMessage, env map[string]string, arg string, logger logrus.FieldLogger,
+	jsonRawConf json.RawMessage, env map[string]string, arg string,
 ) (Config, error) {
 	result := NewConfig()
 	if jsonRawConf != nil {
@@ -117,7 +116,7 @@ func GetConsolidatedConfig(
 	result = result.Apply(envConfig)
 
 	if arg != "" {
-		urlConf, err := ParseArg(arg, logger)
+		urlConf, err := ParseArg(arg)
 		if err != nil {
 			return result, err
 		}

output/csv/config_test.go

@@ -4,15 +4,11 @@ import (
 	"testing"
 	"time"
 
-	"github.com/sirupsen/logrus"
-
 	"gopkg.in/guregu/null.v3"
 
-	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"go.k6.io/k6/lib/testutils"
 	"go.k6.io/k6/lib/types"
 )
 
@@ -103,11 +99,8 @@ func TestParseArg(t *testing.T) {
 		arg := arg
 		testCase := testCase
 
-		testLogger, hook := test.NewNullLogger()
-		testLogger.SetOutput(testutils.NewTestOutput(t))
-
 		t.Run(arg, func(t *testing.T) {
-			config, err := ParseArg(arg, testLogger)
+			config, err := ParseArg(arg)
 
 			if testCase.expectedErr {
 				assert.Error(t, err)
@@ -116,13 +109,6 @@ func TestParseArg(t *testing.T) {
 
 			require.NoError(t, err)
 			assert.Equal(t, testCase.config, config)
-
-			var entries []string
-			for _, v := range hook.AllEntries() {
-				assert.Equal(t, v.Level, logrus.WarnLevel)
-				entries = append(entries, v.Message)
-			}
-			assert.ElementsMatch(t, entries, testCase.expectedLogEntries)
 		})
 	}
 }

output/csv/output.go

@@ -5,7 +5,6 @@ import (
 	"compress/gzip"
 	"encoding/csv"
 	"fmt"
-	"os"
 	"sort"
 	"strconv"
 	"strings"
@@ -44,39 +43,16 @@ func New(params output.Params) (output.Output, error) {
 }
 
 func newOutput(params output.Params) (*Output, error) {
-	resTags := []string{}
-	ignoredTags := []string{}
-	tags := params.ScriptOptions.SystemTags.Map()
-	for tag, flag := range tags {
-		systemTag, err := metrics.SystemTagString(tag)
-		if err != nil {
-			return nil, err
-		}
-
-		// The non-indexable system tags are neither a "resTag"
-		// nor an "ignoreTag". They aren't a "resTag" as they
-		// aren't added as a column in the CSV. Yet they also
-		// shouldn't be ignored as they are added to the
-		// "metadata" column
-		if metrics.NonIndexableSystemTags.Has(systemTag) {
-			continue
-		}
-
-		if flag {
-			resTags = append(resTags, tag)
-		} else {
-			ignoredTags = append(ignoredTags, tag)
-		}
+	resTags, ignoredTags, err := buildTagSets(params)
+	if err != nil {
+		return nil, err
 	}
 
-	sort.Strings(resTags)
-	sort.Strings(ignoredTags)
-
 	logger := params.Logger.WithFields(logrus.Fields{
 		"output":   "csv",
 		"filename": params.ConfigArgument,
 	})
-	config, err := GetConsolidatedConfig(params.JSONConfig, params.Environment, params.ConfigArgument, logger.Logger)
+	config, err := GetConsolidatedConfig(params.JSONConfig, params.Environment, params.ConfigArgument)
 	if err != nil {
 		return nil, err
 	}
@@ -89,7 +65,7 @@ func newOutput(params output.Params) (*Output, error) {
 	fname := config.FileName.String
 
 	if fname == "" || fname == "-" {
-		stdoutWriter := csv.NewWriter(os.Stdout)
+		stdoutWriter := csv.NewWriter(params.StdOut)
 		return &Output{
 			fname:        "-",
 			resTags:      resTags,
@@ -137,6 +113,40 @@ func newOutput(params output.Params) (*Output, error) {
 	return &c, nil
 }
 
+// buildTagSets builds trackable and ignored tag sets from the
+// output params
+func buildTagSets(params output.Params) ([]string, []string, error) {
+	resTags := []string{}
+	ignoredTags := []string{}
+	tags := params.ScriptOptions.SystemTags.Map()
+	for tag, flag := range tags {
+		systemTag, err := metrics.SystemTagString(tag)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		// The non-indexable system tags are neither a "resTag"
+		// nor an "ignoreTag". They aren't a "resTag" as they
+		// aren't added as a column in the CSV. Yet they also
+		// shouldn't be ignored as they are added to the
+		// "metadata" column
+		if metrics.NonIndexableSystemTags.Has(systemTag) {
+			continue
+		}
+
+		if flag {
+			resTags = append(resTags, tag)
+		} else {
+			ignoredTags = append(ignoredTags, tag)
+		}
+	}
+
+	sort.Strings(resTags)
+	sort.Strings(ignoredTags)
+
+	return resTags, ignoredTags, nil
+}
+
 // Description returns a human-readable description of the output.
 func (o *Output) Description() string {
 	if o.fname == "" || o.fname == "-" { // TODO rename