From fb70bc6f3d3f22a40e65f32deea3cea1b6d70a76 Mon Sep 17 00:00:00 2001
From: Mihail Stoykov <M.Stoikov@gmail.com>
Date: Fri, 14 Jan 2022 11:50:35 +0200
Subject: [PATCH] cmd/convert refactor to not use global variables

---
 cmd/common.go       |  7 -------
 cmd/convert.go      | 32 +++++++++++++++-----------------
 cmd/convert_test.go | 19 ++++++++++---------
 cmd/root.go         |  3 ++-
 4 files changed, 27 insertions(+), 34 deletions(-)

diff --git a/cmd/common.go b/cmd/common.go
index 52e17c74608..d6ad8854e0a 100644
--- a/cmd/common.go
+++ b/cmd/common.go
@@ -37,13 +37,6 @@ import (
 	"go.k6.io/k6/loader"
 )
 
-// Use these when interacting with fs and writing to terminal, makes a command testable
-//nolint:gochecknoglobals
-var (
-	defaultFs               = afero.NewOsFs()
-	defaultWriter io.Writer = os.Stdout
-)
-
 // Panic if the given error is not nil.
 func must(err error) {
 	if err != nil {
diff --git a/cmd/convert.go b/cmd/convert.go
index 88e96a4f405..dd7dcf0acd4 100644
--- a/cmd/convert.go
+++ b/cmd/convert.go
@@ -26,6 +26,7 @@ import (
 	"io/ioutil"
 	"path/filepath"
 
+	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 	"gopkg.in/guregu/null.v3"
 
@@ -33,24 +34,21 @@ import (
 	"go.k6.io/k6/lib"
 )
 
-// TODO: fix this... or remove k6 convert
-//nolint: gochecknoglobals
-var (
-	convertOutput       string
-	optionsFilePath     string
-	minSleep            uint
-	maxSleep            uint
-	enableChecks        bool
-	returnOnFailedCheck bool
-	correlate           bool
-	threshold           uint
-	nobatch             bool
-	only                []string
-	skip                []string
-)
-
 //nolint:funlen,gocognit
-func getConvertCmd() *cobra.Command {
+func getConvertCmd(defaultFs afero.Fs, defaultWriter io.Writer) *cobra.Command {
+	var (
+		convertOutput       string
+		optionsFilePath     string
+		minSleep            uint
+		maxSleep            uint
+		enableChecks        bool
+		returnOnFailedCheck bool
+		correlate           bool
+		threshold           uint
+		nobatch             bool
+		only                []string
+		skip                []string
+	)
 	convertCmd := &cobra.Command{
 		Use:   "convert",
 		Short: "Convert a HAR file to a k6 script",
diff --git a/cmd/convert_test.go b/cmd/convert_test.go
index 48ca52796df..748989ed154 100644
--- a/cmd/convert_test.go
+++ b/cmd/convert_test.go
@@ -121,9 +121,10 @@ export default function() {
 }
 `
 
-//nolint:paralleltest // a lot of global variables, hopefully convert will just be dropped
 func TestIntegrationConvertCmd(t *testing.T) {
+	t.Parallel()
 	t.Run("Correlate", func(t *testing.T) {
+		t.Parallel()
 		harFile, err := filepath.Abs("correlate.har")
 		require.NoError(t, err)
 		har, err := ioutil.ReadFile("testdata/example.har")
@@ -132,15 +133,14 @@ func TestIntegrationConvertCmd(t *testing.T) {
 		expectedTestPlan, err := ioutil.ReadFile("testdata/example.js")
 		require.NoError(t, err)
 
-		defaultFs = afero.NewMemMapFs()
+		defaultFs := afero.NewMemMapFs()
 
 		err = afero.WriteFile(defaultFs, harFile, har, 0o644)
 		require.NoError(t, err)
 
 		buf := &bytes.Buffer{}
-		defaultWriter = buf
 
-		convertCmd := getConvertCmd()
+		convertCmd := getConvertCmd(defaultFs, buf)
 		assert.NoError(t, convertCmd.Flags().Set("correlate", "true"))
 		assert.NoError(t, convertCmd.Flags().Set("no-batch", "true"))
 		assert.NoError(t, convertCmd.Flags().Set("enable-status-code-checks", "true"))
@@ -175,28 +175,29 @@ func TestIntegrationConvertCmd(t *testing.T) {
 		}
 	})
 	t.Run("Stdout", func(t *testing.T) {
+		t.Parallel()
 		harFile, err := filepath.Abs("stdout.har")
 		require.NoError(t, err)
-		defaultFs = afero.NewMemMapFs()
+		defaultFs := afero.NewMemMapFs()
 		err = afero.WriteFile(defaultFs, harFile, []byte(testHAR), 0o644)
 		assert.NoError(t, err)
 
 		buf := &bytes.Buffer{}
-		defaultWriter = buf
 
-		convertCmd := getConvertCmd()
+		convertCmd := getConvertCmd(defaultFs, buf)
 		err = convertCmd.RunE(convertCmd, []string{harFile})
 		assert.NoError(t, err)
 		assert.Equal(t, testHARConvertResult, buf.String())
 	})
 	t.Run("Output file", func(t *testing.T) {
+		t.Parallel()
 		harFile, err := filepath.Abs("output.har")
 		require.NoError(t, err)
-		defaultFs = afero.NewMemMapFs()
+		defaultFs := afero.NewMemMapFs()
 		err = afero.WriteFile(defaultFs, harFile, []byte(testHAR), 0o644)
 		assert.NoError(t, err)
 
-		convertCmd := getConvertCmd()
+		convertCmd := getConvertCmd(defaultFs, nil)
 		err = convertCmd.Flags().Set("output", "/output.js")
 		defer func() {
 			err = convertCmd.Flags().Set("output", "")
diff --git a/cmd/root.go b/cmd/root.go
index 5b8b043d27e..3c6316d5487 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -36,6 +36,7 @@ import (
 	"github.com/mattn/go-colorable"
 	"github.com/mattn/go-isatty"
 	"github.com/sirupsen/logrus"
+	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
@@ -167,7 +168,7 @@ func Execute() {
 	c.cmd.AddCommand(
 		getArchiveCmd(logger),
 		getCloudCmd(ctx, logger),
-		getConvertCmd(),
+		getConvertCmd(afero.NewOsFs(), stdout),
 		getInspectCmd(logger),
 		loginCmd,
 		getPauseCmd(ctx),